fix: defer error boundary rendering in forks (#18076)

RIght now, when an async error occurs inside a fork, the error UI will
show immediately. This change defers the removal of the current content
etc until the fork is committed. The BranchManager logic cannot be
reused here since this is not about pending work, and we cannot wait for
other pending work to complete if the fork commits. Instead, batches now
have a "on fork commit" callback set which the boundary pushes to. The
boundary's effects are skipped in the fork until it's committing, at
which point we "resume" the error logic (calling onerror, transforming
it, etc)

Fixes #18060
pull/18086/merge
Simon H 3 days ago committed by GitHub
parent 7796bac806
commit a4ce776070
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: defer error boundary rendering in forks

@ -35,7 +35,7 @@ import { queue_micro_task } from '../task.js';
import * as e from '../../errors.js';
import * as w from '../../warnings.js';
import { DEV } from 'esm-env';
import { Batch, current_batch, schedule_effect } from '../../reactivity/batch.js';
import { Batch, current_batch, previous_batch, schedule_effect } from '../../reactivity/batch.js';
import { internal_set, source } from '../../reactivity/sources.js';
import { tag } from '../../dev/tracing.js';
import { createSubscriber } from '../../../../reactivity/create-subscriber.js';
@ -386,15 +386,29 @@ export class Boundary {
/** @param {unknown} error */
error(error) {
var onerror = this.#props.onerror;
let failed = this.#props.failed;
// If we have nothing to capture the error, or if we hit an error while
// rendering the fallback, re-throw for another boundary to handle
if (!onerror && !failed) {
if (!this.#props.onerror && !this.#props.failed) {
throw error;
}
if (current_batch?.is_fork) {
if (this.#main_effect) current_batch.skip_effect(this.#main_effect);
if (this.#pending_effect) current_batch.skip_effect(this.#pending_effect);
if (this.#failed_effect) current_batch.skip_effect(this.#failed_effect);
current_batch.on_fork_commit(() => {
this.#handle_error(error);
});
} else {
this.#handle_error(error);
}
}
/**
* @param {unknown} error
*/
#handle_error(error) {
if (this.#main_effect) {
destroy_effect(this.#main_effect);
this.#main_effect = null;
@ -416,6 +430,8 @@ export class Boundary {
set_hydrate_node(skip_nodes());
}
var onerror = this.#props.onerror;
let failed = this.#props.failed;
var did_reset = false;
var calling_on_error = false;

@ -265,6 +265,8 @@ export function run(thunks) {
for (const fn of thunks.slice(1)) {
promise = promise
.then(() => {
restore();
if (errored) {
throw errored.error;
}
@ -273,7 +275,6 @@ export function run(thunks) {
throw STALE_REACTION;
}
restore();
return fn();
})
.catch(handle_error);

@ -120,6 +120,12 @@ export class Batch {
*/
#discard_callbacks = new Set();
/**
* Callbacks that should run only when a fork is committed.
* @type {Set<(batch: Batch) => void>}
*/
#fork_commit_callbacks = new Set();
/**
* Async effects that are currently in flight
* @type {Map<Effect, number>}
@ -489,6 +495,7 @@ export class Batch {
discard() {
for (const fn of this.#discard_callbacks) fn(this);
this.#discard_callbacks.clear();
this.#fork_commit_callbacks.clear();
batches.delete(this);
}
@ -686,6 +693,16 @@ export class Batch {
this.#discard_callbacks.add(fn);
}
/** @param {(batch: Batch) => void} fn */
on_fork_commit(fn) {
this.#fork_commit_callbacks.add(fn);
}
run_fork_commit_callbacks() {
for (const fn of this.#fork_commit_callbacks) fn(this);
this.#fork_commit_callbacks.clear();
}
settled() {
return (this.#deferred ??= deferred()).promise;
}
@ -1212,6 +1229,10 @@ export function fork(fn) {
source.wv = increment_write_version();
}
batch.activate();
batch.run_fork_commit_callbacks();
batch.deactivate();
// trigger any `$state.eager(...)` expressions with the new state.
// eager effects don't get scheduled like other effects, so we
// can't just encounter them during traversal, we need to

@ -0,0 +1,33 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
const [show, commit] = target.querySelectorAll('button');
show.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>show</button>
<button>commit</button>
<button>discard</button>
`
);
commit.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>show</button>
<button>commit</button>
<button>discard</button>
failed
`
);
}
});

@ -0,0 +1,18 @@
<script>
import { fork } from 'svelte';
let show = $state(false);
let f;
</script>
<button onclick={() => f = fork(() => show = true)}>show</button>
<button onclick={() => f.commit()}>commit</button>
<button onclick={() => f.discard()}>discard</button>
<svelte:boundary>
{#if show}
{await Promise.reject('boom')}
{/if}
{#snippet failed()}
failed
{/snippet}
</svelte:boundary>
Loading…
Cancel
Save