diff --git a/.changeset/spicy-teeth-tan.md b/.changeset/spicy-teeth-tan.md new file mode 100644 index 0000000000..c7efa65130 --- /dev/null +++ b/.changeset/spicy-teeth-tan.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: defer batch resolution until earlier intersecting batches have committed diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 9b24400840..d713385c75 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -307,15 +307,16 @@ export function wait(blockers) { * @returns {(skip?: boolean) => void} */ export function increment_pending() { - var boundary = /** @type {Boundary} */ (/** @type {Effect} */ (active_effect).b); + var effect = /** @type {Effect} */ (active_effect); + var boundary = /** @type {Boundary} */ (effect.b); var batch = /** @type {Batch} */ (current_batch); var blocking = boundary.is_rendered(); boundary.update_pending_count(1, batch); - batch.increment(blocking); + batch.increment(blocking, effect); return (skip = false) => { boundary.update_pending_count(-1, batch); - batch.decrement(blocking, skip); + batch.decrement(blocking, effect, skip); }; } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index a850bbc6da..ec6a34c39a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -110,20 +110,20 @@ var source_stacks = DEV ? new Set() : null; let uid = 1; export class Batch { - // for debugging. TODO remove once async is stable id = uid++; /** - * The current values of any sources that are updated in this batch + * The current values of any signals that are updated in this batch. + * Tuple format: [value, is_derived] (note: is_derived is false for deriveds, too, if they were overridden via assignment) * They keys of this map are identical to `this.#previous` - * @type {Map} + * @type {Map} */ current = new Map(); /** - * The values of any sources that are updated in this batch _before_ those updates took place. + * The values of any signals (sources and deriveds) that are updated in this batch _before_ those updates took place. * They keys of this map are identical to `this.#current` - * @type {Map} + * @type {Map} */ previous = new Map(); @@ -141,14 +141,16 @@ export class Batch { #discard_callbacks = new Set(); /** - * The number of async effects that are currently in flight + * Async effects that are currently in flight + * @type {Map} */ - #pending = 0; + #pending = new Map(); /** - * The number of async effects that are currently in flight, _not_ inside a pending boundary + * Async effects that are currently in flight, _not_ inside a pending boundary + * @type {Map} */ - #blocking_pending = 0; + #blocking_pending = new Map(); /** * A deferred that resolves when the batch is committed, used with `settled()` @@ -188,8 +190,35 @@ export class Batch { #decrement_queued = false; + /** @type {Set} */ + #blockers = new Set(); + #is_deferred() { - return this.is_fork || this.#blocking_pending > 0; + return this.is_fork || this.#blocking_pending.size > 0; + } + + #is_blocked() { + for (const batch of this.#blockers) { + for (const effect of batch.#blocking_pending.keys()) { + var skipped = false; + var e = effect; + + while (e.parent !== null) { + if (this.#skipped_branches.has(e)) { + skipped = true; + break; + } + + e = e.parent; + } + + if (!skipped) { + return true; + } + } + } + + return false; } /** @@ -285,7 +314,7 @@ export class Batch { collected_effects = null; legacy_updates = null; - if (this.#is_deferred()) { + if (this.#is_deferred() || this.#is_blocked()) { this.#defer_effects(render_effects); this.#defer_effects(effects); @@ -293,7 +322,7 @@ export class Batch { reset_branch(e, t); } } else { - if (this.#pending === 0) { + if (this.#pending.size === 0) { batches.delete(this); } @@ -412,17 +441,18 @@ export class Batch { /** * Associate a change to a given source with the current * batch, noting its previous and current values - * @param {Source} source + * @param {Value} source * @param {any} old_value + * @param {boolean} [is_derived] */ - capture(source, old_value) { + capture(source, old_value, is_derived = false) { if (old_value !== UNINITIALIZED && !this.previous.has(source)) { this.previous.set(source, old_value); } // Don't save errors in `batch_values`, or they won't be thrown in `runtime.js#get` if ((source.f & ERROR_VALUE) === 0) { - this.current.set(source, source.v); + this.current.set(source, [source.v, is_derived]); batch_values?.set(source, source.v); } } @@ -471,22 +501,126 @@ export class Batch { batches.delete(this); } +<<<<<<< async-derived-coordinate-batches +======= + #commit() { + // If there are other pending batches, they now need to be 'rebased' — + // in other words, we re-run block/async effects with the newly + // committed state, unless the batch in question has a more + // recent value for a given source + for (const batch of batches) { + var is_earlier = batch.id < this.id; + + /** @type {Source[]} */ + var sources = []; + + for (const [source, [value, is_derived]] of this.current) { + if (batch.current.has(source)) { + var batch_value = /** @type {[any, boolean]} */ (batch.current.get(source))[0]; // faster than destructuring + + if (is_earlier && value !== batch_value) { + // bring the value up to date + batch.current.set(source, [value, is_derived]); + } else { + // same value or later batch has more recent value, + // no need to re-run these effects + continue; + } + } + + sources.push(source); + } + + // Re-run async/block effects that depend on distinct values changed in both batches + var others = [...batch.current.keys()].filter((s) => !this.current.has(s)); + + if (others.length === 0) { + if (is_earlier) { + // this batch is now obsolete and can be discarded + batch.discard(); + } + } else if (sources.length > 0) { + if (DEV) { + invariant(batch.#roots.length === 0, 'Batch has scheduled roots'); + } + + batch.activate(); + + /** @type {Set} */ + var marked = new Set(); + + /** @type {Map} */ + var checked = new Map(); + + for (var source of sources) { + mark_effects(source, others, marked, checked); + } + + // Only apply and traverse when we know we triggered async work with marking the effects + if (batch.#roots.length > 0) { + batch.apply(); + + for (var root of batch.#roots) { + batch.#traverse(root, [], []); + } + + batch.#roots = []; + } + + batch.deactivate(); + } + } + + for (const batch of batches) { + if (batch.#blockers.has(this)) { + batch.#blockers.delete(this); + + if (batch.#blockers.size === 0 && !batch.#is_deferred()) { + batch.activate(); + batch.#process(); + } + } + } + } + +>>>>>>> main /** - * * @param {boolean} blocking + * @param {Effect} effect */ - increment(blocking) { - this.#pending += 1; - if (blocking) this.#blocking_pending += 1; + increment(blocking, effect) { + let pending_count = this.#pending.get(effect) ?? 0; + this.#pending.set(effect, pending_count + 1); + + if (blocking) { + let blocking_pending_count = this.#blocking_pending.get(effect) ?? 0; + this.#blocking_pending.set(effect, blocking_pending_count + 1); + } } /** * @param {boolean} blocking + * @param {Effect} effect * @param {boolean} skip - whether to skip updates (because this is triggered by a stale reaction) */ - decrement(blocking, skip) { - this.#pending -= 1; - if (blocking) this.#blocking_pending -= 1; + decrement(blocking, effect, skip) { + let pending_count = this.#pending.get(effect) ?? 0; + + if (pending_count === 1) { + this.#pending.delete(effect); + } else { + this.#pending.set(effect, pending_count - 1); + } + + if (blocking) { + let blocking_pending_count = this.#blocking_pending.get(effect) ?? 0; + + if (blocking_pending_count === 1) { + this.#blocking_pending.delete(effect); + } else { + this.#blocking_pending.set(effect, blocking_pending_count - 1); + } + } if (this.#decrement_queued || skip) return; this.#decrement_queued = true; @@ -559,15 +693,37 @@ export class Batch { // if there are multiple batches, we are 'time travelling' — // we need to override values with the ones in this batch... - batch_values = new Map(this.current); + batch_values = new Map(); + for (const [source, [value]] of this.current) { + batch_values.set(source, value); + } - // ...and undo changes belonging to other batches + // ...and undo changes belonging to other batches unless they block this one for (const batch of batches) { if (batch === this || batch.is_fork) continue; - for (const [source, previous] of batch.previous) { - if (!batch_values.has(source)) { - batch_values.set(source, previous); + // A batch is blocked on an earlier batch if it overlaps with the earlier batch's changes but is not a superset + var intersects = false; + var differs = false; + + if (batch.id < this.id) { + for (const [source, [, is_derived]] of batch.current) { + // Derived values don't partake in the blocking mechanism, because a derived could + // be triggered in one batch already but not the other one yet, causing a false-positive + if (is_derived) continue; + + intersects ||= this.current.has(source); + differs ||= !this.current.has(source); + } + } + + if (intersects && differs) { + this.#blockers.add(batch); + } else { + for (const [source, previous] of batch.previous) { + if (!batch_values.has(source)) { + batch_values.set(source, previous); + } } } } @@ -969,7 +1125,7 @@ export function fork(fn) { batch.is_fork = false; // apply changes and update write versions so deriveds see the change - for (var [source, value] of batch.current) { + for (var [source, [value]] of batch.current) { source.v = value; source.wv = increment_write_version(); } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index c05e5b768b..b5e8464d70 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -464,7 +464,7 @@ export function update_derived(derived) { // change, `derived.equals` may incorrectly return `true` if (!current_batch?.is_fork || derived.deps === null) { derived.v = value; - current_batch?.capture(derived, old_value); + current_batch?.capture(derived, old_value, true); // deriveds without dependencies should never be recomputed if (derived.deps === null) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-each-overlap/_config.js b/packages/svelte/tests/runtime-runes/samples/async-each-overlap/_config.js deleted file mode 100644 index d03f9ad09d..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/async-each-overlap/_config.js +++ /dev/null @@ -1,102 +0,0 @@ -import { tick } from 'svelte'; -import { test } from '../../test'; - -export default test({ - skip: true, - async test({ assert, target }) { - const [add, shift] = target.querySelectorAll('button'); - - add.click(); - await tick(); - add.click(); - await tick(); - add.click(); - await tick(); - - // TODO pending count / number of pushes is off - - assert.htmlEqual( - target.innerHTML, - ` - - -

pending=6 values.length=1 values=[1]

-
not keyed: -
1
-
-
keyed: -
1
-
- ` - ); - - shift.click(); - await tick(); - shift.click(); - await tick(); - assert.htmlEqual( - target.innerHTML, - ` - - -

pending=4 values.length=2 values=[1,2]

-
not keyed: -
1
-
2
-
-
keyed: -
1
-
2
-
- ` - ); - - shift.click(); - await tick(); - shift.click(); - await tick(); - assert.htmlEqual( - target.innerHTML, - ` - - -

pending=2 values.length=3 values=[1,2,3]

-
not keyed: -
1
-
2
-
3
-
-
keyed: -
1
-
2
-
3
-
- ` - ); - - shift.click(); - await tick(); - shift.click(); - await tick(); - assert.htmlEqual( - target.innerHTML, - ` - - -

pending=0 values.length=4 values=[1,2,3,4]

-
not keyed: -
1
-
2
-
3
-
4
-
-
keyed: -
1
-
2
-
3
-
4
-
- ` - ); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-overlapping-array/_config.js b/packages/svelte/tests/runtime-runes/samples/async-overlapping-array/_config.js new file mode 100644 index 0000000000..2dad5babea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-overlapping-array/_config.js @@ -0,0 +1,205 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + + const [add, shift, pop] = target.querySelectorAll('button'); + + add.click(); + await tick(); + add.click(); + await tick(); + add.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=6 values.length=1 values=[1]

+
not keyed: +
1
+
+
keyed: +
1
+
+ ` + ); + + shift.click(); + await tick(); + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=4 values.length=2 values=[1,2]

+
not keyed: +
1
+
2
+
+
keyed: +
1
+
2
+
+ ` + ); + + shift.click(); + await tick(); + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=2 values.length=3 values=[1,2,3]

+
not keyed: +
1
+
2
+
3
+
+
keyed: +
1
+
2
+
3
+
+ ` + ); + + shift.click(); + await tick(); + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=0 values.length=4 values=[1,2,3,4]

+
not keyed: +
1
+
2
+
3
+
4
+
+
keyed: +
1
+
2
+
3
+
4
+
+ ` + ); + + add.click(); + await tick(); + add.click(); + await tick(); + add.click(); + await tick(); + add.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=8 values.length=4 values=[1,2,3,4]

+
not keyed: +
1
+
2
+
3
+
4
+
+
keyed: +
1
+
2
+
3
+
4
+
+ ` + ); + + // pop should have no effect until earlier promises have also resolved + pop.click(); + await tick(); + pop.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=6 values.length=4 values=[1,2,3,4]

+
not keyed: +
1
+
2
+
3
+
4
+
+
keyed: +
1
+
2
+
3
+
4
+
+ ` + ); + + pop.click(); + await tick(); + pop.click(); + await tick(); + pop.click(); + await tick(); + pop.click(); + await tick(); + pop.click(); + await tick(); + pop.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + + +

pending=0 values.length=8 values=[1,2,3,4,5,6,7,8]

+
not keyed: +
1
+
2
+
3
+
4
+
5
+
6
+
7
+
8
+
+
keyed: +
1
+
2
+
3
+
4
+
5
+
6
+
7
+
8
+
+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-each-overlap/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-overlapping-array/main.svelte similarity index 84% rename from packages/svelte/tests/runtime-runes/samples/async-each-overlap/main.svelte rename to packages/svelte/tests/runtime-runes/samples/async-overlapping-array/main.svelte index af9d395457..e41b6926b8 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-each-overlap/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-overlapping-array/main.svelte @@ -11,18 +11,14 @@ return p.promise; } - function shift() { - const fn = queue.shift(); - if (fn) fn(); - } - function addValue() { values.push(values.length+1); } - + +

pending={$effect.pending()}