breaking(experimental-async): defer render/pre effects created during traversal

Run them only once the batch is committed. Strictly speaking this is a breaking change in async mode, but it's experimental so it's fine.
As a consequence add a new `reveal` method to batch which collects these not-yet-run effects and schedules them in its own batch, too, in case it commits first. This should also help for our "fork only branches" case later.
Extracted from #18880
defer-render-effects
Simon Holthausen 1 day ago
parent 40bb08482f
commit d3df5e861f
No known key found for this signature in database

@ -0,0 +1,5 @@
---
'svelte': patch
---
breaking(experimental-async): run render/pre effects created during traversal alongside existing render effects, in tree order

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

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

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

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

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

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

@ -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
// `<span>` 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, `<p>0</p><span>1</span>${buttons}`);
}
});

@ -0,0 +1,9 @@
<script>
let { count } = $props();
$effect(() => {
console.log(`effect ${count}`);
});
</script>
<span>child {count}</span>

@ -0,0 +1,49 @@
import { tick } from 'svelte';
import { test } from '../../test';
const buttons = `<button>a</button><button>b</button><button>shift</button><button>inc</button>`;
// 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}<p>0</p><span>item 1</span>`);
// pending batch creates the branches offscreen
a.click();
await tick();
assert.htmlEqual(target.innerHTML, `${buttons}<p>0</p><span>item 1</span>`);
assert.deepEqual(logs, []);
// unrelated batch selects the same branches and commits first
b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`${buttons}<p>0</p><span>child 0</span><span>item 1 0</span><span>fallback 0</span>`
);
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}<p>1</p><span>child 0</span><span>item 1 0</span><span>fallback 0</span>`
);
assert.deepEqual(logs, ['effect 0', 'effect 0']);
inc.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`${buttons}<p>1</p><span>child 1</span><span>item 1 1</span><span>fallback 1</span>`
);
assert.deepEqual(logs, ['effect 0', 'effect 0', 'effect 1']);
}
});

@ -0,0 +1,41 @@
<script>
import Child from './Child.svelte';
let a = $state(false);
let b = $state(false);
let count = $state(0);
let x = $state(0);
let resolvers = [];
function push(v) {
if (v === 0) return v;
return new Promise((r) => resolvers.push(() => r(v)));
}
// `b || a` rather than `a || b`, so that the pending batch (which sets `a`) doesn't
// short-circuit and the blocks also depend on `b`, which is set by the unrelated batch
let items = $derived(b || a ? [1] : []);
let empty = $derived(b || a ? [] : [1]);
</script>
<button onclick={() => { a = true; x++; }}>a</button>
<button onclick={() => (b = true)}>b</button>
<button onclick={() => resolvers.shift()?.()}>shift</button>
<button onclick={() => count++}>inc</button>
<p>{await push(x)}</p>
{#if b || a}
<Child {count} />
{/if}
{#each items as item (item)}
<span>item {item} {count}</span>
{/each}
{#each empty as item (item)}
<span>item {item}</span>
{:else}
<span>fallback {count}</span>
{/each}

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

@ -1,6 +1,7 @@
<script module>
<script>
let active = $state(false);
let panelWidth = $state(null);
let counter = $state(0);
const store = {
get active() { return active; },
@ -19,10 +20,6 @@
};
</script>
<script>
let counter = $state(0);
</script>
<button onclick={() => store.open()}>Open</button>
<button onclick={() => store.close()}>Close</button>
<button onclick={() => counter++}>{counter}</button>
@ -34,4 +31,3 @@
closed
{/if}
</div>

@ -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, `<input /><p>hello!!!</p><p>hello!!!</p><p>hello!!!</p>`);
}
});

@ -0,0 +1,22 @@
<script>
let message = $state('hello');
function log(value, label) {
console.log(label, value);
return value;
}
</script>
<input bind:value={message} />
{#if true}
<p>{log(message, 'a')}</p>
{/if}
{#if message.length > 6}
<p>{log(message, 'b')}</p>
{/if}
{#if true}
<p>{log(message, 'c')}</p>
{/if}
Loading…
Cancel
Save