From 8fb7ceeba5dd1d3b7fc3f587f4e2138bf1b978ed Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Mon, 6 Jul 2026 13:50:56 +0200 Subject: [PATCH 01/10] fix: don't treat declaration tags as parts inside each blocks (#18507) Closes #18506 Declaration tags in the scope of an each block were considered "parts" and transformed to use their value instead of the signal itself. We can safely check on the `kind` of the binding since a `kind` of `state`, `state_raw` or `derived` in the each scope needs to be a declaration tag (and it's a stable reference so we can use them directly) --- .changeset/young-doodles-beam.md | 5 +++++ .../phases/3-transform/client/visitors/shared/utils.js | 6 +++++- .../samples/declaration-tags-each/_config.js | 10 ++++++++++ .../samples/declaration-tags-each/main.svelte | 6 ++++++ 4 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .changeset/young-doodles-beam.md create mode 100644 packages/svelte/tests/runtime-runes/samples/declaration-tags-each/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/declaration-tags-each/main.svelte diff --git a/.changeset/young-doodles-beam.md b/.changeset/young-doodles-beam.md new file mode 100644 index 0000000000..b68f55d241 --- /dev/null +++ b/.changeset/young-doodles-beam.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't treat declaration tags as parts inside each blocks diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 51c84dd01c..c305d53ea5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -8,7 +8,7 @@ import { sanitize_template_string } from '../../../../../utils/sanitize_template import { regex_is_valid_identifier } from '../../../../patterns.js'; import is_reference from 'is-reference'; import { dev, is_ignored, locator, component_name } from '../../../../../state.js'; -import { build_getter } from '../../utils.js'; +import { build_getter, is_state_source } from '../../utils.js'; import { ExpressionMetadata } from '../../../../nodes.js'; /** @@ -272,6 +272,10 @@ export function build_bind_this(expression, value, { state, visit }) { const binding = state.scope.get(node.name); if (!binding) return; + // if it is a state or a derived it means is a declaration tag...in that case we don't want to pass the + // value but the signal itself or assignment will break + if (is_state_source(binding, state.analysis) || binding.kind === 'derived') return; + for (const [owner, scope] of state.scopes) { if (owner.type === 'EachBlock' && scope === binding.scope) { ids.push(node); diff --git a/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/_config.js b/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/_config.js new file mode 100644 index 0000000000..d5cdd103b5 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/_config.js @@ -0,0 +1,10 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ target, assert }) { + const [btn1, btn2] = target.querySelectorAll('button'); + flushSync(() => btn2.click()); + assert.equal(btn1.textContent, '1'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/main.svelte b/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/main.svelte new file mode 100644 index 0000000000..04d55913b4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/declaration-tags-each/main.svelte @@ -0,0 +1,6 @@ +{#each Array(1)} + {let ref = $state()} + {let count = $state(0)} + + +{/each} \ No newline at end of file From 41642b70ee471dfe024dfface4c0c9939fa60d3b Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Thu, 9 Jul 2026 15:06:08 +0200 Subject: [PATCH 02/10] fix: avoid declaration tag warning in event handlers (#18500) Closes #18493 Co-authored-by: justjavac --- .changeset/dull-oranges-fry.md | 5 +++++ .../compiler/phases/2-analyze/visitors/shared/function.js | 4 +++- .../declaration-tag-state-referenced-locally/input.svelte | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 .changeset/dull-oranges-fry.md diff --git a/.changeset/dull-oranges-fry.md b/.changeset/dull-oranges-fry.md new file mode 100644 index 0000000000..0efcc6fa2d --- /dev/null +++ b/.changeset/dull-oranges-fry.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid declaration tag warning in event handlers diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js index 7bdb2243f2..6b3d227eca 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js @@ -18,7 +18,9 @@ export function visit_function(node, context) { context.next({ ...context.state, - function_depth: context.state.function_depth + 1, + // we generally want to use scope.function_depth unless we specifically increased + // that in state.function_depth (e.g. a derived) + function_depth: Math.max(context.state.scope.function_depth, context.state.function_depth) + 1, expression: null }); } diff --git a/packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally/input.svelte b/packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally/input.svelte index 90cc208e72..f422be9622 100644 --- a/packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally/input.svelte +++ b/packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally/input.svelte @@ -7,3 +7,7 @@ {let e = $state(0), f = e} {a}{b}{c}{d}{e}{f} + + \ No newline at end of file From 5edd8b0602c81494ebdad7f1a5d8077f869c2097 Mon Sep 17 00:00:00 2001 From: Ivan <273312799+Socialpranker@users.noreply.github.com> Date: Thu, 9 Jul 2026 23:59:23 +0200 Subject: [PATCH 03/10] fix: chain preprocessor sourcemaps with an empty sources[0] (#18518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #18491 — the compiler discards an upstream plugin's sourcemap when it's generated without a `source` option (e.g. `new MagicString(code).generateMap()`), producing wrong devtools/stack-trace positions. ### Root cause `compile()` already supports composing an incoming sourcemap into its output via `options.sourcemap` (`merge_with_preprocessor_map` → `apply_preprocessor_sourcemap` → `combine_sourcemaps` in `packages/svelte/src/compiler/utils/mapped_code.js`). This is the same mechanism `preprocess()` uses internally, and it's the documented contract for tools that transform a `.svelte` file before compiling it (see `CompileOptions.sourcemap`'s doc comment). `combine_sourcemaps` composes maps by matching `sourcefile === filename` (the basename of the file being compiled). A sourcemap produced by `new MagicString(code).generateMap()` **without** a `source` option — which is exactly what the reporter's Vite plugin does — has `sources: ['']`. That empty string never equals `filename`, so `remapping()` treats the node as a leaf (the "original" file) instead of a branch to keep chaining through, and the whole incoming map is silently dropped. The result: every position that flows through that segment resolves to `{ source: null, line: null, column: null }`, which is what produces the wrong/missing devtools mapping described in the issue. Vite itself already has to handle this exact ambiguity: in `pluginContainer.ts`'s `_getCombinedSourcemap`, an empty `sources[0]` from a MagicString-based transform is patched to refer to the file being transformed before Vite uses it internally. This PR applies the same normalization on svelte's side, so the contract holds regardless of whether the caller happens to pass a `source` option to `generateMap()`. ### Fix In `apply_preprocessor_sourcemap`, normalize an incoming map's `sources: ['']` (or `[null]`/`[undefined]`) to `[filename]` before calling `combine_sourcemaps`, so the chain-matching step can actually find it. --- .changeset/tame-donkeys-jump.md | 5 ++++ .../svelte/src/compiler/utils/mapped_code.js | 7 +++++ .../samples/sourcemap-empty-source/_config.js | 29 +++++++++++++++++++ .../sourcemap-empty-source/input.svelte | 6 ++++ 4 files changed, 47 insertions(+) create mode 100644 .changeset/tame-donkeys-jump.md create mode 100644 packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/_config.js create mode 100644 packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/input.svelte diff --git a/.changeset/tame-donkeys-jump.md b/.changeset/tame-donkeys-jump.md new file mode 100644 index 0000000000..aae38910db --- /dev/null +++ b/.changeset/tame-donkeys-jump.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: chain preprocessor sourcemaps with an empty `sources[0]` instead of dropping them diff --git a/packages/svelte/src/compiler/utils/mapped_code.js b/packages/svelte/src/compiler/utils/mapped_code.js index 7686ba59c6..635743c0c4 100644 --- a/packages/svelte/src/compiler/utils/mapped_code.js +++ b/packages/svelte/src/compiler/utils/mapped_code.js @@ -311,6 +311,13 @@ function apply_preprocessor_sourcemap(filename, svelte_map, preprocessor_map_inp typeof preprocessor_map_input === 'string' ? JSON.parse(preprocessor_map_input) : preprocessor_map_input; + // A preprocessor map with a missing/empty `sources[0]` (e.g. from a MagicString transform + // created without a `source` option) can't be matched against `filename` during combination, + // which silently drops the chain instead of erroring. Normalize it to `filename` first, the + // same way Vite treats an empty `sources[0]` as referring to the file being transformed. + if (preprocessor_map.sources?.length === 1 && !preprocessor_map.sources[0]) { + preprocessor_map.sources = [filename]; + } const result_map = combine_sourcemaps(filename, [svelte_map, preprocessor_map]); // Svelte expects a SourceMap which includes toUrl and toString. Instead of wrapping our output in a class, // we just tack on the extra properties. diff --git a/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/_config.js b/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/_config.js new file mode 100644 index 0000000000..43a2a4c16a --- /dev/null +++ b/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/_config.js @@ -0,0 +1,29 @@ +import * as fs from 'node:fs'; +import MagicString from 'magic-string'; +import { test } from '../../test'; + +// Simulates a bundler plugin (e.g. a Vite plugin using `magic-string`) that transforms +// the Svelte source *before* it reaches `compile()`, and hands its own sourcemap to +// `compileOptions.sourcemap` — the documented way to let svelte chain an upstream map +// into its own output map. Crucially, the upstream map is generated *without* a `source` +// option, exactly like `new MagicString(code).generateMap()` — this yields a sourcemap +// whose `sources` is `['']`, which previously broke the chain entirely (see #18491) +// instead of being treated as "this file", causing every mapping through it to resolve +// to `{ source: null, line: null, column: null }`. +const input = fs.readFileSync(new URL('./input.svelte', import.meta.url), 'utf-8'); +const src = new MagicString(input); +src.overwrite( + src.original.indexOf('count * 2'), + src.original.indexOf('count * 2') + 'count * 2'.length, + 'count * 2', + { + storeName: false + } +); + +export default test({ + compileOptions: { + sourcemap: src.generateMap({ hires: true }) + }, + client: [{ str: 'let doubled' }] +}); diff --git a/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/input.svelte b/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/input.svelte new file mode 100644 index 0000000000..b4e0b50bdb --- /dev/null +++ b/packages/svelte/tests/sourcemaps/samples/sourcemap-empty-source/input.svelte @@ -0,0 +1,6 @@ + + + From c2b7642263ec1f9d89f5d027d68f272742c449a7 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:24:35 +0200 Subject: [PATCH 04/10] chore: add benchmarks (#18523) Add two kairo variants to better measure of `#traverse` / block effects perf --- .../tests/kairo_broad_block.bench.js | 47 ++++++++++++++++++ .../tests/kairo_deep_block.bench.js | 48 +++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 benchmarking/benchmarks/reactivity/tests/kairo_broad_block.bench.js create mode 100644 benchmarking/benchmarks/reactivity/tests/kairo_deep_block.bench.js diff --git a/benchmarking/benchmarks/reactivity/tests/kairo_broad_block.bench.js b/benchmarking/benchmarks/reactivity/tests/kairo_broad_block.bench.js new file mode 100644 index 0000000000..e8451ec3fa --- /dev/null +++ b/benchmarking/benchmarks/reactivity/tests/kairo_broad_block.bench.js @@ -0,0 +1,47 @@ +import assert from 'node:assert'; +import * as $ from 'svelte/internal/client'; +import { block } from '../../../../packages/svelte/src/internal/client/reactivity/effects.js'; + +// Like `kairo_broad`, but each derived is also read by a block effect, as +// happens with e.g. `{#if derived}` in a component. Measures our #traverse perf better. +export default () => { + let head = $.state(0); + let last = head; + let counter = 0; + + const destroy = $.effect_root(() => { + for (let i = 0; i < 50; i++) { + let current = $.derived(() => { + return $.get(head) + i; + }); + let current2 = $.derived(() => { + return $.get(current) + 1; + }); + block(() => { + $.get(current2); + }); + $.render_effect(() => { + $.get(current2); + counter++; + }); + last = current2; + } + }); + + return { + destroy, + run() { + $.flush(() => { + $.set(head, 1); + }); + counter = 0; + for (let i = 0; i < 50; i++) { + $.flush(() => { + $.set(head, i); + }); + assert.equal($.get(last), i + 50); + } + assert.equal(counter, 50 * 50); + } + }; +}; diff --git a/benchmarking/benchmarks/reactivity/tests/kairo_deep_block.bench.js b/benchmarking/benchmarks/reactivity/tests/kairo_deep_block.bench.js new file mode 100644 index 0000000000..a8d9fa8c35 --- /dev/null +++ b/benchmarking/benchmarks/reactivity/tests/kairo_deep_block.bench.js @@ -0,0 +1,48 @@ +import assert from 'node:assert'; +import * as $ from 'svelte/internal/client'; +import { block } from '../../../../packages/svelte/src/internal/client/reactivity/effects.js'; + +let len = 50; +const iter = 50; + +// Like `kairo_deep`, but the derived chain is also read by a block effect, as +// happens with e.g. `{#if derived}` in a component. Measures our #traverse perf better. +export default () => { + let head = $.state(0); + let current = head; + for (let i = 0; i < len; i++) { + let c = current; + current = $.derived(() => { + return $.get(c) + 1; + }); + } + let counter = 0; + + const destroy = $.effect_root(() => { + block(() => { + $.get(current); + }); + + $.render_effect(() => { + $.get(current); + counter++; + }); + }); + + return { + destroy, + run() { + $.flush(() => { + $.set(head, 1); + }); + counter = 0; + for (let i = 0; i < iter; i++) { + $.flush(() => { + $.set(head, i); + }); + assert.equal($.get(current), len + i); + } + assert.equal(counter, iter); + } + }; +}; From bfbb026f2f7db6ced0d86ba0feb40587c0e8f598 Mon Sep 17 00:00:00 2001 From: JY Wey <34165386+JaiWey@users.noreply.github.com> Date: Sat, 11 Jul 2026 01:42:01 +1200 Subject: [PATCH 05/10] fix: skip unnecessary derived effect in earlier batch (#18525) fixes #18438 Currently the `mark` method inside `#merge` for `earlier_batch` will schedule effect for undirty derived. Add the condition for derived to prevent unnecessary effect be scheduled. ``` if ((flags & DERIVED) !== 0) { mark(/** @type {Derived} */ (reaction)); } ``` --- .changeset/curly-wasps-hide.md | 5 ++++ .../src/internal/client/reactivity/batch.js | 6 ++++ .../samples/async-batch-derived/_config.js | 28 ++++++++++++++++++ .../samples/async-batch-derived/main.svelte | 29 +++++++++++++++++++ 4 files changed, 68 insertions(+) create mode 100644 .changeset/curly-wasps-hide.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-batch-derived/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-batch-derived/main.svelte diff --git a/.changeset/curly-wasps-hide.md b/.changeset/curly-wasps-hide.md new file mode 100644 index 0000000000..7e55d77ba0 --- /dev/null +++ b/.changeset/curly-wasps-hide.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: skip unnecessary derived effect in earlier batch diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 7d14b80519..5becae2dc9 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -530,6 +530,12 @@ export class Batch { const mark = (value) => { var reactions = value.reactions; if (reactions === null) return; + // skip if value is derived and is neither dirty nor maybe dirty. transitive + // deriveds (a derived depending on another derived) are only MAYBE_DIRTY, so + // we must continue traversing them to reach the effects that depend on them + if ((value.f & DERIVED) !== 0 && (value.f & (DIRTY | MAYBE_DIRTY)) === 0) { + return; + } for (const reaction of reactions) { var flags = reaction.f; diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-derived/_config.js b/packages/svelte/tests/runtime-runes/samples/async-batch-derived/_config.js new file mode 100644 index 0000000000..d5286d9d95 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-derived/_config.js @@ -0,0 +1,28 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [increment, pop] = target.querySelectorAll('button'); + + increment.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `

Loading...

` + ); + increment.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `

Loading...

` + ); + pop.click(); + await tick(); + assert.htmlEqual(target.innerHTML, ` 2 2 1`); + + pop.click(); + await tick(); + assert.htmlEqual(target.innerHTML, ` 2 2 1`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-derived/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-batch-derived/main.svelte new file mode 100644 index 0000000000..af7f0f468c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-derived/main.svelte @@ -0,0 +1,29 @@ + + + + +{#snippet defaultPending()} +

Loading...

+{/snippet} + +{#if count > 0} + + {await push(count)} {count} {other} + +{/if} From 08a9e9e7e43453e2146357dd72d44cca1e41e29e Mon Sep 17 00:00:00 2001 From: Rabindra Kumar Meher Date: Fri, 10 Jul 2026 19:29:34 +0530 Subject: [PATCH 06/10] fix: prevent derived connection leak in untracked contexts (#18517) Fixes #18501. Problem is that is_updating_effect was also set to true for branch/root effects which are not reactive --------- Co-authored-by: Simon Holthausen --- packages/svelte/src/internal/client/runtime.js | 5 ++++- packages/svelte/tests/signals/test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 227203523e..188d16a820 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -61,6 +61,9 @@ import { without_reactive_context } from './dom/elements/bindings/shared.js'; import { set_signal_status, update_derived_status } from './reactivity/status.js'; import * as w from './warnings.js'; +/** + * True if updating in an effect context that is reactive (i.e. not branch/root effects) + */ let is_updating_effect = false; export let is_destroying_effect = false; @@ -444,7 +447,7 @@ export function update_effect(effect) { var was_updating_effect = is_updating_effect; active_effect = effect; - is_updating_effect = true; + is_updating_effect = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0; // Branch/root effects are not reactive contexts if (DEV) { var previous_component_fn = dev_current_component_function; diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 927ce2e665..07f79bd395 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1503,4 +1503,22 @@ describe('signals', () => { assert.deepEqual(log, ['inner destroyed', 'inner destroyed']); }; }); + + test('derived read in an untracked context should not leak in deps reactions', () => { + return () => { + let s = state('hello'); + let a = derived(() => $.get(s)); + let b = derived(() => $.get(a)); + + let destroy = effect_root(() => { + $.get(b); + }); + + destroy(); + + // a was spuriously added to s.reactions via is_updating_effect + // even though the entire derived chain was read in an untracked context + assert.equal(s.reactions, null); + }; + }); }); From 8e4f26552c7dd40cc056a28357f6171542d2549d Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:01:20 +0200 Subject: [PATCH 07/10] chore: changeset (#18527) for #18517 --- .changeset/common-ways-deny.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/common-ways-deny.md diff --git a/.changeset/common-ways-deny.md b/.changeset/common-ways-deny.md new file mode 100644 index 0000000000..75e9375965 --- /dev/null +++ b/.changeset/common-ways-deny.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't (re)connect deriveds when read inside branch/root effects From b4d1583ae20f3869a88a731d9a265c546c099f66 Mon Sep 17 00:00:00 2001 From: Thribhuvan Date: Fri, 10 Jul 2026 19:31:48 +0530 Subject: [PATCH 08/10] fix: transform computed keys in keyed each block destructuring patterns (#18521) fixes #18519 --- .changeset/shiny-keys-dance.md | 5 ++++ .../3-transform/client/visitors/EachBlock.js | 5 +++- .../Child.svelte | 7 +++++ .../_config.js | 29 +++++++++++++++++++ .../main.svelte | 12 ++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 .changeset/shiny-keys-dance.md create mode 100644 packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/main.svelte diff --git a/.changeset/shiny-keys-dance.md b/.changeset/shiny-keys-dance.md new file mode 100644 index 0000000000..20cf754212 --- /dev/null +++ b/.changeset/shiny-keys-dance.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: transform computed keys in keyed `{#each}` destructuring patterns diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index a1371b516a..b33eddd461 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -294,7 +294,10 @@ export function EachBlock(node, context) { let key_function = b.id('$.index'); if (node.metadata.keyed) { - const pattern = /** @type {Pattern} */ (node.context); // can only be keyed when a context is provided + // can only be keyed when a context is provided + const pattern = /** @type {Pattern} */ ( + context.visit(/** @type {Pattern} */ (node.context), key_state) + ); const expression = /** @type {Expression} */ ( context.visit(/** @type {Expression} */ (node.key), key_state) ); diff --git a/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/Child.svelte b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/Child.svelte new file mode 100644 index 0000000000..35544d0a55 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/Child.svelte @@ -0,0 +1,7 @@ + + +{#each options as { [labelKey]: label, [valueKey]: value } (value)} +

{label}: {value}

+{/each} diff --git a/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/_config.js b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/_config.js new file mode 100644 index 0000000000..8a8789d712 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/_config.js @@ -0,0 +1,29 @@ +import { flushSync } from 'svelte'; +import { ok, test } from '../../test'; + +// https://github.com/sveltejs/svelte/issues/18519 +export default test({ + html: ` + +

1: a1

+

2: a2

+

3: a3

+ `, + + test({ assert, target }) { + const btn = target.querySelector('button'); + ok(btn); + + flushSync(() => btn.click()); + + assert.htmlEqual( + target.innerHTML, + ` + +

3: a3

+

2: a2

+

1: a1

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/main.svelte new file mode 100644 index 0000000000..6885a6918e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-keyed-computed-destructuring-key/main.svelte @@ -0,0 +1,12 @@ + + + + From af6f1d309b0fc7a9dcde7099b5f231078c9f5738 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:06:53 +0200 Subject: [PATCH 09/10] chore: run every benchmark in its own process (#18542) While benchmarking it became apparent that when running the full `pnpm bench:compare` suite results could be wildly different compared to only comparing single benchmarks. The reason (most likely) is that the heap/GC/JIT state from one benchmark contaminates the others. Therefore this adjusts the runners such that each benchmark entry is run in its own forked process, not just each branch. This should give better, more stable results. Also a little fix to compare results by name; I made the mistake of adding a new benchmark to main which wasn't present on other branches yet, which made the results really confusing. --- benchmarking/compare/generate-report.js | 33 +++- benchmarking/compare/runner.js | 73 +++++++-- benchmarking/run.js | 203 +++++++++++++++--------- 3 files changed, 220 insertions(+), 89 deletions(-) diff --git a/benchmarking/compare/generate-report.js b/benchmarking/compare/generate-report.js index a61f58909b..a06aaf9fc2 100644 --- a/benchmarking/compare/generate-report.js +++ b/benchmarking/compare/generate-report.js @@ -33,11 +33,38 @@ export function generate_report(outdir) { write(''); - for (let i = 0; i < results[0].length; i += 1) { - write(`${results[0][i].benchmark}`); + // match results by benchmark name — branches may have different benchmark + // lists (e.g. a benchmark that only exists on one of the branches), so + // pairing by array index would misattribute results + const by_name = results.map((result) => new Map(result.map((r) => [r.benchmark, r]))); + + /** @type {string[]} */ + const names = []; + + for (const result of results) { + for (const { benchmark } of result) { + if (!names.includes(benchmark)) { + names.push(benchmark); + } + } + } + + for (const name of names) { + const entries = by_name.map((map) => map.get(name)); + const missing = entries + .map((entry, b) => (entry === undefined ? branches[b] : null)) + .filter((branch) => branch !== null); + + write(`${name}`); + + if (missing.length > 0) { + write(` skipped (missing on ${missing.join(', ')})`); + write(''); + continue; + } for (const metric of ['time', 'gc_time']) { - const times = results.map((result) => +result[i][metric]); + const times = entries.map((entry) => +entry[metric]); let min = Infinity; let max = -Infinity; let min_index = -1; diff --git a/benchmarking/compare/runner.js b/benchmarking/compare/runner.js index 31a8e6b44b..fa746f9869 100644 --- a/benchmarking/compare/runner.js +++ b/benchmarking/compare/runner.js @@ -1,18 +1,67 @@ +import { fork } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; import { reactivity_benchmarks } from '../benchmarks/reactivity/index.js'; import { with_cpu_profile } from '../utils.js'; -const results = []; -const PROFILE_DIR = process.env.BENCH_PROFILE_DIR; +const PROFILE_DIR = process.env.BENCH_PROFILE_DIR ?? null; +const single = process.env.BENCH_SINGLE; -for (let i = 0; i < reactivity_benchmarks.length; i += 1) { - const benchmark = reactivity_benchmarks[i]; +if (single) { + // child mode — run a single benchmark and report the result to the parent + const benchmark = reactivity_benchmarks.find((b) => b.label === single); - process.stderr.write(`Running ${i + 1}/${reactivity_benchmarks.length} ${benchmark.label} `); - results.push({ - benchmark: benchmark.label, - ...(await with_cpu_profile(PROFILE_DIR, benchmark.label, () => benchmark.fn())) - }); - process.stderr.write('\x1b[2K\r'); -} + if (!benchmark) { + throw new Error(`Unknown benchmark ${single}`); + } + + const result = await with_cpu_profile(PROFILE_DIR, benchmark.label, () => benchmark.fn()); + + // exit via the callback so the message is guaranteed to be delivered + /** @type {NodeJS.Process} */ (process).send(result, () => process.exit(0)); +} else { + // parent mode — run every benchmark in its own child process, so that + // heap/GC/JIT state from one benchmark cannot contaminate the others + const filename = fileURLToPath(import.meta.url); + const results = []; + + for (let i = 0; i < reactivity_benchmarks.length; i += 1) { + const benchmark = reactivity_benchmarks[i]; + + process.stderr.write(`Running ${i + 1}/${reactivity_benchmarks.length} ${benchmark.label} `); + + const result = await new Promise((fulfil, reject) => { + const child = fork(filename, [], { + env: { + ...process.env, + BENCH_SINGLE: benchmark.label + } + }); + + /** @type {object | null} */ + let message_received = null; -process.send(results); + child.on('message', (message) => { + message_received = /** @type {object} */ (message); + }); + + child.on('error', reject); + + child.on('exit', (code) => { + if (message_received === null) { + reject(new Error(`benchmark ${benchmark.label} exited with code ${code}`)); + } else { + fulfil(message_received); + } + }); + }); + + results.push({ + benchmark: benchmark.label, + .../** @type {object} */ (result) + }); + + process.stderr.write('\x1b[2K\r'); + } + + /** @type {NodeJS.Process} */ (process).send(results); +} diff --git a/benchmarking/run.js b/benchmarking/run.js index 80e40a5ff1..e44e816dd0 100644 --- a/benchmarking/run.js +++ b/benchmarking/run.js @@ -1,94 +1,149 @@ +import { fork } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; import * as $ from '../packages/svelte/src/internal/client/index.js'; import { reactivity_benchmarks } from './benchmarks/reactivity/index.js'; import { ssr_benchmarks } from './benchmarks/ssr/index.js'; import { with_cpu_profile } from './utils.js'; -// e.g. `pnpm bench kairo` to only run the kairo benchmarks -const filters = process.argv.slice(2); - const PROFILE_DIR = './benchmarking/.profiles'; -const suites = [ - { - benchmarks: reactivity_benchmarks.filter( - (b) => filters.length === 0 || filters.some((f) => b.label.includes(f)) - ), - name: 'reactivity benchmarks' - }, - { - benchmarks: ssr_benchmarks.filter( - (b) => filters.length === 0 || filters.some((f) => b.label.includes(f)) - ), - name: 'server-side rendering benchmarks' - } -].filter((suite) => suite.benchmarks.length > 0); - -if (suites.length === 0) { - console.log('No benchmarks matched provided filters'); - process.exit(1); -} - -const COLUMN_WIDTHS = [25, 9, 9]; -const TOTAL_WIDTH = COLUMN_WIDTHS.reduce((a, b) => a + b); - -const pad_right = (str, n) => str + ' '.repeat(n - str.length); -const pad_left = (str, n) => ' '.repeat(n - str.length) + str; +const single = process.env.BENCH_SINGLE; -let total_time = 0; -let total_gc_time = 0; +if (single) { + // child mode — run a single benchmark and report the result to the parent + const benchmark = [...reactivity_benchmarks, ...ssr_benchmarks].find((b) => b.label === single); -$.push({}, true); + if (!benchmark) { + throw new Error(`Unknown benchmark ${single}`); + } -try { - for (const { benchmarks, name } of suites) { - let suite_time = 0; - let suite_gc_time = 0; + $.push({}, true); + + const result = await with_cpu_profile(PROFILE_DIR, benchmark.label, () => benchmark.fn()); + + $.pop(); + + // exit via the callback so the message is guaranteed to be delivered + /** @type {NodeJS.Process} */ (process).send(result, () => process.exit(0)); +} else { + // parent mode — run every benchmark in its own child process, so that + // heap/GC/JIT state from one benchmark cannot contaminate the others + + // e.g. `pnpm bench kairo` to only run the kairo benchmarks + const filters = process.argv.slice(2); + + const suites = [ + { + benchmarks: reactivity_benchmarks.filter( + (b) => filters.length === 0 || filters.some((f) => b.label.includes(f)) + ), + name: 'reactivity benchmarks' + }, + { + benchmarks: ssr_benchmarks.filter( + (b) => filters.length === 0 || filters.some((f) => b.label.includes(f)) + ), + name: 'server-side rendering benchmarks' + } + ].filter((suite) => suite.benchmarks.length > 0); - console.log(`\nRunning ${name}...\n`); - console.log( - pad_right('Benchmark', COLUMN_WIDTHS[0]) + - pad_left('Time', COLUMN_WIDTHS[1]) + - pad_left('GC time', COLUMN_WIDTHS[2]) - ); - console.log('='.repeat(TOTAL_WIDTH)); + if (suites.length === 0) { + console.log('No benchmarks matched provided filters'); + process.exit(1); + } - for (const benchmark of benchmarks) { - const results = await with_cpu_profile(PROFILE_DIR, benchmark.label, () => benchmark.fn()); + const filename = fileURLToPath(import.meta.url); + + /** + * @param {string} label + * @returns {Promise<{ time: number, gc_time: number }>} + */ + const run_benchmark = (label) => { + return new Promise((fulfil, reject) => { + const child = fork(filename, [], { + env: { + ...process.env, + BENCH_SINGLE: label + } + }); + + /** @type {{ time: number, gc_time: number } | null} */ + let result = null; + + child.on('message', (message) => { + result = /** @type {{ time: number, gc_time: number }} */ (message); + }); + + child.on('error', reject); + + child.on('exit', (code) => { + if (result === null) { + reject(new Error(`benchmark ${label} exited with code ${code}`)); + } else { + fulfil(result); + } + }); + }); + }; + + const COLUMN_WIDTHS = [25, 9, 9]; + const TOTAL_WIDTH = COLUMN_WIDTHS.reduce((a, b) => a + b); + + /** @type {(str: string, n: number) => string} */ + const pad_right = (str, n) => str + ' '.repeat(n - str.length); + /** @type {(str: string, n: number) => string} */ + const pad_left = (str, n) => ' '.repeat(n - str.length) + str; + + let total_time = 0; + let total_gc_time = 0; + + try { + for (const { benchmarks, name } of suites) { + let suite_time = 0; + let suite_gc_time = 0; + + console.log(`\nRunning ${name}...\n`); + console.log( + pad_right('Benchmark', COLUMN_WIDTHS[0]) + + pad_left('Time', COLUMN_WIDTHS[1]) + + pad_left('GC time', COLUMN_WIDTHS[2]) + ); + console.log('='.repeat(TOTAL_WIDTH)); + + for (const benchmark of benchmarks) { + const results = await run_benchmark(benchmark.label); + console.log( + pad_right(benchmark.label, COLUMN_WIDTHS[0]) + + pad_left(results.time.toFixed(2), COLUMN_WIDTHS[1]) + + pad_left(results.gc_time.toFixed(2), COLUMN_WIDTHS[2]) + ); + total_time += results.time; + total_gc_time += results.gc_time; + suite_time += results.time; + suite_gc_time += results.gc_time; + } + + console.log('='.repeat(TOTAL_WIDTH)); console.log( - pad_right(benchmark.label, COLUMN_WIDTHS[0]) + - pad_left(results.time.toFixed(2), COLUMN_WIDTHS[1]) + - pad_left(results.gc_time.toFixed(2), COLUMN_WIDTHS[2]) + pad_right('suite', COLUMN_WIDTHS[0]) + + pad_left(suite_time.toFixed(2), COLUMN_WIDTHS[1]) + + pad_left(suite_gc_time.toFixed(2), COLUMN_WIDTHS[2]) ); - total_time += results.time; - total_gc_time += results.gc_time; - suite_time += results.time; - suite_gc_time += results.gc_time; + console.log('='.repeat(TOTAL_WIDTH)); } - console.log('='.repeat(TOTAL_WIDTH)); - console.log( - pad_right('suite', COLUMN_WIDTHS[0]) + - pad_left(suite_time.toFixed(2), COLUMN_WIDTHS[1]) + - pad_left(suite_gc_time.toFixed(2), COLUMN_WIDTHS[2]) - ); - console.log('='.repeat(TOTAL_WIDTH)); - } - - if (PROFILE_DIR !== null) { console.log(`\nCPU profiles written to ${PROFILE_DIR}`); + } catch (e) { + // eslint-disable-next-line no-console + console.error(e); + process.exit(1); } -} catch (e) { - // eslint-disable-next-line no-console - console.error(e); - process.exit(1); -} -$.pop(); + console.log(''); -console.log(''); - -console.log( - pad_right('total', COLUMN_WIDTHS[0]) + - pad_left(total_time.toFixed(2), COLUMN_WIDTHS[1]) + - pad_left(total_gc_time.toFixed(2), COLUMN_WIDTHS[2]) -); + console.log( + pad_right('total', COLUMN_WIDTHS[0]) + + pad_left(total_time.toFixed(2), COLUMN_WIDTHS[1]) + + pad_left(total_gc_time.toFixed(2), COLUMN_WIDTHS[2]) + ); +} From 4da9f74cd95e838acc56c4aa0d2e926d5f68972d Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:07:24 +0200 Subject: [PATCH 10/10] chore: bench compare report as HTML (#18543) Creates a `results.html` file which is much better to look at compared to the terminal output. --- .gitignore | 1 + benchmarking/compare/generate-report.js | 46 +- benchmarking/compare/index.js | 2 +- benchmarking/compare/results.template.html | 741 +++++++++++++++++++++ 4 files changed, 785 insertions(+), 5 deletions(-) create mode 100644 benchmarking/compare/results.template.html diff --git a/.gitignore b/.gitignore index 556cae6344..e4f9f9cf87 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,4 @@ packages/svelte/scripts/_baseline/ benchmarking/.profiles benchmarking/compare/.results benchmarking/compare/.profiles +benchmarking/compare/results.html diff --git a/benchmarking/compare/generate-report.js b/benchmarking/compare/generate-report.js index a06aaf9fc2..70f7b30fbb 100644 --- a/benchmarking/compare/generate-report.js +++ b/benchmarking/compare/generate-report.js @@ -2,15 +2,27 @@ import fs from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; -export function generate_report(outdir) { +const REPORT_DATA_PLACEHOLDER = '%%REPORT_DATA%%'; +const report_template = fs.readFileSync( + new URL('./results.template.html', import.meta.url), + 'utf-8' +); + +if (!report_template.includes(REPORT_DATA_PLACEHOLDER)) { + throw new Error(`Missing ${REPORT_DATA_PLACEHOLDER} in results.template.html`); +} + +export function generate_report(outdir, branches) { const result_files = fs .readdirSync(outdir) - .filter((file) => file.endsWith('.json')) + .filter((file) => file.endsWith('.json') && (!branches || branches.includes(file.slice(0, -5)))) .sort((a, b) => a.localeCompare(b)); - const branches = result_files.map((file) => file.slice(0, -5)); + // always do this so that ordering lines up (branches argument might be passed in a different order than the result files are sorted + branches = result_files.map((file) => file.slice(0, -5)); + const results = result_files.map((file) => - JSON.parse(fs.readFileSync(`${outdir}/${file}`, 'utf-8')) + JSON.parse(fs.readFileSync(path.join(outdir, file), 'utf-8')) ); if (results.length === 0) { @@ -95,6 +107,32 @@ export function generate_report(outdir) { write(''); } + + const benchmarks = names.map((name) => ({ + name, + values: by_name.map((map) => { + const entry = map.get(name); + + if (entry === undefined) return null; + + return { + time: Number(entry.time), + gc_time: Number(entry.gc_time) + }; + }) + })); + const data = JSON.stringify({ + generated_at: new Date().toISOString(), + branches, + benchmarks + }) + .replaceAll('<', '\\u003c') + .replaceAll('\u2028', '\\u2028') + .replaceAll('\u2029', '\\u2029'); + const html_file = path.resolve(outdir, '../results.html'); + + fs.writeFileSync(html_file, report_template.replace(REPORT_DATA_PLACEHOLDER, data)); + console.log(`\nHTML report written to ${html_file}`); } function char(i) { diff --git a/benchmarking/compare/index.js b/benchmarking/compare/index.js index 9064ee7da9..2e76f46e1b 100644 --- a/benchmarking/compare/index.js +++ b/benchmarking/compare/index.js @@ -85,4 +85,4 @@ if (PROFILE_DIR !== null) { console.log(`\nCPU profiles written to ${PROFILE_DIR}`); } -generate_report(outdir); +generate_report(outdir, requested_branches); diff --git a/benchmarking/compare/results.template.html b/benchmarking/compare/results.template.html new file mode 100644 index 0000000000..02f208ac4f --- /dev/null +++ b/benchmarking/compare/results.template.html @@ -0,0 +1,741 @@ + + + + + + Benchmark comparison + + + +
+

Benchmark comparison

+

+ Runtime results across branches. Green cells are fastest for an entry and red cells expose + the largest regressions. Overall runtime normalizes every benchmark to its fastest result + before averaging, so long-running entries do not outweigh short ones. +

+ +
+ +
+
+
+

Branch standings

+

+ Wins count the fastest branch for each comparable entry. Normalized runtime is the + average slowdown against each entry's fastest result; lower is better. Click a heading + to sort. +

+
+
+
+ + + + + + + + + +
+ +
+
+
+ +
+
+
+

Results by benchmark

+

+ Each cell shows runtime, difference from the winner, and GC time. Missing entries are + excluded from both standings. +

+
+
+
+ + +
+
+
+
+
+ +
+
+ + + + +