diff --git a/.changeset/fluffy-bars-sleep.md b/.changeset/fluffy-bars-sleep.md new file mode 100644 index 0000000000..d4611ddfdf --- /dev/null +++ b/.changeset/fluffy-bars-sleep.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent stale reads from new dependencies during pending async work diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 75ce33b1c5..c46b73dc78 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -176,6 +176,14 @@ export class Batch { */ #new_effects = []; + /** + * Values that were first read by a reaction while this batch was time travelling + * over an earlier batch that changed them. These reads connect the batches even + * though this batch did not write to the values itself. + * @type {Set} + */ + #new_dependencies = new Set(); + /** * Deferred effects (which run after async work has completed) that are DIRTY * @type {Set} @@ -540,6 +548,12 @@ export class Batch { while (batch !== null) { if (!batch.is_fork) { + for (const value of this.#new_dependencies) { + if (batch.current.has(value)) { + return batch; + } + } + // if the batches are connected, break for (const [value, [, is_derived]] of this.current) { if (batch.current.has(value) && !is_derived) { @@ -558,6 +572,10 @@ export class Batch { * @param {Batch} batch */ #merge(batch) { + for (const value of batch.#new_dependencies) { + this.#new_dependencies.add(value); + } + for (const [source, value] of batch.current) { if (!this.previous.has(source) && batch.previous.has(source)) { this.previous.set(source, batch.previous.get(source)); @@ -656,6 +674,28 @@ export class Batch { } } + /** + * If a reaction discovers a dependency that was changed by an earlier pending + * batch, connect the two batches and expose the current value while traversing. + * The current batch will be merged into the earlier one before it can commit. + * @param {Value} value + */ + capture_dependency(value) { + if (this.is_fork || this.current.has(value)) return; + + var batch = this.#prev; + + while (batch !== null) { + if (!batch.is_fork && batch.current.has(value)) { + this.#new_dependencies.add(value); + batch_values?.set(value, /** @type {[any, boolean]} */ (batch.current.get(value))[0]); + return; + } + + batch = batch.#prev; + } + } + activate() { current_batch = this; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 27def05300..f95d9f3f07 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -548,6 +548,14 @@ export function settled() { export function get(signal) { var flags = signal.f; var is_derived = (flags & DERIVED) !== 0; + var is_new_dependency = + batch_values !== null && + !untracking && + ((active_reaction !== null && + (active_reaction.deps === null || !includes.call(active_reaction.deps, signal))) || + (active_reaction === null && + active_effect !== null && + (active_effect.f & REACTION_RAN) === 0)); captured_signals?.add(signal); @@ -707,6 +715,10 @@ export function get(signal) { } if (batch_values?.has(signal)) { + if (is_new_dependency) { + (current_batch ?? previous_batch)?.capture_dependency(signal); + } + return batch_values.get(signal); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/_config.js b/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/_config.js new file mode 100644 index 0000000000..2f6c9a96e0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/_config.js @@ -0,0 +1,29 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['client'], + async test({ assert, target }) { + await tick(); + const [update, show, resolve] = target.querySelectorAll('button'); + + update.click(); + await tick(); + + show.click(); + await tick(); + + resolve.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + +

1

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/main.svelte new file mode 100644 index 0000000000..43e663e1b2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-read-new-dependency/main.svelte @@ -0,0 +1,17 @@ + + + + + + +{await wait(value)} + +

{show ? value.x : ''}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js index dee8af2446..28ce3c9d4f 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js @@ -16,20 +16,12 @@ export default test({ - world - ` // if this does not show world - that would also be ok + ` ); resolve.click(); await tick(); - assert.deepEqual(logs, [ - 'universe', - 'world', - '$effect: world', - '$effect: universe', - '$effect: universe' - ]); - // assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); // this would also be ok + assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); assert.htmlEqual( target.innerHTML, ` diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js index d99f0df731..00b38262e8 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js @@ -17,13 +17,7 @@ export default test({
- world - "world" - world - world - world - "world" - ` // if this does not show world "world" world world world "world" - then this would also be ok + ` ); resolve.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js index eb4485e8a6..aa9e274887 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js @@ -29,13 +29,7 @@ export default test({
- world - "world" - world - world - world - "world" - ` // if this does not show world "world" world world world "world" - then this would also be ok + ` ); resolve.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js index f4b6cc777b..601fbdeabe 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js @@ -11,26 +11,19 @@ export default test({ y.click(); await tick(); - assert.deepEqual(logs, ['universe', 'world', '$effect: world']); + assert.deepEqual(logs, ['universe', 'universe']); assert.htmlEqual( target.innerHTML, ` - world ` ); resolve.click(); await tick(); - assert.deepEqual(logs, [ - 'universe', - 'world', - '$effect: world', - '$effect: universe', - '$effect: universe' - ]); + assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); assert.htmlEqual( target.innerHTML, `