fix: send `$effect.pending` count to the correct boundary (#16732)

* fix: send `$effect.pending` count to the correct boundary

* make boundary.pending private, use boundary.is_pending consistently

* move error to correct place

* we need that error

* update JSDoc
pull/16734/head
Rich Harris 4 weeks ago committed by GitHub
parent d92fa432d1
commit 18dd45640b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: send `$effect.pending` count to the correct boundary

@ -1,7 +1,7 @@
/** @import { TemplateNode, Value } from '#client' */ /** @import { TemplateNode, Value } from '#client' */
import { flatten } from '../../reactivity/async.js'; import { flatten } from '../../reactivity/async.js';
import { get } from '../../runtime.js'; import { get } from '../../runtime.js';
import { get_pending_boundary } from './boundary.js'; import { get_boundary } from './boundary.js';
/** /**
* @param {TemplateNode} node * @param {TemplateNode} node
@ -9,7 +9,7 @@ import { get_pending_boundary } from './boundary.js';
* @param {(anchor: TemplateNode, ...deriveds: Value[]) => void} fn * @param {(anchor: TemplateNode, ...deriveds: Value[]) => void} fn
*/ */
export function async(node, expressions, fn) { export function async(node, expressions, fn) {
var boundary = get_pending_boundary(); var boundary = get_boundary();
boundary.update_pending_count(1); boundary.update_pending_count(1);

@ -49,11 +49,11 @@ export function boundary(node, props, children) {
} }
export class Boundary { export class Boundary {
pending = false;
/** @type {Boundary | null} */ /** @type {Boundary | null} */
parent; parent;
#pending = false;
/** @type {TemplateNode} */ /** @type {TemplateNode} */
#anchor; #anchor;
@ -81,6 +81,7 @@ export class Boundary {
/** @type {DocumentFragment | null} */ /** @type {DocumentFragment | null} */
#offscreen_fragment = null; #offscreen_fragment = null;
#local_pending_count = 0;
#pending_count = 0; #pending_count = 0;
#is_creating_fallback = false; #is_creating_fallback = false;
@ -95,12 +96,12 @@ export class Boundary {
#effect_pending_update = () => { #effect_pending_update = () => {
if (this.#effect_pending) { if (this.#effect_pending) {
internal_set(this.#effect_pending, this.#pending_count); internal_set(this.#effect_pending, this.#local_pending_count);
} }
}; };
#effect_pending_subscriber = createSubscriber(() => { #effect_pending_subscriber = createSubscriber(() => {
this.#effect_pending = source(this.#pending_count); this.#effect_pending = source(this.#local_pending_count);
if (DEV) { if (DEV) {
tag(this.#effect_pending, '$effect.pending()'); tag(this.#effect_pending, '$effect.pending()');
@ -125,7 +126,7 @@ export class Boundary {
this.parent = /** @type {Effect} */ (active_effect).b; this.parent = /** @type {Effect} */ (active_effect).b;
this.pending = !!this.#props.pending; this.#pending = !!this.#props.pending;
this.#effect = block(() => { this.#effect = block(() => {
/** @type {Effect} */ (active_effect).b = this; /** @type {Effect} */ (active_effect).b = this;
@ -156,7 +157,7 @@ export class Boundary {
this.#pending_effect = null; this.#pending_effect = null;
}); });
this.pending = false; this.#pending = false;
} }
}); });
} else { } else {
@ -169,7 +170,7 @@ export class Boundary {
if (this.#pending_count > 0) { if (this.#pending_count > 0) {
this.#show_pending_snippet(); this.#show_pending_snippet();
} else { } else {
this.pending = false; this.#pending = false;
} }
} }
}, flags); }, flags);
@ -179,6 +180,14 @@ export class Boundary {
} }
} }
/**
* Returns `true` if the effect exists inside a boundary whose pending snippet is shown
* @returns {boolean}
*/
is_pending() {
return this.#pending || (!!this.parent && this.parent.is_pending());
}
has_pending_snippet() { has_pending_snippet() {
return !!this.#props.pending; return !!this.#props.pending;
} }
@ -220,12 +229,25 @@ export class Boundary {
} }
} }
/** @param {1 | -1} d */ /**
* Updates the pending count associated with the currently visible pending snippet,
* if any, such that we can replace the snippet with content once work is done
* @param {1 | -1} d
*/
#update_pending_count(d) { #update_pending_count(d) {
if (!this.has_pending_snippet()) {
if (this.parent) {
this.parent.#update_pending_count(d);
return;
}
e.await_outside_boundary();
}
this.#pending_count += d; this.#pending_count += d;
if (this.#pending_count === 0) { if (this.#pending_count === 0) {
this.pending = false; this.#pending = false;
if (this.#pending_effect) { if (this.#pending_effect) {
pause_effect(this.#pending_effect, () => { pause_effect(this.#pending_effect, () => {
@ -240,14 +262,16 @@ export class Boundary {
} }
} }
/** @param {1 | -1} d */ /**
* Update the source that powers `$effect.pending()` inside this boundary,
* and controls when the current `pending` snippet (if any) is removed.
* Do not call from inside the class
* @param {1 | -1} d
*/
update_pending_count(d) { update_pending_count(d) {
if (this.has_pending_snippet()) {
this.#update_pending_count(d); this.#update_pending_count(d);
} else if (this.parent) {
this.parent.#update_pending_count(d);
}
this.#local_pending_count += d;
effect_pending_updates.add(this.#effect_pending_update); effect_pending_updates.add(this.#effect_pending_update);
} }
@ -308,7 +332,7 @@ export class Boundary {
}); });
} }
this.pending = true; this.#pending = true;
this.#main_effect = this.#run(() => { this.#main_effect = this.#run(() => {
this.#is_creating_fallback = false; this.#is_creating_fallback = false;
@ -318,7 +342,7 @@ export class Boundary {
if (this.#pending_count > 0) { if (this.#pending_count > 0) {
this.#show_pending_snippet(); this.#show_pending_snippet();
} else { } else {
this.pending = false; this.#pending = false;
} }
}; };
@ -384,12 +408,8 @@ function move_effect(effect, fragment) {
} }
} }
export function get_pending_boundary() { export function get_boundary() {
var boundary = /** @type {Effect} */ (active_effect).b; const boundary = /** @type {Effect} */ (active_effect).b;
while (boundary !== null && !boundary.has_pending_snippet()) {
boundary = boundary.parent;
}
if (boundary === null) { if (boundary === null) {
e.await_outside_boundary(); e.await_outside_boundary();

@ -3,7 +3,7 @@
import { DESTROYED } from '#client/constants'; import { DESTROYED } from '#client/constants';
import { DEV } from 'esm-env'; import { DEV } from 'esm-env';
import { component_context, is_runes, set_component_context } from '../context.js'; import { component_context, is_runes, set_component_context } from '../context.js';
import { get_pending_boundary } from '../dom/blocks/boundary.js'; import { get_boundary } from '../dom/blocks/boundary.js';
import { invoke_error_boundary } from '../error-handling.js'; import { invoke_error_boundary } from '../error-handling.js';
import { import {
active_effect, active_effect,
@ -39,7 +39,7 @@ export function flatten(sync, async, fn) {
var parent = /** @type {Effect} */ (active_effect); var parent = /** @type {Effect} */ (active_effect);
var restore = capture(); var restore = capture();
var boundary = get_pending_boundary(); var boundary = get_boundary();
Promise.all(async.map((expression) => async_derived(expression))) Promise.all(async.map((expression) => async_derived(expression)))
.then((result) => { .then((result) => {

@ -15,7 +15,7 @@ import {
} from '#client/constants'; } from '#client/constants';
import { async_mode_flag } from '../../flags/index.js'; import { async_mode_flag } from '../../flags/index.js';
import { deferred, define_property } from '../../shared/utils.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 { import {
active_effect, active_effect,
is_dirty, is_dirty,
@ -298,7 +298,10 @@ export class Batch {
this.#render_effects.push(effect); this.#render_effects.push(effect);
} else if ((flags & CLEAN) === 0) { } else if ((flags & CLEAN) === 0) {
if ((flags & ASYNC) !== 0) { if ((flags & ASYNC) !== 0) {
var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects; var effects = effect.b?.is_pending()
? this.#boundary_async_effects
: this.#async_effects;
effects.push(effect); effects.push(effect);
} else if (is_dirty(effect)) { } else if (is_dirty(effect)) {
if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect); if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect);
@ -668,9 +671,9 @@ export function schedule_effect(signal) {
} }
export function suspend() { export function suspend() {
var boundary = get_pending_boundary(); var boundary = get_boundary();
var batch = /** @type {Batch} */ (current_batch); var batch = /** @type {Batch} */ (current_batch);
var pending = boundary.pending; var pending = boundary.is_pending();
boundary.update_pending_count(1); boundary.update_pending_count(1);
if (!pending) batch.increment(); if (!pending) batch.increment();

@ -135,7 +135,7 @@ export function async_derived(fn, location) {
prev = promise; prev = promise;
var batch = /** @type {Batch} */ (current_batch); var batch = /** @type {Batch} */ (current_batch);
var pending = boundary.pending; var pending = boundary.is_pending();
if (should_suspend) { if (should_suspend) {
boundary.update_pending_count(1); boundary.update_pending_count(1);

@ -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,
`
<button>increment</button>
<button>shift</button>
<p>loading...</p>
`
);
shift.click();
shift.click();
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<button>shift</button>
<p>0</p>
<p>0</p>
<p>0</p>
<p>inner pending: 0</p>
<p>outer pending: 0</p>
`
);
increment.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<button>shift</button>
<p>0</p>
<p>0</p>
<p>0</p>
<p>inner pending: 3</p>
<p>outer pending: 0</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<button>shift</button>
<p>0</p>
<p>0</p>
<p>0</p>
<p>inner pending: 2</p>
<p>outer pending: 0</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<button>shift</button>
<p>0</p>
<p>0</p>
<p>0</p>
<p>inner pending: 1</p>
<p>outer pending: 0</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<button>shift</button>
<p>1</p>
<p>1</p>
<p>1</p>
<p>inner pending: 0</p>
<p>outer pending: 0</p>
`
);
}
});

@ -0,0 +1,34 @@
<script>
let value = $state(0);
let deferreds = [];
function push(value) {
const deferred = Promise.withResolvers();
deferreds.push({ value, deferred });
return deferred.promise;
}
function shift() {
const d = deferreds.shift();
d?.deferred.resolve(d.value);
}
</script>
<button onclick={() => value++}>increment</button>
<button onclick={() => shift()}>shift</button>
<svelte:boundary>
<svelte:boundary>
<p>{await push(value)}</p>
<p>{await push(value)}</p>
<p>{await push(value)}</p>
<p>inner pending: {$effect.pending()}</p>
</svelte:boundary>
<p>outer pending: {$effect.pending()}</p>
{#snippet pending()}
<p>loading...</p>
{/snippet}
</svelte:boundary>
Loading…
Cancel
Save