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`

Author: Rich-Harris

.changeset/angry-ideas-listen.md

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

packages/svelte/src/internal/client/dom/blocks/boundary.js

@@ -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, () => {

packages/svelte/src/internal/client/reactivity/batch.js

@@ -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,36 +247,16 @@ 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;
 			var is_skippable_branch = is_branch && (flags & CLEAN) !== 0;
 
 			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;
 		}
 	}