fix: ignore stale boundary resets after destruction (#18886)

- `destroy_effect` cleared a boundary's component context while retained
reset callbacks and queued error work could still re-enter its run path.
- Reset handling is intentionally shared with hydration by
[#18556](https://github.com/sveltejs/svelte/pull/18556), while `onerror`
is deferred for safe state mutation by
[#17561](https://github.com/sveltejs/svelte/pull/17561); neither path
checked the boundary lifetime.
- Make retained and deferred boundary continuations inert during
destruction instead of masking null contexts in `bind:this` or `#run`,
which would still let dead effects and callbacks restart.
- Cover resets from `onerror` and failed snippets, cleanup ordering,
delayed error transforms, DOM rendering, and hydration.

Fixes https://github.com/sveltejs/svelte/issues/18885
main
svelte-triage-bot[bot] 2 days ago committed by GitHub
parent 7fb397e78f
commit 7fe14a508a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: ignore stale boundary reset callbacks

@ -1,5 +1,11 @@
/** @import { Effect, Source, TemplateNode, } from '#client' */ /** @import { Effect, Source, TemplateNode, } from '#client' */
import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants'; import {
BOUNDARY_EFFECT,
DESTROYED,
DESTROYING,
EFFECT_PRESERVED,
EFFECT_TRANSPARENT
} from '#client/constants';
import { import {
HYDRATION_ERROR, HYDRATION_ERROR,
HYDRATION_START_ELSE, HYDRATION_START_ELSE,
@ -224,6 +230,8 @@ export class Boundary {
var calling_on_error = false; var calling_on_error = false;
const reset = () => { const reset = () => {
if (this.#is_destroyed()) return;
if (did_reset) { if (did_reset) {
w.svelte_boundary_reset_noop(); w.svelte_boundary_reset_noop();
return; return;
@ -247,6 +255,8 @@ export class Boundary {
}; };
const invoke_onerror = () => { const invoke_onerror = () => {
if (this.#is_destroyed()) return;
try { try {
calling_on_error = true; calling_on_error = true;
this.#props.onerror?.(error, reset); this.#props.onerror?.(error, reset);
@ -259,6 +269,10 @@ export class Boundary {
return { reset, invoke_onerror }; return { reset, invoke_onerror };
} }
#is_destroyed() {
return (this.#effect.f & (DESTROYED | DESTROYING)) !== 0;
}
#hydrate_pending_content() { #hydrate_pending_content() {
const pending = this.#props.pending; const pending = this.#props.pending;
if (!pending) return; if (!pending) return;
@ -267,6 +281,8 @@ export class Boundary {
this.#pending_effect = branch(() => pending(this.#anchor)); this.#pending_effect = branch(() => pending(this.#anchor));
queue_micro_task(() => { queue_micro_task(() => {
if (this.#is_destroyed()) return;
var fragment = (this.#offscreen_fragment = document.createDocumentFragment()); var fragment = (this.#offscreen_fragment = document.createDocumentFragment());
var anchor = create_text(); var anchor = create_text();
var handled = false; var handled = false;
@ -465,7 +481,7 @@ export class Boundary {
if (this.#failed_effect) current_batch.skip_effect(this.#failed_effect); if (this.#failed_effect) current_batch.skip_effect(this.#failed_effect);
current_batch.oncommit(() => { current_batch.oncommit(() => {
this.#handle_error(error); if (!this.#is_destroyed()) this.#handle_error(error);
}); });
} else { } else {
this.#handle_error(error); this.#handle_error(error);
@ -501,11 +517,13 @@ export class Boundary {
/** @param {unknown} transformed_error */ /** @param {unknown} transformed_error */
const handle_error_result = (transformed_error) => { const handle_error_result = (transformed_error) => {
if (this.#is_destroyed()) return;
const { reset, invoke_onerror } = this.#create_reset(transformed_error); const { reset, invoke_onerror } = this.#create_reset(transformed_error);
invoke_onerror(); invoke_onerror();
if (failed) { if (failed && !this.#is_destroyed()) {
this.#failed_effect = this.#run(() => { this.#failed_effect = this.#run(() => {
try { try {
return branch(() => { return branch(() => {
@ -531,6 +549,8 @@ export class Boundary {
}; };
queue_micro_task(() => { queue_micro_task(() => {
if (this.#is_destroyed()) return;
// Run the error through the API-level transformError transform (e.g. SvelteKit's handleError) // Run the error through the API-level transformError transform (e.g. SvelteKit's handleError)
/** @type {unknown} */ /** @type {unknown} */
var result; var result;

@ -0,0 +1,50 @@
import { tick } from 'svelte';
import { test } from '../../test';
/** @type {Array<() => void>} */
const resolvers = [];
export default test({
transformError: (error) => new Promise((resolve) => resolvers.push(() => resolve(error))),
async test({ assert, target, logs }) {
const [error, toggle, reset, destroy] = target.querySelectorAll('button');
const paragraph = /** @type {HTMLParagraphElement} */ (target.querySelector('p'));
error.click();
await tick();
resolvers.shift()?.();
await tick();
assert.htmlEqual(paragraph.innerHTML, 'boom');
// A retained reset is inert after its boundary has been destroyed
toggle.click();
await tick();
reset.click();
await tick();
assert.htmlEqual(paragraph.innerHTML, 'boom');
// Resolving an error transform cannot resume a destroyed boundary
toggle.click();
await tick();
error.click();
await tick();
toggle.click();
await tick();
resolvers.shift()?.();
await tick();
assert.htmlEqual(paragraph.innerHTML, 'boom');
// A failed snippet's reset is also inert while the boundary is being destroyed
toggle.click();
await tick();
error.click();
await tick();
resolvers.shift()?.();
await tick();
destroy.click();
await tick();
assert.htmlEqual(paragraph.innerHTML, 'boom,boom');
assert.deepEqual(logs, ['render', 'render', 'render']);
}
});

@ -0,0 +1,47 @@
<script>
let show = $state(true);
let must_throw = $state(false);
let reset;
let element;
let errors = $state([]);
let reset_during_cleanup = false;
function throw_error() {
throw new Error('boom');
}
function toggle() {
must_throw = false;
show = !show;
}
function reset_on_cleanup(_, reset_boundary) {
return {
destroy() {
if (reset_during_cleanup) reset_boundary();
}
};
}
function track_render() {
console.log('render');
}
</script>
<button onclick={() => (must_throw = true)}>error</button>
<button onclick={toggle}>toggle</button>
<button onclick={() => reset()}>reset</button>
<button onclick={() => { reset_during_cleanup = true; toggle(); }}>destroy</button>
<p>{errors.join(',')}</p>
{#if show}
<svelte:boundary onerror={(error, fn) => { errors.push(error.message); reset = fn; }}>
{track_render()}
<input bind:this={element} />
{must_throw ? throw_error() : ''}
{#snippet failed(_, failed_reset)}
<input bind:this={element} use:reset_on_cleanup={failed_reset} />
{/snippet}
</svelte:boundary>
{/if}
Loading…
Cancel
Save