diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 7434c57e88..80176f3184 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -714,6 +714,12 @@ export class Batch { if (b !== this) this.dependent.add(b); } + for (let b = first_batch; b !== null; b = b.next) { + if (b.dependent.has(batch)) { + b.dependent.add(this); + } + } + for (const c of batch.#commit_callbacks) { this.oncommit(() => c(batch)); } @@ -836,6 +842,10 @@ export class Batch { batch.current.delete(source); if (![...batch.current.values()].some((v) => !v[1])) { + // The real world has overtaken every write of this fork, so it is obsolete. Discard it + // right away (its speculative branches must not be adopted by anyone), and empty + // `current` so that `commit()` can tell this apart from a user-initiated discard + batch.current.clear(); batch.discard(); } else { if (current) batch.current.set(source, current); @@ -1489,6 +1499,15 @@ export function fork(fn) { return; } + if (batch.current.size === 0) { + // Nothing to commit: either the fork never wrote anything (e.g. it assigned a value + // that was already current), or the real world has since written to every source + // it did write to and the fork was discarded as obsolete (see `notify_fork`) + committed = true; + batch.discard(); + return; + } + if (!batch.linked) { e.fork_discarded(); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 72a2621b05..9bb1e4e2e4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -602,17 +602,10 @@ export function get(signal) { // rather than updating `new_deps`, which creates GC cost if (new_deps === null && deps !== null && deps[skipped_deps] === signal) { skipped_deps++; + } else if (new_deps === null) { + new_deps = [signal]; } else { - if (new_deps === null) { - new_deps = [signal]; - } else { - new_deps.push(signal); - } - - // Only a signal that wasn't a dependency of this reaction before counts as new — - // reading existing dependencies in a different order must not (it would make - // the reaction see the latest value instead of its batch's view, see below) - first_time = deps === null || !includes.call(deps, signal); + new_deps.push(signal); } } } else { @@ -793,18 +786,21 @@ export function get(signal) { } } - // A reaction that reads a signal for the first time must see the latest value, rather than - // this batch's view, if that view could hide the write of an _earlier_ batch — the user's - // program made that write before this batch's writes, so hiding it could e.g. crash a newly - // created branch (see `async-state-read-new-dependency`). Earlier batches' writes are hidden - // only while flushing a committing batch (`previous_batch` is set, see `apply(true)`) and in - // eager batches (which hide every other batch). Everywhere else `batch_values` only hides - // _later_ batches' writes, which is correct even for new readers: that's the state the - // program was in when this batch's writes happened. - var see_latest = first_time && (previous_batch !== null || current_batch?.is_eager); - - if (!see_latest && batch_values?.has(signal)) { - return batch_values.get(signal); + if (batch_values?.has(signal)) { + // A reaction that reads a signal for the first time while flushing render effects or + // during an eager batch needs to show the latest value, because maybe it would crash + // with the old version (see test `async-state-read-new-dependency` and its variants). + var see_latest = + (previous_batch !== null || current_batch?.is_eager) && + (first_time || + (active_reaction !== null && + !untracking && + (active_reaction.f & REACTION_IS_UPDATING) !== 0 && + (active_reaction.deps === null || !includes.call(active_reaction.deps, signal)))); + + if (!see_latest) { + return batch_values.get(signal); + } } if ((signal.f & ERROR_VALUE) !== 0) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/_config.js new file mode 100644 index 0000000000..9e17498d26 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/_config.js @@ -0,0 +1,22 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ``; + +// If a fork is auto-discarded it should not throw on user-commit. +export default test({ + mode: ['client'], + async test({ assert, target }) { + const [preload, other, commit] = target.querySelectorAll('button'); + + preload.click(); + await tick(); + other.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}
true 1
`); + + commit.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}true 1
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/main.svelte new file mode 100644 index 0000000000..b4e35f29a7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-empty/main.svelte @@ -0,0 +1,14 @@ + + + + + + +{open} {other} {error}
diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/_config.js new file mode 100644 index 0000000000..0ccca0c573 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/_config.js @@ -0,0 +1,42 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + +`; + +// A fork writes `x`, then the real world writes `x` as well (with async work still pending). +// The fork's write is overtaken and it has nothing left to commit — `commit()` must resolve +// rather than throw `fork_discarded`, and the real world's value wins +export default test({ + mode: ['client'], + async test({ assert, target }) { + await tick(); + const [fork, x5, commit, shift] = target.querySelectorAll('button'); + + fork.click(); // speculative: delay(10) + await tick(); + x5.click(); // real: delay(5); the fork adopts x = 5 and re-runs: delay(5) + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}0
`); + + commit.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}0
`); + + shift.click(); // the fork's obsolete delay(10) + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}0
`); + + shift.click(); // the real delay(5) + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}5
`); + + shift.click(); // the fork's delay(5), rejected when the fork was cleaned up + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}5
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/main.svelte new file mode 100644 index 0000000000..c6c6ba9c1b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork-commit-overtaken/main.svelte @@ -0,0 +1,20 @@ + + + + + + + +{await delay(x)} {error}
diff --git a/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/_config.js b/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/_config.js new file mode 100644 index 0000000000..59c394c468 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/_config.js @@ -0,0 +1,49 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + + + + + +`; + +// Test ensure dependencies on earlier batches are also merged correctly +export default test({ + async test({ assert, target }) { + const [A0, A, B, resolve_q, resolve_p, resolve_t, resolve_r, init] = + target.querySelectorAll('button'); + + init.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}q0 r0 p0 t0 s0
`); + + A0.click(); // q=1, r=1 -> slow(q=1), slow(r=1) + await tick(); + A.click(); // q=2, p=1, s=1 -> slow(q=2), slow(p=1); depends on A0 + await tick(); + B.click(); // s=2, t=1 -> slow(t=1); depends on A + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}q0 r0 p0 t0 s0
`); + + // A's runs resolve -> A merges into A0, which is still waiting on slow(r=1) + resolve_q.click(); + resolve_p.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}q0 r0 p0 t0 s0
`); + + // B's run resolves -> B must wait for A0 + resolve_t.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}q0 r0 p0 t0 s0
`); + + // A0's run resolves -> everything commits at once + resolve_r.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}q2 r1 p1 t1 s2
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/main.svelte new file mode 100644 index 0000000000..6407c69583 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-merge-dependent-of-merged/main.svelte @@ -0,0 +1,27 @@ + + + + + + + + + + + +q{await slow('q', q)} r{await slow('r', r)} p{await slow('p', p)} t{await slow('t', t)} s{s}
+1
+2
` ); @@ -34,7 +34,7 @@ export default test({ -1
+2
` ); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-read-new-dependency/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-state-read-new-dependency/main.svelte index 80e54f9d53..74dcbb1110 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-read-new-dependency/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-state-read-new-dependency/main.svelte @@ -14,4 +14,5 @@ {await wait(value)} -{show ? value.x : ''}
\ No newline at end of file + +{show ? value.x + value.x : ''}
\ No newline at end of file