Skip to content

fix: add a warning when the misuse of reset in an error:boundary causes an error to be thrown when flushing the boundary content #16171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
36 changes: 20 additions & 16 deletions packages/svelte/src/internal/client/dom/blocks/boundary.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '#client/constants';
import { component_context, set_component_context } from '../../context.js';
import { invoke_error_boundary } from '../../error-handling.js';
import { handle_error, invoke_error_boundary } from '../../error-handling.js';
import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js';
import {
active_effect,
Expand Down Expand Up @@ -36,6 +36,8 @@ function with_boundary(boundary, fn) {

try {
fn();
} catch (e) {
handle_error(e);
} finally {
set_active_effect(previous_effect);
set_active_reaction(previous_reaction);
Expand Down Expand Up @@ -74,7 +76,16 @@ export function boundary(node, props, boundary_fn) {
throw error;
}

if (boundary_effect) {
destroy_effect(boundary_effect);
} else if (hydrating) {
set_hydrate_node(hydrate_open);
next();
set_hydrate_node(remove_nodes());
}

var did_reset = false;
var calling_on_error = false;

var reset = () => {
if (did_reset) {
Expand All @@ -84,37 +95,30 @@ export function boundary(node, props, boundary_fn) {

did_reset = true;

if (calling_on_error) {
w.reset_misuse();
throw error;
}

pause_effect(boundary_effect);

with_boundary(boundary, () => {
is_creating_fallback = false;
try {
boundary_effect = branch(() => boundary_fn(anchor));
} catch (error) {
// If the new subtree immediately throws during mount, warn the dev.
w.reset_misuse();
throw error;
}
boundary_effect = branch(() => boundary_fn(anchor));
});
};

var previous_reaction = active_reaction;

try {
set_active_reaction(null);
calling_on_error = true;
onerror?.(error, reset);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rich-Harris People might want to auto-fix the state of the application inside the onerror for instance they might decide to show an error snackbar and reset a form without any confirmation from the user

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't work — the update is still in progress and everything gets torn. The svelte_boundary_reset_onerror message has an example of fixing it by waiting for a tick() before calling reset()

calling_on_error = false;
} finally {
set_active_reaction(previous_reaction);
}

if (boundary_effect) {
destroy_effect(boundary_effect);
} else if (hydrating) {
set_hydrate_node(hydrate_open);
next();
set_hydrate_node(remove_nodes());
}

if (failed) {
// Render the `failed` snippet in a microtask
queue_micro_task(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
test({ assert, target, warnings }) {
const [toggle] = target.querySelectorAll('button');

flushSync(() => toggle.click());
assert.htmlEqual(
target.innerHTML,
// TODO the synthetic stack shouldn't be part of the message here
`<button>toggle</button><p>yikes! in {expression} in undefined</p><button>reset</button>`
);

const [, reset] = target.querySelectorAll('button');
flushSync(() => reset.click());
assert.htmlEqual(
target.innerHTML,
`<button>toggle</button><p>yikes! in {expression} in undefined</p><button>reset</button>`
);

flushSync(() => toggle.click());

const [, reset2] = target.querySelectorAll('button');
flushSync(() => reset2.click());
assert.htmlEqual(target.innerHTML, `<button>toggle</button><p>hello!</p>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<script>
let must_throw = $state(false);

function throw_error() {
throw new Error('yikes!');
}
</script>

<button onclick={() => must_throw = !must_throw}>toggle</button>

<svelte:boundary>
<p>{must_throw ? throw_error() : 'hello!'}</p>

{#snippet failed(error, reset)}
<p>{error.message}</p>
<button onclick={reset}>reset</button>
{/snippet}
</svelte:boundary>


Loading