From 2cfc5819d0a190056f4a559c8fe2d7ebf679cc7e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 18 Sep 2026 23:02:31 +0200 Subject: [PATCH] fucking hell forks are hard --- .../svelte/src/internal/client/constants.js | 2 + .../internal/client/dom/blocks/branches.js | 21 ++++- .../src/internal/client/dom/operations.js | 11 ++- .../src/internal/client/reactivity/batch.js | 58 +++++++++++--- .../internal/client/reactivity/deriveds.js | 1 - .../src/internal/client/reactivity/sources.js | 1 - .../async-derived-not-overfiring/_config.js | 1 - .../async-derived-not-underfiring/_config.js | 1 - .../async-fork-async-effect-replay/_config.js | 6 +- .../async-fork-branch-update/_config.js | 45 ++++++----- .../_config.js | 32 ++++++++ .../main.svelte | 22 ++++++ .../async-fork-nested-branch/Child.svelte | 8 ++ .../async-fork-nested-branch/_config.js | 79 +++++++++++++++++++ .../async-fork-nested-branch/main.svelte | 36 +++++++++ .../Child.svelte | 8 ++ .../async-fork-uninitialized-debug/_config.js | 50 ++++++++++++ .../main.svelte | 20 +++++ 18 files changed, 361 insertions(+), 41 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/main.svelte diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index b086bfedff..586dd30b93 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -54,6 +54,8 @@ export const EFFECT_OFFSCREEN = 1 << 25; // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; export const ASYNC = 1 << 22; +/** Set on branch effects that only exist for fork batches */ +export const FORK_ONLY_BRANCH = 1 << 23; export const ERROR_VALUE = 1 << 23; diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index 7446c68c99..ed55b39974 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -7,7 +7,7 @@ import { pause_effect, resume_effect } from '../../reactivity/effects.js'; -import { HMR_ANCHOR } from '../../constants.js'; +import { FORK_ONLY_BRANCH, HMR_ANCHOR } from '../../constants.js'; import { hydrate_node, hydrating } from '../hydration.js'; import { create_text, should_defer_append } from '../operations.js'; import { DEV } from 'esm-env'; @@ -188,18 +188,26 @@ export class BranchManager { ensure(key, fn) { var batch = /** @type {Batch} */ (current_batch); var defer = should_defer_append(); + var first = false; if (fn && !this.#onscreen.has(key) && !this.#offscreen.has(key)) { + first = true; + if (defer) { var fragment = document.createDocumentFragment(); var target = create_text(); fragment.append(target); + const b = branch(() => fn(target)); this.#offscreen.set(key, { - effect: branch(() => fn(target)), + effect: b, fragment }); + + if (batch.is_fork) { + b.f ^= FORK_ONLY_BRANCH; + } } else { this.#onscreen.set( key, @@ -210,6 +218,15 @@ export class BranchManager { this.#batches.set(batch, key); + const offscreen = this.#offscreen.get(key); + if (offscreen && offscreen.effect.f & FORK_ONLY_BRANCH) { + if (batch.is_fork) { + batch.unskip_effect(offscreen.effect, undefined, !first); + } else { + offscreen.effect.f ^= FORK_ONLY_BRANCH; + } + } + if (defer) { for (const [k, effect] of this.#onscreen) { if (k === key) { diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 21b5ee4ecb..05b3fa4f8a 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -3,7 +3,7 @@ import { hydrate_node, hydrating, reset, set_hydrate_node } from './hydration.js import { DEV } from 'esm-env'; import { init_array_prototype_warnings } from '../dev/equality.js'; import { get_descriptor, is_extensible } from '../../shared/utils.js'; -import { active_effect } from '../runtime.js'; +import { active_effect, new_deps, skipped_deps } from '../runtime.js'; import { async_mode_flag } from '../../flags/index.js'; import { ATTRIBUTES_CACHE, @@ -13,7 +13,7 @@ import { TEXT_CACHE, TEXT_NODE } from '#client/constants'; -import { eager_block_effects } from '../reactivity/batch.js'; +import { current_batch, eager_block_effects } from '../reactivity/batch.js'; import { NAMESPACE_HTML } from '../../../constants.js'; // export these for reference in the compiled code, making global name deduplication unnecessary @@ -249,7 +249,12 @@ export function should_defer_append() { if (eager_block_effects !== null) return false; var flags = /** @type {Effect} */ (active_effect).f; - return (flags & REACTION_RAN) !== 0; + var ran = (flags & REACTION_RAN) !== 0; + if (ran || !current_batch?.is_fork) return ran; + // In a fork we generally want to defer the append, unless this is the first run + // and that run is terminal, i.e. there are no deps so the e.g. if block can never + // rerun, which means it can never end up in commit callbacks for other batches. + return new_deps !== null || skipped_deps !== 0; } /** diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 1cae37307f..f71f80b1a0 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -17,7 +17,8 @@ import { ERROR_VALUE, MANAGED_EFFECT, REACTION_RAN, - DESTROYING + DESTROYING, + FORK_ONLY_BRANCH } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property, includes } from '../../shared/utils.js'; @@ -246,9 +247,10 @@ export class Batch { /** * Inverse of #skipped_branches which we need to tell prior batches to unskip them when committing - * @type {Set} + * true indicates that this branch is new to the eyes of this fork but was already created before. + * @type {Map} */ - #unskipped_branches = new Set(); + #unskipped_branches = new Map(); is_fork = false; @@ -284,10 +286,9 @@ export class Batch { } else { last_batch.next = this; this.prev = last_batch; + last_batch = this; } } - - last_batch = this; } #is_deferred() { @@ -330,8 +331,9 @@ export class Batch { * any tracked dirty/maybe_dirty child effects * @param {Effect} effect * @param {(e: Effect) => void} callback + * @param {boolean} is_fork_init */ - unskip_effect(effect, callback = (e) => this.schedule(e)) { + unskip_effect(effect, callback = (e) => this.schedule(e), is_fork_init = false) { var tracked = this.#skipped_branches.get(effect); if (tracked) { this.#skipped_branches.delete(effect); @@ -346,7 +348,7 @@ export class Batch { callback(e); } } - this.#unskipped_branches.add(effect); + if (!this.#unskipped_branches.has(effect)) this.#unskipped_branches.set(effect, is_fork_init); } /** @@ -434,6 +436,14 @@ export class Batch { set_signal_status(d, MAYBE_DIRTY); } + if (!this.is_fork) { + for (const e of this.#unskipped_branches.keys()) { + if (e.f & FORK_ONLY_BRANCH) { + e.f ^= FORK_ONLY_BRANCH; + } + } + } + // An earlier batch might have created new branches which contain effects that we need // to mark as dirty to also execute them. // TODO does this make the similar logic in fork.commit below obsolete? @@ -585,14 +595,41 @@ export class Batch { root.f ^= CLEAN; var effect = root.first; + var all_dirty = null; while (effect !== null) { + if (all_dirty) { + if (effect.f & CLEAN) effect.f ^= CLEAN; + if ((effect.f & DIRTY) === 0) effect.f |= MAYBE_DIRTY; + } + var flags = effect.f; var is_branch = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0; var is_skippable_branch = is_branch && (flags & CLEAN) !== 0; var skip = is_skippable_branch || (flags & INERT) !== 0 || this.#skipped_branches.has(effect); + if ((flags & FORK_ONLY_BRANCH) !== 0) { + var first_time = this.#unskipped_branches.get(effect); + + if (first_time === undefined) { + skip = true; + this.skip_effect(effect); + reset_branch( + effect, + /** @type {{d: Effect[], m: Effect[]}} */ (this.#skipped_branches.get(effect)) + ); + } else if (first_time) { + // We're seeing a fork-only branch for the first time in another fork. We need to traverse + // all effects inside it (they're all marked MAYBE_DIRTY). This is necessary because + // dependencies of the effects inside could've updated since the last time this branch ran. + this.#unskipped_branches.set(effect, false); + all_dirty ??= effect; + if (effect.f & CLEAN) effect.f ^= CLEAN; + skip = false; + } + } + if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; @@ -625,6 +662,8 @@ export class Batch { } effect = effect.parent; + + if (effect === all_dirty) all_dirty = null; } } } @@ -698,7 +737,7 @@ export class Batch { ? !this.seen_effects.has(effect) && !this.#dirty_effects.has(effect) && !this.#maybe_dirty_effects.has(effect) - : flags & (ASYNC | BLOCK_EFFECT) && this.seen_effects.has(effect) + : (flags & (ASYNC | BLOCK_EFFECT)) === 0 || this.seen_effects.has(effect) ) { this.#maybe_dirty_effects.delete(effect); set_signal_status(effect, status); @@ -744,7 +783,7 @@ export class Batch { this.#unskipped_branches.delete(s); } - for (const s of batch.#unskipped_branches) { + for (const s of batch.#unskipped_branches.keys()) { const v = this.#skipped_branches.get(s); // TODO i do wonder at this point if it's less code / easier / more robust to do what mark() below does // instead and just rerun all the block effects. Though it will certainly overrun some blocks, potentially @@ -1882,7 +1921,6 @@ export function fork(fn) { // for (const run of batch.on_fork_commit.values()) { // run(); // } - batch.flush(); await settled; }, diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 56bc0895f9..31c5742e02 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -448,7 +448,6 @@ export function update_derived(derived) { !derived.equals(/** @type {any[]} */ (batch_values?.get(derived))[0]) ) { current_batch?.capture(derived, derived.v); - // TODO also bump wv_values? } // don't mark derived clean if we're reading it inside a diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 5c65f9ba0d..2ba11a84e9 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -278,7 +278,6 @@ export function internal_set(source, value, updated_during_traversal = null) { !source.equals(/** @type {any[]} */ (batch_values?.get(source))[0]) ) { current_batch?.capture(source, source.v); - // TODO also bump wv_values? } return value; diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-not-overfiring/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-not-overfiring/_config.js index b959ba5407..2a26431ce8 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-derived-not-overfiring/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-not-overfiring/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO fix async test({ assert, target, logs }) { await tick(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-not-underfiring/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-not-underfiring/_config.js index 7d31d30b46..2f6fe7279c 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-derived-not-underfiring/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-not-underfiring/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO fix async test({ assert, target, logs }) { await tick(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-async-effect-replay/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-async-effect-replay/_config.js index 1f9a10c5a1..165ac7e2c8 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-fork-async-effect-replay/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-async-effect-replay/_config.js @@ -20,17 +20,17 @@ export default test({ // Transfer an invalidation into the fork while its async work is pending. update.click(); await tick(); - assert.equal(instance.get_calls(), 2); // can also be 1 at this point already, would also be ok + assert.equal(instance.get_calls(), 1); // can also be 2 at this point, would also be ok try { resolve.click(); await tick(); - assert.equal(instance.get_calls(), 2); + assert.equal(instance.get_calls(), 1); // Completing the replacement must not replay the same invalidation. resolve.click(); await tick(); - assert.equal(instance.get_calls(), 2); + assert.equal(instance.get_calls(), 1); } finally { discard.click(); await tick(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-branch-update/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-branch-update/_config.js index 8b50ff2e19..6ab63f7693 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-fork-branch-update/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-branch-update/_config.js @@ -18,28 +18,35 @@ export default test({ target.querySelectorAll('button'); for (const mode of ['commit', 'reveal', 'second-fork']) { - preload.click(); - flushSync(() => increment.click()); - assert.htmlEqual(target.innerHTML, buttons); - - if (mode === 'commit') { - commit.click(); + try { + preload.click(); await tick(); - } else if (mode === 'reveal') { - flushSync(() => reveal.click()); - discard.click(); - } else { - preload_second.click(); - discard.click(); - commit_second.click(); + increment.click(); await tick(); - } + assert.htmlEqual(target.innerHTML, buttons); - assert.htmlEqual(target.innerHTML, `${buttons}

1 2

`); - flushSync(() => increment.click()); - assert.htmlEqual(target.innerHTML, `${buttons}

2 4

`); - flushSync(() => reset.click()); - assert.htmlEqual(target.innerHTML, buttons); + if (mode === 'commit') { + commit.click(); + await tick(); + } else if (mode === 'reveal') { + flushSync(() => reveal.click()); + discard.click(); + } else { + preload_second.click(); + discard.click(); + commit_second.click(); + await tick(); + } + + assert.htmlEqual(target.innerHTML, `${buttons}

1 2

`); + flushSync(() => increment.click()); + assert.htmlEqual(target.innerHTML, `${buttons}

2 4

`); + flushSync(() => reset.click()); + assert.htmlEqual(target.innerHTML, buttons); + } catch (e) { + /** @type {Error} */ (e).message = `${mode}: ${/** @type {Error} */ (e).message}`; + throw e; + } } } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/_config.js new file mode 100644 index 0000000000..1c16112872 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/_config.js @@ -0,0 +1,32 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + +`; + +export default test({ + async test({ assert, target }) { + const [preload, reveal, hide, commit] = target.querySelectorAll('button'); + + preload.click(); + reveal.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `${buttons}0

constant

keyed

boundary

` + ); + + hide.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}0`); + + // The remaining fork write must not resurrect its obsolete branch selection. + commit.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}1`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/main.svelte new file mode 100644 index 0000000000..13247dda7a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch-static/main.svelte @@ -0,0 +1,22 @@ + + + + + + + +{other} + +{#if show} + {#if true}

constant

{/if} + {#key 1}

keyed

{/key} + +

boundary

+ +{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/Child.svelte new file mode 100644 index 0000000000..b131b926f8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/Child.svelte @@ -0,0 +1,8 @@ + + +{#if pending}pending{/if} +

{pages}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/_config.js new file mode 100644 index 0000000000..61eb25eeb8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/_config.js @@ -0,0 +1,79 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + + + + + +`; + +export default test({ + async test({ assert, target, logs }) { + const [preload, reveal, reveal_and_navigate, navigate, resolve, commit, discard, reset] = + target.querySelectorAll('button'); + + for (const mode of ['separate', 'together', 'pending']) { + for (const finish of [commit, discard]) { + try { + preload.click(); + + if (mode !== 'pending') { + resolve.click(); + await tick(); + } + + assert.htmlEqual(target.innerHTML, buttons); + assert.deepEqual(logs, ['load']); + + if (mode === 'together') { + reveal_and_navigate.click(); + } else { + reveal.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}
`); + navigate.click(); + } + + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}
`); + assert.deepEqual(logs, ['load']); + + if (mode === 'pending') { + resolve.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}
`); + } + + finish.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `${buttons}
${finish === commit ? 'pending

2

' : ''}
` + ); + assert.deepEqual(logs, ['load']); + + if (finish === commit) { + navigate.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

2

`); + assert.deepEqual(logs, ['load']); + } + + reset.click(); + await tick(); + assert.htmlEqual(target.innerHTML, buttons); + logs.length = 0; + } catch (e) { + /** @type {Error} */ (e).message = + `${mode}/${finish === commit ? 'commit' : 'discard'}: ${/** @type {Error} */ (e).message}`; + throw e; + } + } + } + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/main.svelte new file mode 100644 index 0000000000..6d24027f05 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-nested-branch/main.svelte @@ -0,0 +1,36 @@ + + + + + + + + + + + +{#if outer} +
+ {#if inner} + console.log(error.message)}> + {#snippet pending()}loading{/snippet} + + + {/if} +
+{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/Child.svelte new file mode 100644 index 0000000000..b131b926f8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/Child.svelte @@ -0,0 +1,8 @@ + + +{#if pending}pending{/if} +

{pages}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/_config.js new file mode 100644 index 0000000000..40a9c5e63d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/_config.js @@ -0,0 +1,50 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + +`; + +export default test({ + async test({ assert, target, logs }) { + const [preload, navigate, commit, discard] = target.querySelectorAll('button'); + + preload.click(); + // Let the async child resolve, without committing its fork. + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.htmlEqual(target.innerHTML, buttons); + assert.deepEqual(logs, []); + + // A real-world update must not evaluate the speculative child in the real world. + navigate.click(); + await tick(); + assert.htmlEqual(target.innerHTML, buttons); + assert.deepEqual(logs, []); + discard.click(); + await tick(); + assert.htmlEqual(target.innerHTML, buttons); + assert.deepEqual(logs, []); + + navigate.click(); + await tick(); + preload.click(); + await new Promise((resolve) => setTimeout(resolve, 0)); + navigate.click(); + await tick(); + assert.htmlEqual(target.innerHTML, buttons); + assert.deepEqual(logs, []); + + commit.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}pending

2

`); + assert.deepEqual(logs, []); + + navigate.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

2

`); + assert.deepEqual(logs, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/main.svelte new file mode 100644 index 0000000000..43e9600df9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-uninitialized-debug/main.svelte @@ -0,0 +1,20 @@ + + + + + + + +{#if show} + console.log(error.message)}> + {#snippet pending()}loading{/snippet} + + +{/if}