From bd4ba56932cd364259c7dc64e3c1f1e8b3c10491 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 17 Feb 2026 12:50:35 -0500 Subject: [PATCH 1/4] chore: make batch.is_deferred a private method (#17729) small tweak --- packages/svelte/src/internal/client/reactivity/batch.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index b382a4e3a5..b5a2651b2a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -142,7 +142,7 @@ export class Batch { #decrement_queued = false; - is_deferred() { + #is_deferred() { return this.is_fork || this.#blocking_pending > 0; } @@ -202,7 +202,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); @@ -475,7 +475,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(); From 3f6521df0e1c476b85f704b8aded0303be8eb4b4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 17 Feb 2026 13:56:49 -0500 Subject: [PATCH 2/4] chore: unify async logic (#17731) We have a bunch of repeated logic around incrementing/decrementing pending states. This DRYs it out to unblock some forthcoming changes around scheduling --- .../src/internal/client/dom/blocks/async.js | 14 ++-------- .../internal/client/dom/blocks/boundary.js | 4 --- .../src/internal/client/reactivity/async.js | 28 +++++++++++-------- .../internal/client/reactivity/deriveds.js | 14 +++------- .../src/internal/client/reactivity/effects.js | 16 ++++++----- 5 files changed, 33 insertions(+), 43 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/async.js b/packages/svelte/src/internal/client/dom/blocks/async.js index e8c9cf0643..43af3d8dd3 100644 --- a/packages/svelte/src/internal/client/dom/blocks/async.js +++ b/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(); } }); } diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index da70cbb19d..08cc994494 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -452,10 +452,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(); diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index b6eba3bf8a..b3c5248179 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/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); + }; +} diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index d11854fc91..80da5528c8 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/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} */ (/** @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(); } }; diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 512c435a27..157587e218 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/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(); + } }); } From 2287ad005aee7fc14681f3913c3f5a0cebfd181e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 17 Feb 2026 16:38:12 -0500 Subject: [PATCH 3/4] fix: detect and error on non-idempotent each block keys in dev mode (#17732) ## Summary Fixes #17721 In dev mode, detect when a keyed each block has a key function that returns different values when called multiple times for the same item (non-idempotent). This catches the common mistake of using array literals like `[thing.group, thing.id]` as keys, which creates a new array object each time and will never match by reference. - Adds new `each_key_volatile` error with helpful message explaining the issue - Checks key idempotency in the each block loop during dev mode - Provides a clear error instead of the cryptic "Cannot read properties of undefined" that occurred previously --------- Co-authored-by: 7nik --- .changeset/volatile-each-key.md | 5 +++++ .../98-reference/.generated/client-errors.md | 8 ++++++++ .../svelte/messages/client-errors/errors.md | 6 ++++++ .../src/internal/client/dom/blocks/each.js | 8 ++++++++ packages/svelte/src/internal/client/errors.js | 19 +++++++++++++++++++ .../samples/each-key-volatile/_config.js | 11 +++++++++++ .../samples/each-key-volatile/main.svelte | 10 ++++++++++ 7 files changed, 67 insertions(+) create mode 100644 .changeset/volatile-each-key.md create mode 100644 packages/svelte/tests/runtime-runes/samples/each-key-volatile/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/each-key-volatile/main.svelte diff --git a/.changeset/volatile-each-key.md b/.changeset/volatile-each-key.md new file mode 100644 index 0000000000..674bce9bec --- /dev/null +++ b/.changeset/volatile-each-key.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: detect and error on non-idempotent each block keys in dev mode diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index 8601a728a7..7fccac5808 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/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 ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index bedf6db0a5..3f20cb989d 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/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 diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 25f7cf91eb..7ae02d073c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/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) { diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 34f1d85540..d60c2dd280 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/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 diff --git a/packages/svelte/tests/runtime-runes/samples/each-key-volatile/_config.js b/packages/svelte/tests/runtime-runes/samples/each-key-volatile/_config.js new file mode 100644 index 0000000000..87d4bf45c7 --- /dev/null +++ b/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' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-key-volatile/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-key-volatile/main.svelte new file mode 100644 index 0000000000..689c257cfa --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-key-volatile/main.svelte @@ -0,0 +1,10 @@ + + +{#each things as thing ([thing.group, thing.id])} +

{thing.group}-{thing.id}

+{/each} From c83aa06d69fba0f30300a9f0614e00030fbfbc31 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 17 Feb 2026 17:38:05 -0500 Subject: [PATCH 4/4] chore: proactively defer effects in pending boundary (#17734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` --- .changeset/angry-ideas-listen.md | 5 ++ .../internal/client/dom/blocks/boundary.js | 44 +++++++++-------- .../src/internal/client/reactivity/batch.js | 49 +++++++++---------- 3 files changed, 50 insertions(+), 48 deletions(-) create mode 100644 .changeset/angry-ideas-listen.md diff --git a/.changeset/angry-ideas-listen.md b/.changeset/angry-ideas-listen.md new file mode 100644 index 0000000000..cb2bf00d1c --- /dev/null +++ b/.changeset/angry-ideas-listen.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: proactively defer effects in pending boundary diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 08cc994494..8f23fb1a2e 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/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, () => { diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index b5a2651b2a..297049fd6b 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/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,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 `` 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; } }