chore: proactively defer effects in pending boundary (#17734)

Currently, (render/template) effects inside pending boundaries are
deferred, but in an indirect manner: first we schedule them, then we
`flush` the current batch, and in the course of traversing the effect
tree we find any dirty effects and defer them at the level of the
topmost pending boundary.

This doesn't really make sense — we can just skip to the end state and
skip the scheduling/traversal, since the effects don't become relevant
until the boundary resolves.

This PR implements that. It is a stepping stone towards a larger
refactor, in which scheduling becomes batch-centric and lazier. While it
shouldn't change any observable behaviour, I've added a changeset out of
an abundance of caution.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [ ] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`
pull/17733/head
Rich Harris 6 days ago committed by GitHub
parent 2287ad005a
commit c83aa06d69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
chore: proactively defer effects in pending boundary

@ -1,7 +1,6 @@
/** @import { Effect, Source, TemplateNode, } from '#client' */
import {
BOUNDARY_EFFECT,
COMMENT_NODE,
DIRTY,
EFFECT_PRESERVED,
EFFECT_TRANSPARENT,
@ -202,7 +201,7 @@ export class Boundary {
this.#pending_effect = null;
});
this.is_pending = false;
this.#resolve();
}
});
}
@ -224,13 +223,33 @@ export class Boundary {
const pending = /** @type {(anchor: Node) => void} */ (this.#props.pending);
this.#pending_effect = branch(() => pending(this.#anchor));
} else {
this.is_pending = false;
this.#resolve();
}
} catch (error) {
this.error(error);
}
}
#resolve() {
this.is_pending = false;
// any effects that were previously deferred should be rescheduled —
// after the next traversal (which will happen immediately, due to the
// same update that brought us here) the effects will be flushed
for (const e of this.#dirty_effects) {
set_signal_status(e, DIRTY);
schedule_effect(e);
}
for (const e of this.#maybe_dirty_effects) {
set_signal_status(e, MAYBE_DIRTY);
schedule_effect(e);
}
this.#dirty_effects.clear();
this.#maybe_dirty_effects.clear();
}
/**
* Defer an effect inside a pending boundary until the boundary resolves
* @param {Effect} effect
@ -294,24 +313,7 @@ export class Boundary {
this.#pending_count += d;
if (this.#pending_count === 0) {
this.is_pending = false;
// any effects that were encountered and deferred during traversal
// should be rescheduled — after the next traversal (which will happen
// immediately, due to the same update that brought us here)
// the effects will be flushed
for (const e of this.#dirty_effects) {
set_signal_status(e, DIRTY);
schedule_effect(e);
}
for (const e of this.#maybe_dirty_effects) {
set_signal_status(e, MAYBE_DIRTY);
schedule_effect(e);
}
this.#dirty_effects.clear();
this.#maybe_dirty_effects.clear();
this.#resolve();
if (this.#pending_effect) {
pause_effect(this.#pending_effect, () => {

@ -18,7 +18,8 @@ import {
EAGER_EFFECT,
HEAD_EFFECT,
ERROR_VALUE,
MANAGED_EFFECT
MANAGED_EFFECT,
REACTION_RAN
} from '#client/constants';
import { async_mode_flag } from '../../flags/index.js';
import { deferred, define_property, includes } from '../../shared/utils.js';
@ -246,9 +247,6 @@ export class Batch {
var effect = root.first;
/** @type {Effect | null} */
var pending_boundary = null;
while (effect !== null) {
var flags = effect.f;
var is_branch = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0;
@ -256,26 +254,9 @@ export class Batch {
var skip = is_skippable_branch || (flags & INERT) !== 0 || this.#skipped_branches.has(effect);
// Inside a `<svelte:boundary>` with a pending snippet,
// all effects are deferred until the boundary resolves
// (except block/async effects, which run immediately)
if (
async_mode_flag &&
pending_boundary === null &&
(flags & BOUNDARY_EFFECT) !== 0 &&
effect.b?.is_pending
) {
pending_boundary = effect;
}
if (!skip && effect.fn !== null) {
if (is_branch) {
effect.f ^= CLEAN;
} else if (
pending_boundary !== null &&
(flags & (EFFECT | RENDER_EFFECT | MANAGED_EFFECT)) !== 0
) {
/** @type {Boundary} */ (pending_boundary.b).defer_effect(effect);
} else if ((flags & EFFECT) !== 0) {
effects.push(effect);
} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
@ -294,10 +275,6 @@ export class Batch {
}
while (effect !== null) {
if (effect === pending_boundary) {
pending_boundary = null;
}
var next = effect.next;
if (next !== null) {
@ -839,6 +816,19 @@ function depends_on(reaction, sources, checked) {
export function schedule_effect(signal) {
var effect = (last_scheduled_effect = signal);
var boundary = effect.b;
// defer render effects inside a pending boundary
// TODO the `REACTION_RAN` check is only necessary because of legacy `$:` effects AFAICT — we can remove later
if (
boundary?.is_pending &&
(signal.f & (EFFECT | RENDER_EFFECT | MANAGED_EFFECT)) !== 0 &&
(signal.f & REACTION_RAN) === 0
) {
boundary.defer_effect(signal);
return;
}
while (effect.parent !== null) {
effect = effect.parent;
var flags = effect.f;
@ -850,13 +840,18 @@ export function schedule_effect(signal) {
is_flushing &&
effect === active_effect &&
(flags & BLOCK_EFFECT) !== 0 &&
(flags & HEAD_EFFECT) === 0
(flags & HEAD_EFFECT) === 0 &&
(flags & REACTION_RAN) !== 0
) {
return;
}
if ((flags & (ROOT_EFFECT | BRANCH_EFFECT)) !== 0) {
if ((flags & CLEAN) === 0) return;
if ((flags & CLEAN) === 0) {
// branch is already dirty, bail
return;
}
effect.f ^= CLEAN;
}
}

Loading…
Cancel
Save