diff --git a/.changeset/some-teams-pay.md b/.changeset/some-teams-pay.md new file mode 100644 index 0000000000..d7ae69566d --- /dev/null +++ b/.changeset/some-teams-pay.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid erroneous async derived expressions for blocks diff --git a/.changeset/yummy-insects-wonder.md b/.changeset/yummy-insects-wonder.md new file mode 100644 index 0000000000..6e705cd8cf --- /dev/null +++ b/.changeset/yummy-insects-wonder.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: reschedule effects inside unskipped branches diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index c0bfe272e5..1dbc34fdc3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -312,10 +312,10 @@ export function EachBlock(node, context) { declarations.push(b.let(node.index, index)); } - const is_async = node.metadata.expression.is_async(); + const has_await = node.metadata.expression.has_await; - const get_collection = b.thunk(collection, node.metadata.expression.has_await); - const thunk = is_async ? b.thunk(b.call('$.get', b.id('$$collection'))) : get_collection; + const get_collection = b.thunk(collection, has_await); + const thunk = has_await ? b.thunk(b.call('$.get', b.id('$$collection'))) : get_collection; const render_args = [b.id('$$anchor'), item]; if (uses_index || collection_id) render_args.push(index); @@ -342,15 +342,18 @@ export function EachBlock(node, context) { statements.unshift(b.stmt(b.call('$.validate_each_keys', thunk, key_function))); } - if (is_async) { + if (node.metadata.expression.is_async()) { context.state.init.push( b.stmt( b.call( '$.async', context.state.node, node.metadata.expression.blockers(), - b.array([get_collection]), - b.arrow([context.state.node, b.id('$$collection')], b.block(statements)) + has_await ? b.array([get_collection]) : b.void0, + b.arrow( + has_await ? [context.state.node, b.id('$$collection')] : [context.state.node], + b.block(statements) + ) ) ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js index 0567edc610..2706cf7f0a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js @@ -11,10 +11,11 @@ import { build_expression } from './shared/utils.js'; export function HtmlTag(node, context) { context.state.template.push_comment(); - const is_async = node.metadata.expression.is_async(); + const has_await = node.metadata.expression.has_await; + const has_blockers = node.metadata.expression.has_blockers(); const expression = build_expression(context, node.expression, node.metadata.expression); - const html = is_async ? b.call('$.get', b.id('$$html')) : expression; + const html = has_await ? b.call('$.get', b.id('$$html')) : expression; const is_svg = context.state.metadata.namespace === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; @@ -31,15 +32,18 @@ export function HtmlTag(node, context) { ); // push into init, so that bindings run afterwards, which might trigger another run and override hydration - if (is_async) { + if (has_await || has_blockers) { context.state.init.push( b.stmt( b.call( '$.async', context.state.node, node.metadata.expression.blockers(), - b.array([b.thunk(expression, node.metadata.expression.has_await)]), - b.arrow([context.state.node, b.id('$$html')], b.block([statement])) + has_await ? b.array([b.thunk(expression, true)]) : b.void0, + b.arrow( + has_await ? [context.state.node, b.id('$$html')] : [context.state.node], + b.block([statement]) + ) ) ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index fcbb59ba74..c0e66635df 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -25,10 +25,11 @@ export function IfBlock(node, context) { statements.push(b.var(alternate_id, b.arrow([b.id('$$anchor')], alternate))); } - const is_async = node.metadata.expression.is_async(); + const has_await = node.metadata.expression.has_await; + const has_blockers = node.metadata.expression.has_blockers(); const expression = build_expression(context, node.test, node.metadata.expression); - const test = is_async ? b.call('$.get', b.id('$$condition')) : expression; + const test = has_await ? b.call('$.get', b.id('$$condition')) : expression; /** @type {Expression[]} */ const args = [ @@ -72,15 +73,18 @@ export function IfBlock(node, context) { statements.push(add_svelte_meta(b.call('$.if', ...args), node, 'if')); - if (is_async) { + if (has_await || has_blockers) { context.state.init.push( b.stmt( b.call( '$.async', context.state.node, node.metadata.expression.blockers(), - b.array([b.thunk(expression, node.metadata.expression.has_await)]), - b.arrow([context.state.node, b.id('$$condition')], b.block(statements)) + has_await ? b.array([b.thunk(expression, true)]) : b.void0, + b.arrow( + has_await ? [context.state.node, b.id('$$condition')] : [context.state.node], + b.block(statements) + ) ) ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js index d050155e8b..143a4e8edd 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js @@ -11,29 +11,35 @@ import { build_expression, add_svelte_meta } from './shared/utils.js'; export function KeyBlock(node, context) { context.state.template.push_comment(); - const is_async = node.metadata.expression.is_async(); + const has_await = node.metadata.expression.has_await; + const has_blockers = node.metadata.expression.has_blockers(); const expression = build_expression(context, node.expression, node.metadata.expression); - const key = b.thunk(is_async ? b.call('$.get', b.id('$$key')) : expression); + const key = b.thunk(has_await ? b.call('$.get', b.id('$$key')) : expression); const body = /** @type {Expression} */ (context.visit(node.fragment)); - let statement = add_svelte_meta( + const statement = add_svelte_meta( b.call('$.key', context.state.node, key, b.arrow([b.id('$$anchor')], body)), node, 'key' ); - if (is_async) { - statement = b.stmt( - b.call( - '$.async', - context.state.node, - node.metadata.expression.blockers(), - b.array([b.thunk(expression, node.metadata.expression.has_await)]), - b.arrow([context.state.node, b.id('$$key')], b.block([statement])) + if (has_await || has_blockers) { + context.state.init.push( + b.stmt( + b.call( + '$.async', + context.state.node, + node.metadata.expression.blockers(), + has_await ? b.array([b.thunk(expression, true)]) : b.void0, + b.arrow( + has_await ? [context.state.node, b.id('$$key')] : [context.state.node], + b.block([statement]) + ) + ) ) ); + } else { + context.state.init.push(statement); } - - context.state.init.push(statement); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index c8192cf00a..10024298fa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -93,10 +93,11 @@ export function SvelteElement(node, context) { ); } - const is_async = node.metadata.expression.is_async(); + const has_await = node.metadata.expression.has_await; + const has_blockers = node.metadata.expression.has_blockers(); const expression = /** @type {Expression} */ (context.visit(node.tag)); - const get_tag = b.thunk(is_async ? b.call('$.get', b.id('$$tag')) : expression); + const get_tag = b.thunk(has_await ? b.call('$.get', b.id('$$tag')) : expression); /** @type {Statement[]} */ const inner = inner_context.state.init; @@ -139,15 +140,18 @@ export function SvelteElement(node, context) { ) ); - if (is_async) { + if (has_await || has_blockers) { context.state.init.push( b.stmt( b.call( '$.async', context.state.node, node.metadata.expression.blockers(), - b.array([b.thunk(expression, node.metadata.expression.has_await)]), - b.arrow([context.state.node, b.id('$$tag')], b.block(statements)) + has_await ? b.array([b.thunk(expression, true)]) : b.void0, + b.arrow( + has_await ? [context.state.node, b.id('$$tag')] : [context.state.node], + b.block(statements) + ) ) ) ); diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index 527f0b0a8f..6b77903574 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -200,17 +200,17 @@ export class BranchManager { if (defer) { for (const [k, effect] of this.#onscreen) { if (k === key) { - batch.skipped_effects.delete(effect); + batch.unskip_effect(effect); } else { - batch.skipped_effects.add(effect); + batch.skip_effect(effect); } } for (const [k, branch] of this.#offscreen) { if (k === key) { - batch.skipped_effects.delete(branch.effect); + batch.unskip_effect(branch.effect); } else { - batch.skipped_effects.add(branch.effect); + batch.skip_effect(branch.effect); } } diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 232656ec11..6eaeac0f38 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -257,7 +257,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (item.i) internal_set(item.i, index); if (defer) { - batch.skipped_effects.delete(item.e); + batch.unskip_effect(item.e); } } else { item = create_item( @@ -299,7 +299,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (defer) { for (const [key, item] of items) { if (!keys.has(key)) { - batch.skipped_effects.add(item.e); + batch.skip_effect(item.e); } } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 2b6e84889b..cef2df4716 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -130,11 +130,13 @@ export class Batch { #maybe_dirty_effects = new Set(); /** - * A set of branches that still exist, but will be destroyed when this batch - * is committed — we skip over these during `process` - * @type {Set} + * A map of branches that still exist, but will be destroyed when this batch + * is committed — we skip over these during `process`. + * The value contains child effects that were dirty/maybe_dirty before being reset, + * so they can be rescheduled if the branch survives. + * @type {Map} */ - skipped_effects = new Set(); + #skipped_branches = new Map(); is_fork = false; @@ -144,6 +146,38 @@ export class Batch { return this.is_fork || this.#blocking_pending > 0; } + /** + * Add an effect to the #skipped_branches map and reset its children + * @param {Effect} effect + */ + skip_effect(effect) { + if (!this.#skipped_branches.has(effect)) { + this.#skipped_branches.set(effect, { d: [], m: [] }); + } + } + + /** + * Remove an effect from the #skipped_branches map and reschedule + * any tracked dirty/maybe_dirty child effects + * @param {Effect} effect + */ + unskip_effect(effect) { + var tracked = this.#skipped_branches.get(effect); + if (tracked) { + this.#skipped_branches.delete(effect); + + for (var e of tracked.d) { + set_signal_status(e, DIRTY); + schedule_effect(e); + } + + for (e of tracked.m) { + set_signal_status(e, MAYBE_DIRTY); + schedule_effect(e); + } + } + } + /** * * @param {Effect[]} root_effects @@ -172,8 +206,8 @@ export class Batch { this.#defer_effects(render_effects); this.#defer_effects(effects); - for (const e of this.skipped_effects) { - reset_branch(e); + for (const [e, t] of this.#skipped_branches) { + reset_branch(e, t); } } else { // append/remove branches @@ -220,7 +254,7 @@ export class Batch { var is_branch = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0; var is_skippable_branch = is_branch && (flags & CLEAN) !== 0; - var skip = is_skippable_branch || (flags & INERT) !== 0 || this.skipped_effects.has(effect); + var skip = is_skippable_branch || (flags & INERT) !== 0 || this.#skipped_branches.has(effect); // Inside a `` with a pending snippet, // all effects are deferred until the boundary resolves @@ -807,7 +841,8 @@ export function schedule_effect(signal) { var flags = effect.f; // if the effect is being scheduled because a parent (each/await/etc) block - // updated an internal source, bail out or we'll cause a second flush + // updated an internal source, or because a branch is being unskipped, + // bail out or we'll cause a second flush if ( is_flushing && effect === active_effect && @@ -887,20 +922,28 @@ export function eager(fn) { /** * Mark all the effects inside a skipped branch CLEAN, so that - * they can be correctly rescheduled later + * they can be correctly rescheduled later. Tracks dirty and maybe_dirty + * effects so they can be rescheduled if the branch survives. * @param {Effect} effect + * @param {{ d: Effect[], m: Effect[] }} tracked */ -function reset_branch(effect) { +function reset_branch(effect, tracked) { // clean branch = nothing dirty inside, no need to traverse further if ((effect.f & BRANCH_EFFECT) !== 0 && (effect.f & CLEAN) !== 0) { return; } + if ((effect.f & DIRTY) !== 0) { + tracked.d.push(effect); + } else if ((effect.f & MAYBE_DIRTY) !== 0) { + tracked.m.push(effect); + } + set_signal_status(effect, CLEAN); var e = effect.first; while (e !== null) { - reset_branch(e); + reset_branch(e, tracked); e = e.next; } } diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index 13975c68ee..8c29a6ada2 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -47,9 +47,9 @@ export interface RuntimeTest = Record; /** Temporarily skip specific modes, without skipping the entire test */ skip_mode?: Array<'server' | 'async-server' | 'client' | 'hydrate'>; - /** Skip if running with process.env.NO_ASYNC */ + /** Skip if running with process.env.SVELTE_NO_ASYNC */ skip_no_async?: boolean; - /** Skip if running without process.env.NO_ASYNC */ + /** Skip if running without process.env.SVELTE_NO_ASYNC */ skip_async?: boolean; html?: string; ssrHtml?: string; diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/Child.svelte new file mode 100644 index 0000000000..fecbe222e3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/Child.svelte @@ -0,0 +1,9 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/_config.js new file mode 100644 index 0000000000..2f371bc6b7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/_config.js @@ -0,0 +1,16 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + await tick(); + + assert.deepEqual(logs, ['promise resolved with:', 'some-id']); + + const button = target.querySelector('button'); + button?.click(); + await tick(); + + assert.deepEqual(logs, ['promise resolved with:', 'some-id']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/main.svelte new file mode 100644 index 0000000000..d5cdd5967e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unmount-undefined-props/main.svelte @@ -0,0 +1,14 @@ + + +{#if active} + +{/if} + +