diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 1526e0c464..e7aaa4c2c6 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -263,6 +263,13 @@ export class Batch { */ fork_values = null; + /** + * Async and block effects that ran or were proven clean inside this fork. + * When the fork commits, only these effects can retain their speculative results + * @type {Set | null} + */ + fork_effects = null; + /** * Reactions that observed the pre-write world of this batch (via * `batch_values`) while it was pending. When this batch commits, they @@ -519,6 +526,13 @@ export class Batch { return false; } + /** @param {Effect} effect */ + record_fork_effect(effect) { + if (this.is_fork && (effect.f & (ASYNC | BLOCK_EFFECT)) !== 0) { + (this.fork_effects ??= new Set()).add(effect); + } + } + /** * Absorb another batch into this one — their reactivity graphs overlap, * so they describe a single 'world'. All state is transferred, and stale @@ -898,11 +912,17 @@ export class Batch { effects.push(effect); } else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) { render_effects.push(effect); - } else if (is_dirty(effect)) { - if ((flags & BLOCK_EFFECT) !== 0) { - (this.#maybe_dirty_effects ??= new Set()).add(effect); + } else { + var dirty = is_dirty(effect); + + if (dirty) { + if ((flags & BLOCK_EFFECT) !== 0) { + (this.#maybe_dirty_effects ??= new Set()).add(effect); + } + update_effect(effect); + } else if ((flags & MAYBE_DIRTY) !== 0) { + this.record_fork_effect(effect); } - update_effect(effect); } var child = effect.first; @@ -1190,6 +1210,7 @@ export class Batch { */ schedule(effect) { last_scheduled_effect = effect; + if (this.is_fork) this.fork_effects?.delete(effect); if (this.claim(effect)) { this.#dirty_effects ??= new Set(); @@ -1488,17 +1509,18 @@ export function eager(fn) { /** * When a fork is committed, deriveds affected by its writes must recompute with - * the now-committed values, and template/user effects must re-run. Async and - * block effects are skipped — they already ran inside the fork with these - * values (their results were captured by the fork's batch), and eager effects - * are collected so that `$state.eager(...)` expressions update immediately + * the now-committed values, and affected effects must be checked. Async and + * block effects that were validated inside the fork are skipped, and eager + * effects are collected so that `$state.eager(...)` expressions update immediately * @param {Value} value * @param {Batch} batch - * @param {Set} marked + * @param {Map} marked + * @param {number} status */ -function mark_committed_reactions(value, batch, marked) { - if (marked.has(value)) return; - marked.add(value); +function mark_committed_reactions(value, batch, marked, status) { + var previous_status = marked.get(value); + if (previous_status === DIRTY || previous_status === status) return; + marked.set(value, status); var reactions = value.reactions; if (reactions === null) return; @@ -1507,34 +1529,39 @@ function mark_committed_reactions(value, batch, marked) { var flags = reaction.f; if ((flags & EAGER_EFFECT) !== 0) { - set_signal_status(reaction, DIRTY); + if ((flags & DIRTY) === 0) { + set_signal_status(reaction, status); + } eager_effects.add(/** @type {Effect} */ (reaction)); } else if ((flags & DERIVED) !== 0) { batch.claim(reaction); if ((flags & DIRTY) === 0) { - set_signal_status(reaction, MAYBE_DIRTY); + set_signal_status(reaction, status); } - mark_committed_reactions(/** @type {Derived} */ (reaction), batch, marked); + mark_committed_reactions(/** @type {Derived} */ (reaction), batch, marked, MAYBE_DIRTY); } else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0) { - // async and block effects that the fork itself ran are left alone — - // they already ran with these exact values. But if such an effect - // belongs to another live batch's world, it must re-run with the - // committed values (which also entangles us with that batch, as - // with any other write) + // async and block effects validated inside the fork are left alone — + // they already ran with these exact values or were proven unaffected. + // But if such an effect belongs to another live batch's world, it must + // re-run with the committed values (which also entangles us with that + // batch, as with any other write) var owner = /** @type {Effect} */ (reaction).batch; while (owner !== null && owner.merged_into !== null) owner = owner.merged_into; - if (owner !== null && owner !== batch && owner.linked && !owner.is_fork) { + if ( + !batch.fork_effects?.has(/** @type {Effect} */ (reaction)) || + (owner !== null && owner !== batch && owner.linked && !owner.is_fork) + ) { if ((reaction.f & DIRTY) === 0) { - set_signal_status(reaction, DIRTY); + set_signal_status(reaction, status); } batch.schedule(/** @type {Effect} */ (reaction)); } } else if ((flags & DIRTY) === 0) { - set_signal_status(reaction, DIRTY); + set_signal_status(reaction, status); batch.schedule(/** @type {Effect} */ (reaction)); } } @@ -1643,13 +1670,13 @@ export function fork(fn) { // entangles us with any overlapping pending batches batch.activate(); - /** @type {Set} */ - var marked = new Set(); + /** @type {Map} */ + var marked = new Map(); for (var [source, value] of batch.current) { source.v = value; source.wv = increment_write_version(); - mark_committed_reactions(source, batch, marked); + mark_committed_reactions(source, batch, marked, DIRTY); } batch.deactivate(); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 37419044bd..fd7870e6d4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -454,6 +454,7 @@ export function remove_reactions(signal, start_index) { */ export function update_effect(effect) { var flags = effect.f; + var batch = current_batch; if ((flags & DESTROYED) !== 0) { return; @@ -486,6 +487,7 @@ export function update_effect(effect) { var teardown = update_reaction(effect); effect.teardown = typeof teardown === 'function' ? teardown : null; effect.wv = write_version; + batch?.record_fork_effect(effect); // In DEV, increment versions of any sources that were written to during the effect, // so that they are correctly marked as dirty when the effect re-runs diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/_config.js new file mode 100644 index 0000000000..7fb39c8a84 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/_config.js @@ -0,0 +1,18 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [change, commit] = target.querySelectorAll('button'); + + assert.deepEqual(logs, ['hi']); + + change.click(); + await tick(); + assert.deepEqual(logs, ['hi', 'hi']); + + commit.click(); + await tick(); + assert.deepEqual(logs, ['hi', 'hi']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/main.svelte new file mode 100644 index 0000000000..810e09097b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/main.svelte @@ -0,0 +1,12 @@ + + + + + +{#if y}hi{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-4/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-4/_config.js index 6a6399a19e..a20f8c0ba5 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-4/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-4/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target }) { const [x, y, resolve, commit] = target.querySelectorAll('button');