Skip to content

Commit

Permalink
feat: provide better error messages in DEV (#11526)
Browse files Browse the repository at this point in the history
* feat: provide better error messages in DEV

* fix stuff

* fix stuff

* fix tests

* fix

* assert.include results in better errors on mismatches

* remove indentation

* tweak

* rename

* fix issues

* more fixes

* more fixes

* neaten up stack trace

* Update packages/svelte/src/internal/client/reactivity/effects.js

Co-authored-by: Rich Harris <rich.harris@vercel.com>

* Update packages/svelte/src/internal/client/runtime.js

Co-authored-by: Rich Harris <rich.harris@vercel.com>

* address feedback

* lint

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
  • Loading branch information
trueadm and Rich-Harris committed May 10, 2024
1 parent 641e411 commit fcc72ae
Show file tree
Hide file tree
Showing 16 changed files with 144 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-pants-push.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: provide better error messages in DEV
Expand Up @@ -449,13 +449,19 @@ export function client_component(source, analysis, options) {
b.id('import.meta.hot'),
b.block([
b.const(b.id('s'), b.call('$.source', b.id(analysis.name))),
b.const(b.id('filename'), b.member(b.id(analysis.name), b.id('filename'))),
b.stmt(b.assignment('=', b.id(analysis.name), b.call('$.hmr', b.id('s')))),
b.stmt(
b.assignment('=', b.member(b.id(analysis.name), b.id('filename')), b.id('filename'))
),
b.if(
b.id('import.meta.hot.acceptExports'),
b.stmt(
b.call('import.meta.hot.acceptExports', b.array([b.literal('default')]), accept_fn)
),
b.stmt(b.call('import.meta.hot.accept', accept_fn))
b.block([
b.stmt(
b.call('import.meta.hot.acceptExports', b.array([b.literal('default')]), accept_fn)
)
]),
b.block([b.stmt(b.call('import.meta.hot.accept', accept_fn))])
)
])
),
Expand Down
Expand Up @@ -232,7 +232,7 @@ function setup_select_synchronization(value_binding, context) {
context.state.init.push(
b.stmt(
b.call(
'$.render_effect',
'$.template_effect',
b.thunk(
b.block([
b.stmt(
Expand Down Expand Up @@ -448,7 +448,7 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) {
* Resulting code for dynamic looks something like this:
* ```js
* let value;
* $.render_effect(() => {
* $.template_effect(() => {
* if (value !== (value = 'new value')) {
* element.property = value;
* // or
Expand Down Expand Up @@ -1184,7 +1184,7 @@ function serialize_update(statement) {
const body =
statement.type === 'ExpressionStatement' ? statement.expression : b.block([statement]);

return b.stmt(b.call('$.render_effect', b.thunk(body)));
return b.stmt(b.call('$.template_effect', b.thunk(body)));
}

/**
Expand All @@ -1194,7 +1194,7 @@ function serialize_update(statement) {
function serialize_render_stmt(state) {
return state.update.length === 1
? serialize_update(state.update[0])
: b.stmt(b.call('$.render_effect', b.thunk(b.block(state.update))));
: b.stmt(b.call('$.template_effect', b.thunk(b.block(state.update))));
}

/**
Expand Down Expand Up @@ -1739,7 +1739,7 @@ export const template_visitors = {
state.init.push(
b.stmt(
b.call(
'$.render_effect',
'$.template_effect',
b.thunk(
b.block([
b.stmt(
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/constants.js
Expand Up @@ -143,7 +143,8 @@ export const disallowed_paragraph_contents = [
'pre',
'section',
'table',
'ul'
'ul',
'p'
];

// https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/index.js
Expand Up @@ -90,6 +90,7 @@ export {
legacy_pre_effect,
legacy_pre_effect_reset,
render_effect,
template_effect,
user_effect,
user_pre_effect
} from './reactivity/effects.js';
Expand Down
32 changes: 29 additions & 3 deletions packages/svelte/src/internal/client/reactivity/effects.js
Expand Up @@ -34,6 +34,7 @@ import { set } from './sources.js';
import { remove } from '../dom/reconciler.js';
import * as e from '../errors.js';
import { DEV } from 'esm-env';
import { define_property } from '../utils.js';

/**
* @param {'$effect' | '$effect.pre' | '$inspect'} rune
Expand Down Expand Up @@ -150,18 +151,25 @@ export function user_effect(fn) {

// Non-nested `$effect(...)` in a component should be deferred
// until the component is mounted
const defer =
var defer =
current_effect !== null &&
(current_effect.f & RENDER_EFFECT) !== 0 &&
// TODO do we actually need this? removing them changes nothing
current_component_context !== null &&
!current_component_context.m;

if (DEV) {
define_property(fn, 'name', {
value: '$effect'
});
}

if (defer) {
const context = /** @type {import('#client').ComponentContext} */ (current_component_context);
var context = /** @type {import('#client').ComponentContext} */ (current_component_context);
(context.e ??= []).push(fn);
} else {
effect(fn);
var signal = effect(fn);
return signal;
}
}

Expand All @@ -172,6 +180,11 @@ export function user_effect(fn) {
*/
export function user_pre_effect(fn) {
validate_effect('$effect.pre');
if (DEV) {
define_property(fn, 'name', {
value: '$effect.pre'
});
}
return render_effect(fn);
}

Expand Down Expand Up @@ -249,6 +262,19 @@ export function render_effect(fn) {
return create_effect(RENDER_EFFECT, fn, true);
}

/**
* @param {() => void | (() => void)} fn
* @returns {import('#client').Effect}
*/
export function template_effect(fn) {
if (DEV) {
define_property(fn, 'name', {
value: '{expression}'
});
}
return render_effect(fn);
}

/**
* @param {(() => void)} fn
* @param {number} flags
Expand Down
8 changes: 7 additions & 1 deletion packages/svelte/src/internal/client/render.js
Expand Up @@ -167,9 +167,15 @@ export function hydrate(component, options) {
return instance;
}, false);
} catch (error) {
if (!hydrated && options.recover !== false) {
if (
!hydrated &&
options.recover !== false &&
/** @type {Error} */ (error).message.includes('hydration_missing_marker_close')
) {
w.hydration_mismatch();

// If an error occured above, the operations might not yet have been initialised.
init_operations();
clear_text_content(target);

set_hydrating(false);
Expand Down
63 changes: 62 additions & 1 deletion packages/svelte/src/internal/client/runtime.js
Expand Up @@ -28,6 +28,9 @@ import { lifecycle_outside_component } from '../shared/errors.js';
const FLUSH_MICROTASK = 0;
const FLUSH_SYNC = 1;

// Used for DEV time error handling
/** @param {WeakSet<Error>} value */
const handled_errors = new WeakSet();
// Used for controlling the flush of effects.
let current_scheduler_mode = FLUSH_MICROTASK;
// Used for handling scheduling
Expand Down Expand Up @@ -239,6 +242,62 @@ export function check_dirtiness(reaction) {
return is_dirty;
}

/**
* @param {Error} error
* @param {import("#client").Effect} effect
* @param {import("#client").ComponentContext | null} component_context
*/
function handle_error(error, effect, component_context) {
// Given we don't yet have error boundaries, we will just always throw.
if (!DEV || handled_errors.has(error) || component_context === null) {
throw error;
}

const component_stack = [];

const effect_name = effect.fn.name;

if (effect_name) {
component_stack.push(effect_name);
}

/** @type {import("#client").ComponentContext | null} */
let current_context = component_context;

while (current_context !== null) {
var filename = current_context.function?.filename;

if (filename) {
const file = filename.split('/').at(-1);
component_stack.push(file);
}

current_context = current_context.p;
}

const indent = /Firefox/.test(navigator.userAgent) ? ' ' : '\t';
error.message += `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n`;

const stack = error.stack;

// Filter out internal files from callstack
if (stack) {
const lines = stack.split('\n');
const new_lines = [];
for (let i = 0; i < lines.length; i++) {
const line = lines[i];
if (line.includes('svelte/src/internal')) {
continue;
}
new_lines.push(line);
}
error.stack = new_lines.join('\n');
}

handled_errors.add(error);
throw error;
}

/**
* @template V
* @param {import('#client').Reaction} signal
Expand All @@ -260,7 +319,7 @@ export function execute_reaction_fn(signal) {
current_untracking = false;

try {
let res = signal.fn();
let res = (0, signal.fn)();
let dependencies = /** @type {import('#client').Value<unknown>[]} **/ (signal.deps);
if (current_dependencies !== null) {
let i;
Expand Down Expand Up @@ -431,6 +490,8 @@ export function execute_effect(effect) {
execute_effect_teardown(effect);
var teardown = execute_reaction_fn(effect);
effect.teardown = typeof teardown === 'function' ? teardown : null;
} catch (error) {
handle_error(/** @type {Error} */ (error), effect, current_component_context);
} finally {
current_effect = previous_effect;
current_component_context = previous_component_context;
Expand Down
Expand Up @@ -15,7 +15,7 @@ export default test({
component.componentName = 'banana';
throw new Error('Expected an error');
} catch (err) {
assert.equal(
assert.include(
/** @type {Error} */ (err).message,
'svelte_component_invalid_this_value\nThe `this={...}` property of a `<svelte:component>` must be a Svelte component, if defined'
);
Expand Down
Expand Up @@ -12,7 +12,7 @@ export default test({
component.componentName = 'banana';
throw new Error('Expected an error');
} catch (err) {
assert.equal(/** @type {Error} */ (err).message, '$$component is not a function');
assert.include(/** @type {Error} */ (err).message, '$$component is not a function');
}
}
});
4 changes: 2 additions & 2 deletions packages/svelte/tests/runtime-legacy/shared.ts
Expand Up @@ -392,9 +392,9 @@ async function run_test_variant(
}
} catch (err) {
if (config.runtime_error) {
assert.equal((err as Error).message, config.runtime_error);
assert.include((err as Error).message, config.runtime_error);
} else if (config.error && !unintended_error) {
assert.equal((err as Error).message, config.error);
assert.include((err as Error).message, config.error);
} else {
throw err;
}
Expand Down
Expand Up @@ -28,6 +28,6 @@ export default function Bind_component_snippet($$anchor) {

var text = $.sibling(node, true);

$.render_effect(() => $.set_text(text, ` value: ${$.stringify($.get(value))}`));
$.template_effect(() => $.set_text(text, ` value: ${$.stringify($.get(value))}`));
$.append($$anchor, fragment_1);
}
Expand Up @@ -13,17 +13,17 @@ export default function Main($$anchor) {
var custom_element = $.sibling($.sibling(svg, true));
var div_1 = $.sibling($.sibling(custom_element, true));

$.render_effect(() => $.set_attribute(div_1, "foobar", y()));
$.template_effect(() => $.set_attribute(div_1, "foobar", y()));

var svg_1 = $.sibling($.sibling(div_1, true));

$.render_effect(() => $.set_attribute(svg_1, "viewBox", y()));
$.template_effect(() => $.set_attribute(svg_1, "viewBox", y()));

var custom_element_1 = $.sibling($.sibling(svg_1, true));

$.render_effect(() => $.set_custom_element_data(custom_element_1, "fooBar", y()));
$.template_effect(() => $.set_custom_element_data(custom_element_1, "fooBar", y()));

$.render_effect(() => {
$.template_effect(() => {
$.set_attribute(div, "foobar", x);
$.set_attribute(svg, "viewBox", x);
$.set_custom_element_data(custom_element, "fooBar", x);
Expand Down
Expand Up @@ -8,7 +8,7 @@ export default function Each_string_template($$anchor) {
$.each(node, 1, () => ['foo', 'bar', 'baz'], $.index, ($$anchor, thing, $$index) => {
var text = $.text($$anchor);

$.render_effect(() => $.set_text(text, `${$.stringify($.unwrap(thing))}, `));
$.template_effect(() => $.set_text(text, `${$.stringify($.unwrap(thing))}, `));
$.append($$anchor, text);
});

Expand Down
Expand Up @@ -19,7 +19,7 @@ export default function Function_prop_no_getter($$anchor) {
children: ($$anchor, $$slotProps) => {
var text = $.text($$anchor);

$.render_effect(() => $.set_text(text, `clicks: ${$.stringify($.get(count))}`));
$.template_effect(() => $.set_text(text, `clicks: ${$.stringify($.get(count))}`));
$.append($$anchor, text);
}
});
Expand Down
Expand Up @@ -11,14 +11,20 @@ function Hmr($$anchor) {

if (import.meta.hot) {
const s = $.source(Hmr);
const filename = Hmr.filename;

Hmr = $.hmr(s);
Hmr.filename = filename;

if (import.meta.hot.acceptExports) import.meta.hot.acceptExports(["default"], (module) => {
$.set(s, module.default);
}); else import.meta.hot.accept((module) => {
$.set(s, module.default);
});
if (import.meta.hot.acceptExports) {
import.meta.hot.acceptExports(["default"], (module) => {
$.set(s, module.default);
});
} else {
import.meta.hot.accept((module) => {
$.set(s, module.default);
});
}
}

export default Hmr;
export default Hmr;

0 comments on commit fcc72ae

Please sign in to comment.