merge

.changeset/angry-ideas-listen.md

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

.changeset/volatile-each-key.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: detect and error on non-idempotent each block keys in dev mode

documentation/docs/98-reference/.generated/client-errors.md

@@ -62,6 +62,14 @@ Keyed each block has duplicate key at indexes %a% and %b%
 Keyed each block has duplicate key `%value%` at indexes %a% and %b%
 ```
 
+### each_key_volatile
+
+```
+Keyed each block has key that is not idempotent — the key for item at index %index% was `%a%` but is now `%b%`. Keys must be the same each time for a given item
+```
+
+The key expression in a keyed each block must return the same value when called multiple times for the same item. Using expressions like `[item.a, item.b]` creates a new array each time, which will never be equal to itself. Instead, use a primitive value or create a stable key like `item.a + '-' + item.b`.
+
 ### effect_in_teardown
 
 ```

packages/svelte/messages/client-errors/errors.md

@@ -42,6 +42,12 @@ See the [migration guide](/docs/svelte/v5-migration-guide#Components-are-no-long
 
 > Keyed each block has duplicate key `%value%` at indexes %a% and %b%
 
+## each_key_volatile
+
+> Keyed each block has key that is not idempotent — the key for item at index %index% was `%a%` but is now `%b%`. Keys must be the same each time for a given item
+
+The key expression in a keyed each block must return the same value when called multiple times for the same item. Using expressions like `[item.a, item.b]` creates a new array each time, which will never be equal to itself. Instead, use a primitive value or create a stable key like `item.a + '-' + item.b`.
+
 ## effect_in_teardown
 
 > `%rune%` cannot be used inside an effect cleanup function

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

@@ -1,6 +1,5 @@
 /** @import { Blocker, TemplateNode, Value } from '#client' */
-import { flatten } from '../../reactivity/async.js';
-import { Batch, current_batch } from '../../reactivity/batch.js';
+import { flatten, increment_pending } from '../../reactivity/async.js';
 import { get } from '../../runtime.js';
 import {
 	hydrate_next,
@@ -10,7 +9,6 @@ import {
 	set_hydrating,
 	skip_nodes
 } from '../hydration.js';
-import { get_boundary } from './boundary.js';
 
 /**
  * @param {TemplateNode} node
@@ -44,12 +42,7 @@ export function async(node, blockers = [], expressions = [], fn) {
 		return;
 	}
 
-	var boundary = get_boundary();
-	var batch = /** @type {Batch} */ (current_batch);
-	var blocking = boundary.is_rendered();
-
-	boundary.update_pending_count(1);
-	batch.increment(blocking);
+	const decrement_pending = increment_pending();
 
 	if (was_hydrating) {
 		var previous_hydrate_node = hydrate_node;
@@ -72,8 +65,7 @@ export function async(node, blockers = [], expressions = [], fn) {
 				set_hydrating(false);
 			}
 
-			boundary.update_pending_count(-1);
-			batch.decrement(blocking);
+			decrement_pending();
 		}
 	});
 }

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,
@@ -206,7 +205,7 @@ export class Boundary {
 					this.#pending_effect = null;
 				});
 
-				this.is_pending = false;
+				this.#resolve();
 			}
 		});
 	}
@@ -228,13 +227,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
@@ -280,27 +299,6 @@ export class Boundary {
 		}
 	}
 
-	#reschedule_deferred_effects() {
-		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();
-	}
-
 	/**
 	 * 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
@@ -319,7 +317,7 @@ export class Boundary {
 		this.#pending_count += d;
 
 		if (this.#pending_count === 0) {
-			this.#reschedule_deferred_effects();
+			this.#resolve();
 
 			if (this.#pending_effect) {
 				pause_effect(this.#pending_effect, () => {
@@ -460,10 +458,6 @@ export class Boundary {
 	}
 }
 
-export function get_boundary() {
-	return /** @type {Boundary} */ (/** @type {Effect} */ (active_effect).b);
-}
-
 export function pending() {
 	if (active_effect === null) {
 		e.effect_pending_outside_reaction();

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

@@ -250,6 +250,14 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 			var value = array[index];
 			var key = get_key(value, index);
 
+			if (DEV) {
+				// Check that the key function is idempotent (returns the same value when called twice)
+				var key_again = get_key(value, index);
+				if (key !== key_again) {
+					e.each_key_volatile(String(index), String(key), String(key_again));
+				}
+			}
+
 			var item = first_run ? null : items.get(key);
 
 			if (item) {

packages/svelte/src/internal/client/errors.js

@@ -147,6 +147,25 @@ export function each_key_duplicate(a, b, value) {
 	}
 }
 
+/**
+ * Keyed each block has key that is not idempotent — the key for item at index %index% was `%a%` but is now `%b%`. Keys must be the same each time for a given item
+ * @param {string} index
+ * @param {string} a
+ * @param {string} b
+ * @returns {never}
+ */
+export function each_key_volatile(index, a, b) {
+	if (DEV) {
+		const error = new Error(`each_key_volatile\nKeyed each block has key that is not idempotent — the key for item at index ${index} was \`${a}\` but is now \`${b}\`. Keys must be the same each time for a given item\nhttps://svelte.dev/e/each_key_volatile`);
+
+		error.name = 'Svelte error';
+
+		throw error;
+	} else {
+		throw new Error(`https://svelte.dev/e/each_key_volatile`);
+	}
+}
+
 /**
  * `%rune%` cannot be used inside an effect cleanup function
  * @param {string} rune

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

@@ -8,7 +8,7 @@ import {
 	set_component_context,
 	set_dev_stack
 } from '../context.js';
-import { get_boundary } from '../dom/blocks/boundary.js';
+import { Boundary } from '../dom/blocks/boundary.js';
 import { invoke_error_boundary } from '../error-handling.js';
 import {
 	active_effect,
@@ -224,12 +224,7 @@ export function unset_context() {
 export function run(thunks) {
 	const restore = capture();
 
-	var boundary = get_boundary();
-	var batch = /** @type {Batch} */ (current_batch);
-	var blocking = boundary.is_rendered();
-
-	boundary.update_pending_count(1);
-	batch.increment(blocking);
+	const decrement_pending = increment_pending();
 
 	var active = /** @type {Effect} */ (active_effect);
 
@@ -286,10 +281,7 @@ export function run(thunks) {
 		// wait one more tick, so that template effects are
 		// guaranteed to run before `$effect(...)`
 		.then(() => Promise.resolve())
-		.finally(() => {
-			boundary.update_pending_count(-1);
-			batch.decrement(blocking);
-		});
+		.finally(decrement_pending);
 
 	return blockers;
 }
@@ -300,3 +292,17 @@ export function run(thunks) {
 export function wait(blockers) {
 	return Promise.all(blockers.map((b) => b.promise));
 }
+
+export function increment_pending() {
+	var boundary = /** @type {Boundary} */ (/** @type {Effect} */ (active_effect).b);
+	var batch = /** @type {Batch} */ (current_batch);
+	var blocking = boundary.is_rendered();
+
+	boundary.update_pending_count(1);
+	batch.increment(blocking);
+
+	return () => {
+		boundary.update_pending_count(-1);
+		batch.decrement(blocking);
+	};
+}

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';
@@ -142,7 +143,7 @@ export class Batch {
 
 	#decrement_queued = false;
 
-	is_deferred() {
+	#is_deferred() {
 		return this.is_fork || this.#blocking_pending > 0;
 	}
 
@@ -202,7 +203,7 @@ export class Batch {
 			// log_inconsistent_branches(root);
 		}
 
-		if (this.is_deferred()) {
+		if (this.#is_deferred()) {
 			this.#defer_effects(render_effects);
 			this.#defer_effects(effects);
 
@@ -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) {
@@ -475,7 +452,7 @@ export class Batch {
 		queue_micro_task(() => {
 			this.#decrement_queued = false;
 
-			if (!this.is_deferred()) {
+			if (!this.#is_deferred()) {
 				// we only reschedule previously-deferred effects if we expect
 				// to be able to run them after processing the batch
 				this.revive();
@@ -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;
 		}
 	}

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

@@ -40,7 +40,7 @@ import { Boundary } from '../dom/blocks/boundary.js';
 import { component_context } from '../context.js';
 import { UNINITIALIZED } from '../../../constants.js';
 import { batch_values, current_batch } from './batch.js';
-import { unset_context } from './async.js';
+import { increment_pending, unset_context } from './async.js';
 import { deferred, includes, noop } from '../../shared/utils.js';
 import { set_signal_status, update_derived_status } from './status.js';
 
@@ -111,8 +111,6 @@ export function async_derived(fn, label, location) {
 		e.async_derived_orphan();
 	}
 
-	var boundary = /** @type {Boundary} */ (parent.b);
-
 	var promise = /** @type {Promise<V>} */ (/** @type {unknown} */ (undefined));
 	var signal = source(/** @type {V} */ (UNINITIALIZED));
 
@@ -156,10 +154,7 @@ export function async_derived(fn, label, location) {
 		var batch = /** @type {Batch} */ (current_batch);
 
 		if (should_suspend) {
-			var blocking = boundary.is_rendered();
-
-			boundary.update_pending_count(1);
-			batch.increment(blocking);
+			var decrement_pending = increment_pending();
 
 			deferreds.get(batch)?.reject(STALE_REACTION);
 			deferreds.delete(batch); // delete to ensure correct order in Map iteration below
@@ -208,9 +203,8 @@ export function async_derived(fn, label, location) {
 				}
 			}
 
-			if (should_suspend) {
-				boundary.update_pending_count(-1);
-				batch.decrement(blocking);
+			if (decrement_pending) {
+				decrement_pending();
 			}
 		};
 

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

@@ -40,8 +40,8 @@ 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, current_batch, schedule_effect } from './batch.js';
-import { flatten } from './async.js';
+import { Batch, schedule_effect } from './batch.js';
+import { flatten, increment_pending } from './async.js';
 import { without_reactive_context } from '../dom/elements/bindings/shared.js';
 import { set_signal_status } from './status.js';
 
@@ -376,14 +376,16 @@ export function template_effect(fn, sync = [], async = [], blockers = []) {
  * @param {Blocker[]} blockers
  */
 export function deferred_template_effect(fn, sync = [], async = [], blockers = []) {
-	var batch = /** @type {Batch} */ (current_batch);
-	var is_async = async.length > 0 || blockers.length > 0;
-
-	if (is_async) batch.increment(true);
+	if (async.length > 0 || blockers.length > 0) {
+		var decrement_pending = increment_pending();
+	}
 
 	flatten(blockers, sync, async, (values) => {
 		create_effect(EFFECT, () => fn(...values.map(get)), false);
-		if (is_async) batch.decrement(true);
+
+		if (decrement_pending) {
+			decrement_pending();
+		}
 	});
 }
 

packages/svelte/tests/runtime-runes/samples/each-key-volatile/_config.js

@@ -0,0 +1,11 @@
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	mode: ['client'],
+
+	error: 'each_key_volatile'
+});

packages/svelte/tests/runtime-runes/samples/each-key-volatile/main.svelte

@@ -0,0 +1,10 @@
+<script>
+	let things = $state([
+		{ group: 'a', id: 1 },
+		{ group: 'b', id: 2 }
+	]);
+</script>
+
+{#each things as thing ([thing.group, thing.id])}
+	<p>{thing.group}-{thing.id}</p>
+{/each}