diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index e81e68354f..54ebb8033f 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -2,7 +2,7 @@ export const DERIVED = 1 << 1; export const EFFECT = 1 << 2; export const RENDER_EFFECT = 1 << 3; -export const TEMPLATE_EFFECT = 1 << 26; +export const TEMPLATE_EFFECT = 1 << 26; // TODO we might not need this after all /** * An effect that does not destroy its child effects when it reruns. * Runs as part of render effects, i.e. not eagerly as part of tree traversal or effect flushing. @@ -59,6 +59,18 @@ export const ASYNC = 1 << 22; export const ERROR_VALUE = 1 << 23; +export const ANY_EFFECT = + EFFECT | + RENDER_EFFECT | + TEMPLATE_EFFECT | + MANAGED_EFFECT | + BLOCK_EFFECT | + BRANCH_EFFECT | + ROOT_EFFECT | + BOUNDARY_EFFECT | + // proxy for async effects, because we can't use ASYNC, because it's also set on the source + EFFECT_PRESERVED; + export const STATE_SYMBOL = Symbol('$state'); export const LEGACY_PROPS = Symbol('legacy props'); export const LOADING_ATTR_SYMBOL = Symbol(''); diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index c3b5d23915..8286504b3e 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -68,10 +68,23 @@ export class BranchManager { this.#transition = transition; } + #consolidate() { + // Without this, .has/.get/.set could return false negatives when + // #commit/#discard are called with the successor of an obsolete batch + for (const [batch, key] of this.#batches) { + const live = Batch.find(batch); + if (live !== batch) { + this.#batches.set(live, key); + this.#batches.delete(batch); + } + } + } + /** * @param {Batch} batch */ #commit = (batch) => { + this.#consolidate(); // if this batch was made obsolete, bail if (!this.#batches.has(batch)) return; @@ -156,6 +169,7 @@ export class BranchManager { * @param {Batch} batch */ #discard = (batch) => { + this.#consolidate(); this.#batches.delete(batch); const keys = Array.from(this.#batches.values()); diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index faa7277644..e8f1ba48de 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -17,7 +17,8 @@ import { ERROR_VALUE, MANAGED_EFFECT, REACTION_RAN, - STALE_REACTION + STALE_REACTION, + USER_EFFECT } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property, includes } from '../../shared/utils.js'; @@ -185,7 +186,13 @@ export class Batch { return this.is_fork || this.#blocking_pending > 0; } + /** @returns {boolean} */ finished() { + // TODO instead of littering this class with if(this.successor) checks, it's probably less code/better to use Batch.find(batch).x() in the relevant places + if (this.successor) { + return this.successor.finished(); + } + return this.#pending === 0 && !this.is_fork && this.#commit_callbacks.size === 0; } @@ -195,6 +202,11 @@ export class Batch { * @param {(batch: Batch) => void} on_discard */ skip_effect(effect, on_discard) { + if (this.successor) { + this.successor.skip_effect(effect, on_discard); + return; + } + if (!this.#skipped_branches.has(effect)) { this.#skipped_branches.set(effect, { d: [], m: [], on_discard }); } @@ -206,6 +218,11 @@ export class Batch { * @param {Effect} effect */ unskip_effect(effect) { + if (this.successor) { + this.successor.unskip_effect(effect); + return; + } + var tracked = this.#skipped_branches.get(effect); if (tracked) { this.#skipped_branches.delete(effect); @@ -244,6 +261,8 @@ export class Batch { */ var updates = (legacy_updates = []); + if (this.id === 5) debugger; + for (const root of roots) { this.#traverse(root, effects, render_effects); } @@ -261,13 +280,16 @@ export class Batch { collected_effects = null; legacy_updates = null; - if (this.#is_deferred()) { + if (this.#is_deferred() || this.successor) { this.#defer_effects(render_effects); this.#defer_effects(effects); for (const [e, t] of this.#skipped_branches) { reset_branch(e, t); } + + // Once more for the potential new (render) effects + if (this.successor) Batch.merge(this, this.successor); } else { // clear effects. Those that are still needed will be rescheduled through unskipping the skipped branches. this.#dirty_effects.clear(); @@ -372,7 +394,12 @@ export class Batch { * @param {any} value */ capture(source, value) { - if (!this.is_fork) source.batch = this; + if (this.successor) { + this.successor.capture(source, value); + return; + } + + if (!this.is_fork) source.batch = Batch.upsert(source); if (value !== UNINITIALIZED && !this.previous.has(source)) { this.previous.set(source, value); @@ -386,10 +413,20 @@ export class Batch { } activate() { + if (this.successor) { + this.successor.activate(); + return; + } + current_batch = this; } deactivate() { + if (this.successor) { + this.successor.deactivate(); + return; + } + current_batch = null; batch_values = null; } @@ -443,6 +480,11 @@ export class Batch { } discard() { + if (this.successor) { + this.successor.discard(); + return; + } + for (const fn of this.#discard_callbacks) fn(this); this.#discard_callbacks.clear(); } @@ -530,6 +572,11 @@ export class Batch { * @param {boolean} blocking */ increment(blocking) { + if (this.successor) { + this.successor.increment(blocking); + return; + } + this.#pending += 1; if (blocking) this.#blocking_pending += 1; } @@ -539,6 +586,11 @@ export class Batch { * @param {boolean} skip - whether to skip updates (because this is triggered by a stale reaction) */ decrement(blocking, skip) { + if (this.successor) { + this.successor.decrement(blocking, skip); + return; + } + this.#pending -= 1; if (blocking) this.#blocking_pending -= 1; @@ -553,27 +605,61 @@ export class Batch { /** @param {(batch: Batch) => void} fn */ oncommit(fn) { + if (this.successor) { + this.successor.oncommit(fn); + return; + } + this.#commit_callbacks.add(fn); } /** @param {(batch: Batch) => void} fn */ ondiscard(fn) { + if (this.successor) { + this.successor.ondiscard(fn); + return; + } + this.#discard_callbacks.add(fn); } + /** @returns {Promise} */ settled() { + if (this.successor) { + return this.successor.settled(); + } + return (this.#deferred ??= deferred()).promise; } /** * Ensure there is a current batch for scheduling work triggered by `reaction`. * If both the current batch and the reaction batch are active, merge them. - * @param {Reaction} reaction + * @param {Value | Reaction} reaction */ static upsert(reaction) { + if (reaction.f & RENDER_EFFECT && !(reaction.f & USER_EFFECT)) { + return Batch.ensure(); + } + + var existing_batch = [...batches].find( + (b) => !b.is_fork && b.current.has(/** @type {any} */ (reaction)) + ); + var reaction_batch = reaction.batch; var has_reaction_batch = reaction_batch !== null && !reaction_batch.finished(); + if (existing_batch) { + if (reaction_batch) { + Batch.merge(reaction_batch, existing_batch); + } + if (current_batch) { + Batch.merge(current_batch, existing_batch); + } + current_batch = existing_batch; + return existing_batch; + } + if (current_batch === null) { if (has_reaction_batch) { current_batch = reaction_batch; @@ -605,6 +691,8 @@ export class Batch { * @param {Batch} to */ static merge(from, to) { + // from = Batch.find(from); + to = Batch.find(to); if (from === to) return; for (const [source, value] of from.previous) { @@ -614,14 +702,13 @@ export class Batch { } for (const [source, value] of from.current) { - to.current.set(source, value); + to.current.set(source, value); // TODO do we somehow need to know which one's more recent here? if (!to.is_fork) source.batch = to; } - to.#pending += from.#pending; to.#blocking_pending += from.#blocking_pending; - - to.#roots.push(...from.#roots); + to.#pending += from.#pending; + to.#roots.push(...from.#roots.filter((r) => !to.#roots.includes(r))); for (const e of from.#dirty_effects) to.#dirty_effects.add(e); for (const e of from.#maybe_dirty_effects) to.#maybe_dirty_effects.add(e); @@ -658,10 +745,21 @@ export class Batch { from.#decrement_queued = false; from.successor = to; + // Apply before modifying batches, else apply might erronously bail because there's only one batch left + if (batch_values !== null) to.apply(); + batches.delete(from); batches.add(to); } + /** @param {Batch} batch */ + static find(batch) { + while (batch.successor) { + batch = batch.successor; + } + return batch; + } + static ensure() { if (current_batch === null) { const batch = (current_batch = new Batch()); @@ -682,16 +780,15 @@ export class Batch { } } - return current_batch; + return Batch.find(current_batch); } - #scheduling = false; - + // TODO do we need this or can we assume decrement does what we want? reschedule() { - if (!is_flushing_sync && !this.#scheduling && this.#is_deferred()) { - this.#scheduling = true; + if (!is_flushing_sync && !this.#decrement_queued && this.#is_deferred()) { + this.#decrement_queued = true; queue_micro_task(() => { - this.#scheduling = false; + this.#decrement_queued = false; this.flush(); }); } @@ -721,6 +818,11 @@ export class Batch { * @param {Effect} effect */ schedule(effect) { + if (this.successor) { + this.successor.schedule(effect); + return; + } + last_scheduled_effect = effect; // defer render effects inside a pending boundary @@ -1078,20 +1180,15 @@ export function eager(fn) { * @param {{ d: Effect[], m: Effect[] }} tracked */ function reset_branch(effect, tracked) { - // Clean branches normally need no traversal. But if a branch belongs to the - // current batch, we still need to walk it to cancel stale async effects. - if ( - (effect.f & BRANCH_EFFECT) !== 0 && - (effect.f & CLEAN) !== 0 && - effect.batch !== current_batch - ) { + // Clean branches normally need no traversal. + if ((effect.f & BRANCH_EFFECT) !== 0 && (effect.f & CLEAN) !== 0) { return; } - // This branch is stale for the current batch. Cancel async effects that - // belong to this batch so pending async work is rejected/discarded, - // which is important for getting the batch's pending count to 0. - if ((effect.f & ASYNC) !== 0 && effect.batch === current_batch) { + // This branch is stale for the current batch. Cancel async effects so + // pending async work is rejected/discarded, which is important for getting + // the batch's pending count to 0. + if ((effect.f & ASYNC) !== 0) { effect.ac?.abort(STALE_REACTION); effect.ac = null; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 2ebc5689a7..6a72f11c66 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -384,7 +384,10 @@ export function render_effect(fn, flags = 0) { */ export function template_effect(fn, sync = [], async = [], blockers = []) { flatten(blockers, sync, async, (values) => { - create_effect(RENDER_EFFECT | TEMPLATE_EFFECT, () => fn(...values.map(get))); + const e = create_effect(RENDER_EFFECT | TEMPLATE_EFFECT, () => fn(...values.map(get))); + if (DEV) { + e.original_fn = fn; + } }); } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7ce9598276..18b0e37bb8 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -27,10 +27,7 @@ import { ROOT_EFFECT, ASYNC, WAS_MARKED, - CONNECTED, - RENDER_EFFECT, - USER_EFFECT, - TEMPLATE_EFFECT + CONNECTED } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -43,7 +40,8 @@ import { batch_values, eager_block_effects, schedule_effect, - legacy_updates + legacy_updates, + current_batch } from './batch.js'; import { proxy } from '../proxy.js'; import { execute_derived } from './deriveds.js'; @@ -78,7 +76,7 @@ export function set_eager_effects_deferred() { export function source(v, stack) { /** @type {Value} */ var signal = { - batch: null, + batch: current_batch, f: 0, // TODO ideally we could skip this altogether, but it causes type errors v, reactions: null, @@ -243,7 +241,6 @@ export function internal_set(source, value, updated_during_traversal = null) { var batch = Batch.ensure(); batch.capture(source, old_value); - if (!batch.is_fork) source.batch = batch; batch.reschedule(); // It's possible that the current reaction might not have up-to-date dependencies diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 78b2132eec..1a99898726 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -23,7 +23,8 @@ import { ERROR_VALUE, WAS_MARKED, MANAGED_EFFECT, - REACTION_RAN + REACTION_RAN, + ANY_EFFECT } from './constants.js'; import { old_values } from './reactivity/sources.js'; import { @@ -154,6 +155,12 @@ export function increment_write_version() { */ export function is_dirty(reaction) { var flags = reaction.f; + var is_effect = (flags & ANY_EFFECT) !== 0; + + // We don't want to create/merge batches when flushing (render) effects, only during batch.#traverse() + if (current_batch !== null && !is_effect) { + reaction.batch = Batch.upsert(reaction); + } if ((flags & DIRTY) !== 0) { return true; @@ -164,7 +171,7 @@ export function is_dirty(reaction) { } if ((flags & MAYBE_DIRTY) !== 0) { - var dependencies = /** @type {Value[]} */ (reaction.deps); + var dependencies = /** @type {Value[]} */ (reaction.deps); var length = dependencies.length; for (var i = 0; i < length; i++) { @@ -341,6 +348,7 @@ export function update_reaction(reaction) { reaction.f ^= ERROR_VALUE; } + // TODO needed? if (current_batch !== null && !current_batch.is_fork) { reaction.batch = current_batch; } @@ -555,6 +563,10 @@ export function get(signal) { new_deps.push(signal); } } + + // Do this here so that it's invoked while executing deriveds and effects during batch.#traverse(). + // A potential merge will adjust batch_values correctly, which is crucial. + if (current_batch !== null) signal.batch = Batch.upsert(signal); } else { // we're adding a dependency outside the init/update cycle // (i.e. after an `await`) diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/_config.js b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/_config.js new file mode 100644 index 0000000000..00d106c8c2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/_config.js @@ -0,0 +1,59 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [one, two, both, resolve] = target.querySelectorAll('button'); + + one.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 0' + ); + + two.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 0' + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 1' + ); + + one.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 1' + ); + + both.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 1' + ); + + two.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 1' + ); + + resolve.click(); + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ' 3' + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/main.svelte new file mode 100644 index 0000000000..fbcdf07e77 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-1/main.svelte @@ -0,0 +1,19 @@ + + + + + + +{await delay(count1)} diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/_config.js new file mode 100644 index 0000000000..8515670362 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/_config.js @@ -0,0 +1,52 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [count1, count2, both, resolve] = target.querySelectorAll('button'); + + count1.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + count2.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

1

' + ); + + count1.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

1

' + ); + + both.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

1

' + ); + + resolve.click(); + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

3

' + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/main.svelte new file mode 100644 index 0000000000..b9c79bbb05 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-2/main.svelte @@ -0,0 +1,22 @@ + + + + + + +

{await delay(count1)}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/_config.js b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/_config.js new file mode 100644 index 0000000000..de15dfcf5e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/_config.js @@ -0,0 +1,46 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [count1, count2, resolve] = target.querySelectorAll('button'); + + count1.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + count2.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + count2.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + count2.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

0

' + ); + + resolve.click(); + await tick(); + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + '

1

1nested

' + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/main.svelte new file mode 100644 index 0000000000..48f8cb0592 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-merge-3/main.svelte @@ -0,0 +1,22 @@ + + + + + + +

{await delay(count1)}

+{#if count2 > 2} +

{await delay(count1 + 'nested')}

+{/if}