From e82d7494f77f39227f32d36de47a908ad912aad9 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 15 Sep 2026 14:54:25 +0200 Subject: [PATCH] wip --- .../src/internal/client/reactivity/batch.js | 108 ++++++++++++++++-- .../internal/client/reactivity/deriveds.js | 37 +++++- .../svelte/src/internal/client/runtime.js | 10 +- .../_config.js | 1 - .../_config.js | 13 ++- .../async-overlap-multiple-5/_config.js | 82 ++++++------- .../async-state-new-branch-1/_config.js | 23 ++-- .../async-state-new-branch-2/_config.js | 8 +- .../async-state-new-branch-3/_config.js | 8 +- .../async-state-new-branch-fork-5/_config.js | 6 - .../samples/async-state-new-branch/_config.js | 15 +-- 11 files changed, 214 insertions(+), 97 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 75ce33b1c5..cf0d82668e 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -42,7 +42,7 @@ import { log_effect_tree } from '../dev/debug.js'; import { OBSOLETE } from './deriveds.js'; /** @type {Batch | null} */ -let first_batch = null; +export let first_batch = null; /** @type {Batch | null} */ let last_batch = null; @@ -64,6 +64,12 @@ export let previous_batch = null; */ export let batch_values = null; +/** + * Sources which were written to before the current batch. Used to discover dependencies between batches. + * @type {Map | null} + */ +export let held_sources = null; + /** @type {Effect | null} */ let last_scheduled_effect = null; @@ -100,6 +106,15 @@ export class Batch { /** True as soon as `#process` was called */ #started = false; + // TODO temporary + get next() { + return this.#next; + } + // TODO temporary + get prev() { + return this.#prev; + } + linked = true; /** @type {Batch | null} */ @@ -108,6 +123,9 @@ export class Batch { /** @type {Batch | null} */ #next = null; + /** @type {Set} */ + dependent = new Set(); + /** @type {Map>>} */ async_deriveds = new Map(); @@ -205,6 +223,8 @@ export class Batch { is_fork = false; + is_eager = false; + #decrement_queued = false; constructor() { @@ -421,7 +441,6 @@ export class Batch { } const earlier_batch = this.#find_earlier_batch(); - if (earlier_batch) { // If this batch collected deferred effects during traversal, they still need // to run after being merged into the earlier batch. @@ -439,6 +458,7 @@ export class Batch { for (const fn of this.#commit_callbacks) fn(this); this.#commit_callbacks.clear(); + this.apply(true); previous_batch = this; flush_queued_effects(render_effects); flush_queued_effects(effects); @@ -536,15 +556,23 @@ export class Batch { } #find_earlier_batch() { + if (this.is_eager) return null; + var batch = this.#prev; while (batch !== null) { if (!batch.is_fork) { // if the batches are connected, break - for (const [value, [, is_derived]] of this.current) { - if (batch.current.has(value) && !is_derived) { - return batch; - } + // for (const [value, [, is_derived]] of this.current) { + // if (batch.current.has(value) && !is_derived) { + // return batch; + // } + // } + if (this.dependent.has(batch)) { + // TODO what if there's a fork between the chosen batch and the current one, + // then the fork commits (but is still pending), then the chosen batch + // finishes - then we would apply UI update of B1+B3 before B2. + return batch; } } @@ -604,6 +632,8 @@ export class Batch { } else { var effect = /** @type {Effect} */ (reaction); + // TODO this overfires e.g. for async-state-new-branch-fork-5 where it reruns + // the Child async effects with "world" after commit+first resolve. if (flags & (ASYNC | BLOCK_EFFECT) && !this.async_deriveds.has(effect)) { this.#maybe_dirty_effects.delete(effect); set_signal_status(effect, DIRTY); @@ -651,8 +681,26 @@ export class Batch { batch_values?.set(source, value); } + let batch = this.#prev; + while (batch) { + if (batch.current.has(source)) { + this.dependent.add(batch); + break; + } + batch = batch.#prev; + } + if (!this.is_fork) { - source.v = value; + let is_latest_value = true; + batch = this.#next; + while (batch) { + if (batch.current.has(source)) { + is_latest_value = false; + break; + } + batch = batch.#next; + } + if (is_latest_value) source.v = value; } } @@ -715,6 +763,7 @@ export class Batch { } #commit() { + return; // 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 @@ -928,7 +977,7 @@ export class Batch { return current_batch; } - apply() { + apply(include_later = false) { if (!async_mode_flag || (!this.is_fork && this.#prev === null && this.#next === null)) { batch_values = null; return; @@ -937,10 +986,30 @@ 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(); + held_sources = new Map(); for (const [source, [value]] of this.current) { batch_values.set(source, value); } + for (let batch = first_batch; batch !== null; batch = batch.#next) { + if (batch === this) continue; + + if (batch.id < this.id) { + for (const source of batch.current.keys()) { + held_sources.set(source, batch); + } + } + + if (batch.is_fork) continue; + + if (batch.id > this.id || include_later || this.is_eager) { + for (const [source, value] of batch.previous) { + if (!batch_values.has(source)) batch_values.set(source, value); + } + } + } + return; + // ...and undo changes belonging to other batches unless they intersect for (let batch = first_batch; batch !== null; batch = batch.#next) { if (batch === this || batch.is_fork) continue; @@ -1265,6 +1334,8 @@ let eager_versions = []; function eager_flush() { flushSync(() => { + var batch = Batch.ensure(); + batch.is_eager = true; const eager = eager_versions; eager_versions = []; for (const version of eager) { @@ -1295,6 +1366,10 @@ export function eager(fn) { let version = version_map.get(parent) ?? source(0); version_map.set(parent, version); + if (DEV) { + version.label ??= '$state.eager version'; + } + teardown(() => { if (parent.f & DESTROYING) version_map.delete(parent); }); @@ -1449,6 +1524,23 @@ export function fork(fn) { flush_eager_effects(); }); + // let next_batch = batch.next; + // while (next_batch) { + // for (const [effect] of batch.async_deriveds) { + // if (next_batch.async_deriveds.has(effect)) { + // next_batch.dependent.add(batch); + // if (!next_batch.is_fork) { + // set_signal_status(effect, DIRTY); // TODO ideally we can find out if we really need to rerun or if all dependencies' values are equal + // // TODO same for block effects; ideally one mechanism for both + // next_batch.schedule(effect); + // const b = next_batch; + // queue_micro_task(() => b.flush()); + // } + // } + // } + // next_batch = next_batch.next; + // } + 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 728ef1d214..a5f9231234 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -43,10 +43,11 @@ import { get_error } from '../../shared/dev.js'; import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_values, current_batch, previous_batch } from './batch.js'; +import { batch_values, current_batch, first_batch, previous_batch } from './batch.js'; import { increment_pending, unset_context } from './async.js'; import { deferred, includes, noop } from '../../shared/utils.js'; import { set_signal_status, update_derived_status } from './status.js'; +import { queue_micro_task } from '../dom/task.js'; /** * This allows us to track 'reactivity loss' that occurs when signals @@ -133,6 +134,7 @@ export function async_derived(fn, label, location) { if (DEV) { reactivity_loss_tracker = { effect, effect_deps: new Set(), warned: false }; + effect.label ??= label ?? fn.toString(); } /** @type {ReturnType>} */ @@ -179,6 +181,31 @@ export function async_derived(fn, label, location) { var batch = /** @type {Batch} */ (current_batch); + let next_batch = batch.next; + while (next_batch) { + if (next_batch.async_deriveds.has(effect)) { + next_batch.dependent.add(batch); + if (!next_batch.is_fork) { + set_signal_status(effect, DIRTY); // TODO ideally we can find out if we really need to rerun or if all dependencies' values are equal + // TODO same for block effects; ideally one mechanism for both + next_batch.schedule(effect); + const b = next_batch; + queue_micro_task(() => b.flush()); + } + break; // TODO break correct? Don't we need to do the rerun for all of them? + } + next_batch = next_batch.next; + } + + let prev = batch.prev; + while (prev) { + if (prev.async_deriveds.has(effect)) { + batch.dependent.add(prev); + break; + } + prev = prev.prev; + } + if (should_suspend) { // we only increment the batch's pending state for updates, not creation, otherwise // we will decrement to zero before the work that depends on this promise (e.g. a @@ -427,11 +454,15 @@ export function update_derived(derived) { // During time traveling we don't want to reset the status so that // traversal of the graph in the other batches still happens - if (batch_values !== null) { + if ( + batch_values !== null || + (!current_batch && + first_batch?.next) /* means "read outside of reactivity, e.g. in an event hanlder" */ + ) { // only cache the value if we're in a tracking context, otherwise we won't // clear the cache in `mark_reactions` when dependencies are updated if (effect_tracking() || current_batch?.is_fork) { - batch_values.set(derived, value); + batch_values?.set(derived, value); } } else { update_derived_status(derived); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 27def05300..65180e00e5 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -50,6 +50,7 @@ import { batch_values, current_batch, flushSync, + held_sources, previous_batch, schedule_effect } from './reactivity/batch.js'; @@ -548,6 +549,7 @@ export function settled() { export function get(signal) { var flags = signal.f; var is_derived = (flags & DERIVED) !== 0; + var first_time = false; captured_signals?.add(signal); @@ -558,6 +560,8 @@ export function get(signal) { // we don't add the dependency, because that would create a memory leak var destroyed = active_effect !== null && (active_effect.f & DESTROYED) !== 0; + first_time = (active_reaction.f & REACTION_RAN) === 0; + if (!destroyed && (current_sources === null || !current_sources.has(signal))) { var deps = active_reaction.deps; @@ -706,7 +710,11 @@ export function get(signal) { } } - if (batch_values?.has(signal)) { + if (current_batch && held_sources?.has(signal)) { + current_batch.dependent.add(/** @type {Batch} */ (held_sources.get(signal))); + } + + if ((!first_time || current_batch?.is_fork) && batch_values?.has(signal)) { return batch_values.get(signal); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-log-outside-reactivity/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-log-outside-reactivity/_config.js index 1a7ed0eb92..12a4e0d521 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-derived-log-outside-reactivity/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-log-outside-reactivity/_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-linear-order-same-derived/_config.js b/packages/svelte/tests/runtime-runes/samples/async-linear-order-same-derived/_config.js index cc7b2756fa..922980bf3f 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-linear-order-same-derived/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-linear-order-same-derived/_config.js @@ -15,14 +15,19 @@ export default test({ flushSync(() => a.click()); flushSync(() => b.click()); - pop.click(); + shift.click(); + await tick(); + assert.htmlEqual(p.innerHTML, '2 + 2 = 4'); + + shift.click(); await tick(); + assert.htmlEqual(p.innerHTML, '2 + 3 = 5'); - assert.htmlEqual(p.innerHTML, '1 + 3 = 4'); + flushSync(() => a.click()); + flushSync(() => b.click()); pop.click(); await tick(); - - assert.htmlEqual(p.innerHTML, '2 + 3 = 5'); + assert.htmlEqual(p.innerHTML, '3 + 4 = 7'); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-5/_config.js b/packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-5/_config.js index d03f3cfbb8..6ba84c63b2 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-5/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-overlap-multiple-5/_config.js @@ -47,41 +47,12 @@ export default test({ // how it's on main - shift.click(); - await tick(); - assert.htmlEqual( - target.innerHTML, - ` - a 0 | b 0 | c 1 | d 1 - - - - - ` - ); - - shift.click(); - await tick(); - shift.click(); - await tick(); - assert.htmlEqual( - target.innerHTML, - ` - a 1 | b 2 | c 1 | d 3 - - - - - ` - ); - - // how it's on https://github.com/sveltejs/svelte/pull/17971 // shift.click(); // await tick(); // assert.htmlEqual( // target.innerHTML, // ` - // a 0 | b 0 | c 0 | d 0 + // a 0 | b 0 | c 1 | d 1 // // // @@ -91,17 +62,6 @@ export default test({ // shift.click(); // await tick(); - // assert.htmlEqual( - // target.innerHTML, - // ` - // a 1 | b 2 | c 0 | d 2 - // - // - // - // - // ` - // ); - // shift.click(); // await tick(); // assert.htmlEqual( @@ -114,5 +74,45 @@ export default test({ // // ` // ); + + // how it's on https://github.com/sveltejs/svelte/pull/17971 and here + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + a 0 | b 0 | c 0 | d 0 + + + + + ` + ); + + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + a 1 | b 2 | c 0 | d 2 + + + + + ` + ); + + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + a 1 | b 2 | c 1 | d 3 + + + + + ` + ); } }); 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..0bc8fd34aa 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 @@ -10,26 +10,29 @@ export default test({ y.click(); await tick(); + // the new branch reads `x`, which the pending batch has written, as a new + // dependency — the two batches entangle, so the new branch is held back + // until the async work completes assert.htmlEqual( target.innerHTML, ` - world - ` // if this does not show world - that would also be ok + ` // if this does 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']); + // this was also ok (on main): + // assert.deepEqual(logs, [ + // 'universe', + // 'world', + // '$effect: world', + // '$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..b3b1cacef6 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 + ` // if this does 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..0b2d9ac6de 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 + ` // if this does 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-fork-5/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js index e8f16ade3c..4881907ade 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js @@ -44,12 +44,6 @@ export default test({
- world - "world" - world - world - world - "world" ` ); 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..c4e72425ef 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,23 @@ export default test({ y.click(); await tick(); - assert.deepEqual(logs, ['universe', 'world', '$effect: world']); + // the new branch reads `x`, which the pending batch has written, as a new + // dependency — the two batches entangle, so the new branch is held back + // until the async work completes. Its $effect is deferred, but the + // init-time console.log necessarily runs eagerly (with the latest value) + 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, `