diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index a75f97a54f..3548e52fcd 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -143,7 +143,7 @@ export function capture() { if (activate_batch && (previous_effect.f & DESTROYED) === 0) { // TODO we only need optional chaining here because `{#await ...}` blocks // are anomalous. Once we retire them we can get rid of it - previous_batch?.activate(); + previous_batch = previous_batch?.activate(); previous_batch?.apply(); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f71f80b1a0..0e4e7ae96b 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -79,6 +79,12 @@ export let wv_values = null; */ export let held_sources = null; +/** + * Sources where the current batch reads an outdated value not in its own `current` map. Used to discover dependencies between batches. + * @type {Map | null} + */ +export let stale_sources = null; + /** @type {Effect | null} */ let last_scheduled_effect = null; @@ -117,6 +123,9 @@ export class Batch { linked = true; + /** @type {Batch | null} */ + merged_into = null; + /** @type {Map} */ stale_effects = new Map(); @@ -852,6 +861,7 @@ export class Batch { this.oncommit(() => batch.discard()); batch.#unlink(); + batch.merged_into = this; current_batch = this; this.#process(); @@ -896,7 +906,8 @@ export class Batch { let batch = this.next; let is_latest_value = !this.is_fork; while (batch) { - if (source.f & ASYNC) { + // TODO this is obsolete through runtime.js logic? + if (source.f & ASYNC && false) { // TODO I think this is wrong IF the async source was already written to by a later batch; // we gotta check if it's the source is also part of the later batch. const b = batch; @@ -1015,8 +1026,13 @@ export class Batch { // } } + /** + * Activate batch - could be merged into another batch in the meantime, + * in which case that other batch becomes the active batch. + * @returns {Batch} + */ activate() { - current_batch = this; + return (current_batch = this.merged_into?.activate() ?? this); } deactivate() { @@ -1045,6 +1061,7 @@ export class Batch { current_batch = null; batch_values = null; wv_values = null; + stale_sources = null; old_values.clear(); @@ -1305,6 +1322,7 @@ export class Batch { if (!async_mode_flag || (!this.is_fork && this.prev === null && this.next === null)) { batch_values = null; wv_values = null; + stale_sources = null; return; } @@ -1313,6 +1331,8 @@ export class Batch { batch_values = new Map(); wv_values = new Map(); held_sources = new Map(); + stale_sources = new Map(); + for (const [source, [value, _, wv]] of this.current) { batch_values.set(source, value); wv_values.set(source, wv); @@ -1329,6 +1349,12 @@ export class Batch { if (batch.is_fork) continue; + if (batch.id > this.id || this.is_eager) { + for (const source of batch.current.keys()) { + if (!this.current.has(source)) stale_sources.set(source, batch); + } + } + if (batch.id > this.id || include_earlier || this.is_eager) { for (const [source, value] of batch.previous) { if (!batch_values.has(source)) { diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 07ef895933..1a3c77f306 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -251,7 +251,7 @@ export function async_derived(fn, label, location) { if (error === OBSOLETE) return; - batch.activate(); + batch = batch.activate(); if (error) { signal.f |= ERROR_VALUE; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 75575bafed..ba8b4a690e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -22,7 +22,8 @@ import { STALE_REACTION, ERROR_VALUE, MANAGED_EFFECT, - REACTION_RAN + REACTION_RAN, + ASYNC } from './constants.js'; import { old_values } from './reactivity/sources.js'; import { @@ -53,6 +54,7 @@ import { held_sources, previous_batch, schedule_effect, + stale_sources, wv_values } from './reactivity/batch.js'; import { handle_error } from './error-handling.js'; @@ -61,6 +63,7 @@ import { captured_signals } from './legacy.js'; import { without_reactive_context } from './dom/elements/bindings/shared.js'; import { set_signal_status, update_derived_status } from './reactivity/status.js'; import * as w from './warnings.js'; +import { queue_micro_task } from './dom/task.js'; /** * True if updating in an effect context that is reactive (i.e. not branch/root effects) @@ -781,8 +784,34 @@ export function get(signal) { } } - if (current_batch && held_sources?.has(signal)) { - current_batch.dependent.add(/** @type {Batch} */ (held_sources.get(signal))); + if (current_batch || previous_batch?.is_eager) { + const current = /** @type {Batch} */ (current_batch ?? previous_batch); + if (!current.is_eager && held_sources?.has(signal)) { + current.dependent.add(/** @type {Batch} */ (held_sources.get(signal))); + } + const batch = stale_sources?.get(signal); + if (batch) { + if (!current.is_eager) batch.dependent.add(current); + // TODO do we only need this for async/block effects? + if (active_effect && (is_updating_effect || active_effect.f & ASYNC)) { + const effect = active_effect; + // TODO can overfire when two stale reads within one effect, because no "already scheduled this" logic. + // TODO how to know "ok we already did this now" + if (current.is_eager) { + batch.oncommit(() => { + const b = Batch.ensure(); + set_signal_status(effect, DIRTY); + b.schedule(effect); + }); + } else { + queue_micro_task(() => { + set_signal_status(effect, DIRTY); + batch.schedule(effect); + batch.flush(); + }); + } + } + } } if ( 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 6ba84c63b2..f07c86dc0c 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 @@ -45,37 +45,6 @@ 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 and here shift.click(); await tick(); assert.htmlEqual( diff --git a/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-in-committed-batch/_config.js b/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-in-committed-batch/_config.js index 6a65bb3c1a..049f1989bc 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-in-committed-batch/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-in-committed-batch/_config.js @@ -32,11 +32,11 @@ export default test({ await tick(); // B's continuation first-reads `a`, which is overlaid by the pending - // merged batch. B has already committed its UI, so it cannot entangle — - // it reads the latest value (1) instead + // merged batch's pre-write value (0). This is noticed so B tells C + // to rerun the async effect with the current value. shift_t.click(); await tick(); - assert.htmlEqual(target.innerHTML, `${buttons}

late read: 1

loading 2...

`); + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

loading 2...

`); // revert `a` to 0 inside the pending batch — its eventual commit leaves // `a` unchanged. The write re-runs the late reader (it acquired `a` as a @@ -51,7 +51,8 @@ export default test({ await tick(); shift_a.click(); await tick(); - assert.htmlEqual(target.innerHTML, `${buttons}

late read: 1

async a: 0

`); + // only rerun from above still pending + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

async a: 0

`); // resolve the late reader's re-run -> it converges on the committed value shift_t.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-stale-reader/_config.js b/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-stale-reader/_config.js new file mode 100644 index 0000000000..91db9bdd65 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-read-pending-value-stale-reader/_config.js @@ -0,0 +1,49 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ` + + + + + + +`; + +export default test({ + async test({ assert, target }) { + await tick(); + + const [up, , show1, show2, shift_a, shift_t] = target.querySelectorAll('button'); + + show1.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

`); + + show2.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

loading 2...

`); + + // batch A: writes a=1; async-a effect re-runs and is pending + up.click(); + await tick(); + + // B's continuation first-reads `a` through the pending batch's pre-write overlay. This + // causes a rerun of the async effect inside A. + shift_t.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

loading 2...

`); + + // resolve the pending batch's async-a runs -> it commits a=1. + shift_a.click(); + await tick(); + shift_a.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

loading 1...

async a: 1

`); + + // finish triggered rerun + shift_t.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

late read: 1

async a: 1

`); + } +});