From 02b737224e925e22989438e98c5ea63fd07626b4 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Mon, 8 Sep 2025 17:12:34 -0600 Subject: [PATCH] fix: `$effect.pending` sends updates to incorrect boundary --- .../src/internal/client/dom/blocks/async.js | 4 +- .../internal/client/dom/blocks/boundary.js | 93 ++++++++++++++---- .../src/internal/client/reactivity/async.js | 4 +- .../src/internal/client/reactivity/batch.js | 5 +- .../async-effect-pending-nested/_config.js | 95 +++++++++++++++++++ .../async-effect-pending-nested/main.svelte | 34 +++++++ 6 files changed, 207 insertions(+), 28 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte diff --git a/packages/svelte/src/internal/client/dom/blocks/async.js b/packages/svelte/src/internal/client/dom/blocks/async.js index 82f107ab29..5ec50a5988 100644 --- a/packages/svelte/src/internal/client/dom/blocks/async.js +++ b/packages/svelte/src/internal/client/dom/blocks/async.js @@ -1,7 +1,7 @@ /** @import { TemplateNode, Value } from '#client' */ import { flatten } from '../../reactivity/async.js'; import { get } from '../../runtime.js'; -import { get_pending_boundary } from './boundary.js'; +import { get_boundary } from './boundary.js'; /** * @param {TemplateNode} node @@ -9,7 +9,7 @@ import { get_pending_boundary } from './boundary.js'; * @param {(anchor: TemplateNode, ...deriveds: Value[]) => void} fn */ export function async(node, expressions, fn) { - var boundary = get_pending_boundary(); + var boundary = get_boundary(); boundary.update_pending_count(1); diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 12ca547608..b36ac0deed 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -49,11 +49,34 @@ export function boundary(node, props, children) { } export class Boundary { - pending = false; - /** @type {Boundary | null} */ parent; + /** + * Whether this boundary is inside a boundary (including this one) that's showing a pending snippet. + * @type {boolean} + */ + get pending() { + if (this.has_pending_snippet()) { + return this.#pending; + } + + // intentionally not throwing here, as the answer to "am I in a pending snippet" is false when + // there's no pending snippet at all + return this.parent?.pending ?? false; + } + + set pending(value) { + if (this.has_pending_snippet()) { + this.#pending = value; + } else if (this.parent) { + this.parent.pending = value; + } else if (value) { + e.await_outside_boundary(); + } + // if we're trying to set it to `false` and yeeting that into the void, it's fine + } + /** @type {TemplateNode} */ #anchor; @@ -81,7 +104,28 @@ export class Boundary { /** @type {DocumentFragment | null} */ #offscreen_fragment = null; + /** + * Whether this boundary is inside a boundary (including this one) that's showing a pending snippet. + * Derived from {@link props.pending} and {@link cascading_pending_count}. + */ + #pending = false; + + /** + * The number of pending async deriveds/expressions within this boundary, not counting any parent or child boundaries. + * This controls `$effect.pending` for this boundary. + * + * Don't ever set this directly; use {@link update_pending_count} instead. + */ #pending_count = 0; + + /** + * Like {@link #pending_count}, but treats boundaries with no `pending` snippet as porous. + * This controls the pending snippet for this boundary. + * + * Don't ever set this directly; use {@link update_pending_count} instead. + */ + #cascading_pending_count = 0; + #is_creating_fallback = false; /** @@ -149,7 +193,7 @@ export class Boundary { return branch(() => this.#children(this.#anchor)); }); - if (this.#pending_count > 0) { + if (this.#cascading_pending_count > 0) { this.#show_pending_snippet(); } else { pause_effect(/** @type {Effect} */ (this.#pending_effect), () => { @@ -166,7 +210,7 @@ export class Boundary { this.error(error); } - if (this.#pending_count > 0) { + if (this.#cascading_pending_count > 0) { this.#show_pending_snippet(); } else { this.pending = false; @@ -220,11 +264,11 @@ export class Boundary { } } - /** @param {1 | -1} d */ - #update_pending_count(d) { - this.#pending_count += d; + /** @param {number} d */ + #update_cascading_pending_count(d) { + this.#cascading_pending_count = Math.max(this.#cascading_pending_count + d, 0); - if (this.#pending_count === 0) { + if (this.#cascading_pending_count === 0) { this.pending = false; if (this.#pending_effect) { @@ -240,12 +284,21 @@ export class Boundary { } } - /** @param {1 | -1} d */ - update_pending_count(d) { + /** + * @param {number} d + * @param {boolean} safe + */ + update_pending_count(d, safe = false, first = true) { + if (first) { + this.#pending_count = Math.max(this.#pending_count + d, 0); + } + if (this.has_pending_snippet()) { - this.#update_pending_count(d); + this.#update_cascading_pending_count(d); } else if (this.parent) { - this.parent.#update_pending_count(d); + this.parent.update_pending_count(d, safe, false); + } else if (this.parent === null && !safe) { + e.await_outside_boundary(); } effect_pending_updates.add(this.#effect_pending_update); @@ -300,7 +353,9 @@ export class Boundary { // If the failure happened while flushing effects, current_batch can be null Batch.ensure(); - this.#pending_count = 0; + // this ensures we modify the cascading_pending_count of the correct parent + // by the number we're decreasing this boundary by + this.update_pending_count(-this.#pending_count, true); if (this.#failed_effect !== null) { pause_effect(this.#failed_effect, () => { @@ -308,14 +363,16 @@ export class Boundary { }); } - this.pending = true; + // we intentionally do not try to find the nearest pending boundary. If this boundary has one, we'll render it on reset + // but it would be really weird to show the parent's boundary on a child reset. + this.pending = this.has_pending_snippet(); this.#main_effect = this.#run(() => { this.#is_creating_fallback = false; return branch(() => this.#children(this.#anchor)); }); - if (this.#pending_count > 0) { + if (this.#cascading_pending_count > 0) { this.#show_pending_snippet(); } else { this.pending = false; @@ -384,13 +441,9 @@ function move_effect(effect, fragment) { } } -export function get_pending_boundary() { +export function get_boundary() { var boundary = /** @type {Effect} */ (active_effect).b; - while (boundary !== null && !boundary.has_pending_snippet()) { - boundary = boundary.parent; - } - if (boundary === null) { e.await_outside_boundary(); } diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 65d004137f..f91f2e094a 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -3,7 +3,6 @@ import { DESTROYED } from '#client/constants'; import { DEV } from 'esm-env'; import { component_context, is_runes, set_component_context } from '../context.js'; -import { get_pending_boundary } from '../dom/blocks/boundary.js'; import { invoke_error_boundary } from '../error-handling.js'; import { active_effect, @@ -39,7 +38,6 @@ export function flatten(sync, async, fn) { var parent = /** @type {Effect} */ (active_effect); var restore = capture(); - var boundary = get_pending_boundary(); Promise.all(async.map((expression) => async_derived(expression))) .then((result) => { @@ -60,7 +58,7 @@ export function flatten(sync, async, fn) { unset_context(); }) .catch((error) => { - boundary.error(error); + invoke_error_boundary(error, parent); }); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 82f1de67a9..fd5c34ab26 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -10,12 +10,11 @@ import { INERT, RENDER_EFFECT, ROOT_EFFECT, - USER_EFFECT, MAYBE_DIRTY } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property } from '../../shared/utils.js'; -import { get_pending_boundary } from '../dom/blocks/boundary.js'; +import { get_boundary } from '../dom/blocks/boundary.js'; import { active_effect, is_dirty, @@ -668,7 +667,7 @@ export function schedule_effect(signal) { } export function suspend() { - var boundary = get_pending_boundary(); + var boundary = get_boundary(); var batch = /** @type {Batch} */ (current_batch); var pending = boundary.pending; diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js b/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js new file mode 100644 index 0000000000..9fe354bac0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js @@ -0,0 +1,95 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [increment, shift] = target.querySelectorAll('button'); + + assert.htmlEqual( + target.innerHTML, + ` + + +

loading...

+ ` + ); + + shift.click(); + shift.click(); + shift.click(); + + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

0

+

0

+

0

+

inner pending: 0

+

outer pending: 0

+ ` + ); + + increment.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

0

+

0

+

0

+

inner pending: 3

+

outer pending: 0

+ ` + ); + + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

0

+

0

+

0

+

inner pending: 2

+

outer pending: 0

+ ` + ); + + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

0

+

0

+

0

+

inner pending: 1

+

outer pending: 0

+ ` + ); + + shift.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

1

+

1

+

1

+

inner pending: 0

+

outer pending: 0

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte new file mode 100644 index 0000000000..eeafbdc3c4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte @@ -0,0 +1,34 @@ + + + + + + + +

{await push(value)}

+

{await push(value)}

+

{await push(value)}

+

inner pending: {$effect.pending()}

+
+

outer pending: {$effect.pending()}

+ + {#snippet pending()} +

loading...

+ {/snippet} +
+ +