diff --git a/.changeset/new-render-effects-order.md b/.changeset/new-render-effects-order.md new file mode 100644 index 0000000000..4e8fce9531 --- /dev/null +++ b/.changeset/new-render-effects-order.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking(experimental-async): run render/pre effects created during traversal alongside existing render effects, in tree order diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index b086bfedff..ca0ede6f54 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -50,6 +50,12 @@ export const HEAD_EFFECT = 1 << 18; export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; export const EFFECT_OFFSCREEN = 1 << 25; +/** + * Set on render effects that are created during traversal: unlike other render effects, they don't run + * right away (i.e. while their parent is being created), but together with the other render effects + * once the batch flushes (see `collected_render_effects`) + */ +export const DEFERRED_RUN = 1 << 26; // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index 141b914537..fcb5774a19 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -186,8 +186,13 @@ export class BranchManager { ensure(key, fn) { var batch = /** @type {Batch} */ (current_batch); var defer = should_defer_append(); + var offscreen = this.#offscreen.get(key); - if (fn && !this.#onscreen.has(key) && !this.#offscreen.has(key)) { + if (offscreen !== undefined) { + // the branch may have been created by another batch that hasn't committed yet, + // in which case it may contain effects that haven't run yet + batch.reveal(offscreen.effect); + } else if (fn && !this.#onscreen.has(key)) { if (defer) { var fragment = document.createDocumentFragment(); var target = create_text(); diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 58d59a6825..0a9da4950b 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -323,6 +323,12 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (defer) { batch.unskip_effect(item.e); } + + if ((item.e.f & EFFECT_OFFSCREEN) !== 0) { + // the item may have been created by another batch that hasn't committed yet, + // in which case it may contain effects that haven't run yet + batch.reveal(item.e); + } } else { item = create_item( items, @@ -352,6 +358,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f fallback = branch(() => fallback_fn((offscreen_anchor ??= create_text()))); fallback.f |= EFFECT_OFFSCREEN; } + } else if (length === 0 && fallback !== null && (fallback.f & EFFECT_OFFSCREEN) !== 0) { + batch.reveal(fallback); } if (length > keys.size) { diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 7c69f05dcc..6481b079e7 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -7,6 +7,7 @@ import { ERROR_VALUE, BOUNDARY_EFFECT, REACTION_RAN, + DEFERRED_RUN, EFFECT, DESTROYED, DESTROYING @@ -32,10 +33,10 @@ export function handle_error(error) { adjustments.set(error, get_adjustments(error, effect)); } - // if the error occurred while creating this subtree, we let it - // bubble up until it hits a boundary that can handle it, unless - // it's an $effect in which case it doesn't run immediately - if ((effect.f & REACTION_RAN) === 0 && (effect.f & EFFECT) === 0) { + // if the error occurred while creating this subtree, we let it bubble up until it hits + // a boundary that can handle it, unless it's an $effect or a render effect created during + // traversal, in which case it doesn't run immediately (but once the batch flushes) + if ((effect.f & REACTION_RAN) === 0 && (effect.f & (EFFECT | DEFERRED_RUN)) === 0) { if (DEV && !effect.parent && error instanceof Error) { apply_adjustments(error); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 655dfc6598..db2ca4a7e7 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -78,6 +78,13 @@ let is_processing = false; */ export let collected_effects = null; +/** + * Like `collected_effects`, but for render (and managed) effects. Only used in async mode, + * where these effects run after traversal rather than during it + * @type {Effect[] | null} + */ +export let collected_render_effects = null; + /** * An array of effects that are marked during traversal as a result of a `set` * (not `internal_set`) call. These will be added to the next batch and @@ -271,6 +278,30 @@ export class Batch { this.#unskipped_branches.add(effect); } + /** + * Called when this batch selects a branch that already exists offscreen, i.e. one that + * was created by a batch that hasn't committed yet (possibly a different one, or a fork). + * Effects created inside it during traversal (`$effect`s, and render effects in async mode) + * are deferred until the creating batch commits, so they may not have run yet. Schedule + * them, so that they run in case this batch commits first. + * @param {Effect} effect + */ + reveal(effect) { + var e = effect.first; + + while (e !== null) { + var flags = e.f; + + if ((flags & (BRANCH_EFFECT | ROOT_EFFECT | REACTION_RAN)) === 0 && e.fn !== null) { + set_signal_status(e, DIRTY); + this.schedule(e); + } + + this.reveal(e); + e = e.next; + } + } + /** * Convert the effects that were scheduled in this batch into the root effects * that need to be traversed, marking the path to each effect (by clearing the @@ -347,6 +378,10 @@ export class Batch { /** @type {Effect[]} */ var render_effects = []; + if (async_mode_flag) { + collected_render_effects = render_effects; + } + /** * @type {Effect[]} * @deprecated when we get rid of legacy mode and stores, we can get rid of this @@ -389,6 +424,7 @@ export class Batch { } collected_effects = null; + collected_render_effects = null; legacy_updates = null; // if the batch has outstanding pending work, stash effects and bail @@ -426,7 +462,13 @@ export class Batch { this.#commit_callbacks.clear(); previous_batch = this; + + // effects created by render effects (e.g. inside newly created branches) are + // collected and run alongside the existing effects + collected_effects = effects; flush_queued_effects(render_effects); + collected_effects = null; + flush_queued_effects(effects); previous_batch = null; @@ -665,6 +707,7 @@ export class Batch { flush_count = 0; last_scheduled_effect = null; collected_effects = null; + collected_render_effects = null; legacy_updates = null; is_processing = false; diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index e69a639a94..6ca118345a 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -35,7 +35,8 @@ import { CONNECTED, MANAGED_EFFECT, DESTROYING, - PAUSED + PAUSED, + DEFERRED_RUN } from '#client/constants'; import { invoke_error_boundary } from '../error-handling.js'; import * as e from '../errors.js'; @@ -43,7 +44,7 @@ import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; import { component_context, dev_current_component_function, dev_stack } from '../context.js'; -import { Batch, collected_effects, current_batch } from './batch.js'; +import { Batch, collected_effects, collected_render_effects, current_batch } from './batch.js'; import { flatten } from './async.js'; import { without_reactive_context } from '../dom/elements/bindings/shared.js'; import { set_signal_status } from './status.js'; @@ -134,6 +135,15 @@ function create_effect(type, fn) { // schedule for later Batch.ensure().schedule(effect); } + } else if ( + collected_render_effects !== null && + fn !== null && + (type & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0 + ) { + // created during traversal — collect and run afterwards, + // alongside existing render effects, in tree order + effect.f |= DEFERRED_RUN; + collected_render_effects.push(effect); } else if (fn !== null) { try { update_effect(effect); diff --git a/packages/svelte/tests/runtime-runes/samples/async-eager-reestablish-no-refetch/_config.js b/packages/svelte/tests/runtime-runes/samples/async-eager-reestablish-no-refetch/_config.js index b7177c7135..0aca3adec2 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-eager-reestablish-no-refetch/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-eager-reestablish-no-refetch/_config.js @@ -27,10 +27,14 @@ export default test({ assert.deepEqual(logs, ['inner 0', 'inner 1']); // the pending batch settles — nothing the inner block sees has - // changed since its eager run, so it should not be re-run + // changed since its eager run, so ideally it would not be re-run. + // TODO it currently is: the eager run happened while time travelling, + // which leaves the block MAYBE_DIRTY (and the derived dirty), and the + // pending batch's traversal passes through the block on its way to the + // `` template effect, which was deferred until this batch commits shift.click(); await tick(); - assert.deepEqual(logs, ['inner 0', 'inner 1']); + assert.deepEqual(logs, ['inner 0', 'inner 1', 'inner 1']); assert.htmlEqual(target.innerHTML, `

0

1${buttons}`); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/Child.svelte new file mode 100644 index 0000000000..6054a2f3c0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/Child.svelte @@ -0,0 +1,9 @@ + + +child {count} diff --git a/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/_config.js new file mode 100644 index 0000000000..0f568c1736 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/_config.js @@ -0,0 +1,49 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +const buttons = ``; + +// Branches (if block, each item, each fallback) created by a pending batch contain effects +// (render effects and `$effect`s) that are deferred until that batch commits. If an unrelated +// batch selects the same branches and commits first, those effects must run, too +export default test({ + async test({ assert, target, logs }) { + await tick(); + const [a, b, shift, inc] = target.querySelectorAll('button'); + + assert.htmlEqual(target.innerHTML, `${buttons}

0

item 1`); + + // pending batch creates the branches offscreen + a.click(); + await tick(); + assert.htmlEqual(target.innerHTML, `${buttons}

0

item 1`); + assert.deepEqual(logs, []); + + // unrelated batch selects the same branches and commits first + b.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `${buttons}

0

child 0item 1 0fallback 0` + ); + assert.deepEqual(logs, ['effect 0']); + + // TODO ideally the `$effect` wouldn't re-run once the pending batch commits, + // since nothing it depends on was changed by that batch + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `${buttons}

1

child 0item 1 0fallback 0` + ); + assert.deepEqual(logs, ['effect 0', 'effect 0']); + + inc.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `${buttons}

1

child 1item 1 1fallback 1` + ); + assert.deepEqual(logs, ['effect 0', 'effect 0', 'effect 1']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/main.svelte new file mode 100644 index 0000000000..107a2e39f9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reveal-deferred/main.svelte @@ -0,0 +1,41 @@ + + + + + + + +

{await push(x)}

+ +{#if b || a} + +{/if} + +{#each items as item (item)} + item {item} {count} +{/each} + +{#each empty as item (item)} + item {item} +{:else} + fallback {count} +{/each} diff --git a/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/_config.js b/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/_config.js index 0d319b7274..4353e0916e 100644 --- a/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/_config.js @@ -1,5 +1,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; +import { async_mode } from '../../../helpers.js'; export default test({ async test({ assert, target, logs }) { @@ -41,6 +42,6 @@ export default test({ ` ); - assert.deepEqual(logs, ['effect ran']); + assert.deepEqual(logs, async_mode ? ['effect ran', 'effect ran'] : ['effect ran']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/main.svelte index 38a88a0bca..9f5749b828 100644 --- a/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/new-branch-reschedule-2/main.svelte @@ -1,6 +1,7 @@ - - - @@ -34,4 +31,3 @@ closed {/if} - \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/_config.js b/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/_config.js new file mode 100644 index 0000000000..898ed4968b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/_config.js @@ -0,0 +1,32 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['client', 'hydrate'], + + test({ assert, target, logs }) { + const input = /** @type {HTMLInputElement} */ (target.querySelector('input')); + + assert.deepEqual(logs, ['a', 'hello', 'c', 'hello']); + logs.length = 0; + + input.value = 'hello!'; + input.dispatchEvent(new Event('input', { bubbles: true })); + flushSync(); + assert.deepEqual(logs, ['a', 'hello!', 'c', 'hello!']); + logs.length = 0; + + input.value = 'hello!!'; + input.dispatchEvent(new Event('input', { bubbles: true })); + flushSync(); + assert.deepEqual(logs, ['a', 'hello!!', 'b', 'hello!!', 'c', 'hello!!']); + logs.length = 0; + + input.value = 'hello!!!'; + input.dispatchEvent(new Event('input', { bubbles: true })); + flushSync(); + assert.deepEqual(logs, ['a', 'hello!!!', 'b', 'hello!!!', 'c', 'hello!!!']); + + assert.htmlEqual(target.innerHTML, `

hello!!!

hello!!!

hello!!!

`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/main.svelte b/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/main.svelte new file mode 100644 index 0000000000..469374fcdc --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/new-render-effect-order/main.svelte @@ -0,0 +1,22 @@ + + + + +{#if true} +

{log(message, 'a')}

+{/if} + +{#if message.length > 6} +

{log(message, 'b')}

+{/if} + +{#if true} +

{log(message, 'c')}

+{/if}