msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: `$effect.pending` sends updates to incorrect boundary

Browse Source
elliott/effect-pending-correct-boundary
S. Elliott Johnson 7 days ago
parent d92fa432d1
commit 02b737224e
6 changed files with 207 additions and 28 deletions
Show Stats Download Patch File Download Diff File

4
packages/svelte/src/internal/client/dom/blocks/async.js
Unescape View File

@ -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);

93
packages/svelte/src/internal/client/dom/blocks/boundary.js
Unescape View File

@ -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();
}

4
packages/svelte/src/internal/client/reactivity/async.js
Unescape View File

@ -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);
});
}

5
packages/svelte/src/internal/client/reactivity/batch.js
Unescape View File

@ -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;

95
packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js
Unescape View File

@ -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>
`
);
}
});

34
packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte
Unescape View File

@ -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>
Write Preview
Loading…
Cancel
Save