From 09c4cb5084acb6962f33561e7368097491001a80 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 18 Feb 2026 15:56:44 -0500 Subject: [PATCH] fix: re-run non-render-bound deriveds on the server (#17674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I tried to bring #14977 up-to-date but it's slipped too far. Also, I wanted to try a slightly different approach. In this PR, deriveds are memoized if they're created during render — in other words if you have something like this... ```svelte ``` ...`thing` will only be computed once. This seems correct since the inputs should never change during render. For deriveds created _outside_ render, we re-run the derived each time it is accessed, which fixes #14954. This way, there's still _some_ overhead compared to how deriveds work in the browser (where they only recompute when their dependencies have changed), but only in the rare places where it is necessary. There is one wrinkle: writable deriveds. On `main` these are just regular old variables, which means they can be written to during render. This PR currently preserves that behaviour, but I'm not sure it's desirable. It prevents the values of non-render-bound deriveds from ever updating, and makes no sense in the context of render-bound deriveds since they shouldn't be changing during render _anyway_. So my preference would be to disallow writes to deriveds on the server, but I'm not sure if we would need to consider that a breaking change. Draft because of that question, and also because I think we might be able to tidy up some stuff around class fields. - [x] figure out if we can delete some existing code around derived class fields - [x] figure out what to do about writable deriveds - [x] add a test ### 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. - [x] 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/weak-sloths-laugh.md | 5 ++ .../server/visitors/AssignmentExpression.js | 51 +++++++++++------ .../3-transform/server/visitors/Component.js | 3 +- .../3-transform/server/visitors/Identifier.js | 5 ++ .../server/visitors/VariableDeclaration.js | 57 +++++++++++++++---- .../server/visitors/shared/utils.js | 4 ++ .../phases/3-transform/shared/assignments.js | 1 - packages/svelte/src/internal/client/index.js | 3 +- .../svelte/src/internal/client/runtime.js | 24 -------- packages/svelte/src/internal/server/index.js | 23 ++++++-- .../svelte/src/internal/server/renderer.js | 35 +++++++----- packages/svelte/src/internal/shared/utils.js | 24 ++++++++ .../svelte/tests/runtime-legacy/shared.ts | 9 +++ .../derived-server-memoization/_config.js | 11 ++++ .../derived-server-memoization/main.svelte | 21 +++++++ .../state.svelte.js | 20 +++++++ .../_expected/server/index.svelte.js | 6 +- .../_expected/server/index.svelte.js | 12 ++-- .../_expected/server/index.svelte.js | 4 +- 19 files changed, 232 insertions(+), 86 deletions(-) create mode 100644 .changeset/weak-sloths-laugh.md create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-server-memoization/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-server-memoization/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-server-memoization/state.svelte.js diff --git a/.changeset/weak-sloths-laugh.md b/.changeset/weak-sloths-laugh.md new file mode 100644 index 0000000000..474fc4cdc7 --- /dev/null +++ b/.changeset/weak-sloths-laugh.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: re-run non-render-bound deriveds on the server diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index 49a284d12b..e6a1a86aa8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -68,35 +68,52 @@ function build_assignment(operator, left, right, context) { object = object.object; } - if (object.type !== 'Identifier' || !is_store_name(object.name)) { + if (object.type !== 'Identifier') { return null; } - const name = object.name.slice(1); + if (is_store_name(object.name)) { + const name = object.name.slice(1); - if (!context.state.scope.get(name)) { - return null; + if (!context.state.scope.get(name)) { + return null; + } + + if (object === left) { + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); + + return b.call('$.store_set', b.id(name), value); + } + + return b.call( + '$.store_mutate', + b.assignment('??=', b.id('$$store_subs'), b.object([])), + b.literal(object.name), + b.id(name), + b.assignment( + operator, + /** @type {Pattern} */ (context.visit(left)), + /** @type {Expression} */ (context.visit(right)) + ) + ); } - if (object === left) { + const binding = context.state.scope.get(object.name); + + // TODO 6.0 this won't work perfectly: once a derived is written to, it will + // no longer recompute. It might be better to disallow writing to deriveds + // on the server, to prevent this bug occurring + if (binding?.kind === 'derived' && object === left) { let value = /** @type {Expression} */ ( context.visit(build_assignment_value(operator, left, right)) ); - return b.call('$.store_set', b.id(name), value); + return b.call(binding.node, value); } - return b.call( - '$.store_mutate', - b.assignment('??=', b.id('$$store_subs'), b.object([])), - b.literal(object.name), - b.id(name), - b.assignment( - operator, - /** @type {Pattern} */ (context.visit(left)), - /** @type {Expression} */ (context.visit(right)) - ) - ); + return null; } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Component.js index 503b380c50..8e7d7bcdbf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Component.js @@ -1,3 +1,4 @@ +/** @import { Expression } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types.js' */ import * as b from '#compiler/builders'; @@ -8,5 +9,5 @@ import { build_inline_component } from './shared/component.js'; * @param {ComponentContext} context */ export function Component(node, context) { - build_inline_component(node, b.id(node.name), context); + build_inline_component(node, /** @type {Expression} */ (context.visit(b.id(node.name))), context); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Identifier.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Identifier.js index fa887650b3..460a3411c2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Identifier.js @@ -14,6 +14,11 @@ export function Identifier(node, context) { return b.id('$$sanitized_props'); } + if (node.name.startsWith('$$derived_array')) { + // terrible hack, but easier than adding new stuff to `context.state` for now + return b.call(node); + } + return build_getter(node, context.state); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js index 58fde5964f..2c7b14afa5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js @@ -84,10 +84,52 @@ export function VariableDeclaration(node, context) { const args = /** @type {CallExpression} */ (init).arguments; const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0; - if (rune === '$derived.by') { - declarations.push( - b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), b.call(value)) - ); + if (rune === '$derived' || rune === '$derived.by') { + const is_async = + rune === '$derived' && + context.state.analysis.async_deriveds.has( + /** @type {CallExpression} */ (declarator.init) + ); + + let init = is_async + ? b.await(b.call('$.async_derived', b.thunk(value, true))) + : b.call('$.derived', rune === '$derived' ? b.thunk(value) : value); + + if (declarator.id.type === 'Identifier') { + declarations.push( + b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), init) + ); + } else { + const call = /** @type {CallExpression} */ (declarator.init); + + let rhs = value; + + if (rune !== '$derived' || call.arguments[0].type !== 'Identifier') { + const id = b.id(context.state.scope.generate('$$d')); + + rhs = b.call(id); + declarations.push(b.declarator(id, init)); + } + + const { inserts, paths } = extract_paths(declarator.id, rhs); + + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$derived_array'); + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value))); + const call = b.call('$.derived', expression); + + declarations.push(b.declarator(id, call)); + } + + for (const path of paths) { + const expression = /** @type {Expression} */ (context.visit(path.expression)); + const call = b.call('$.derived', b.thunk(expression)); + + declarations.push(b.declarator(path.node, call)); + } + } + continue; } @@ -96,13 +138,6 @@ export function VariableDeclaration(node, context) { continue; } - if (rune === '$derived') { - declarations.push( - b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), value) - ); - continue; - } - declarations.push(...create_state_declarators(declarator, context.state.scope, value)); } } else { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js index 1af283791c..a87642bc4c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js @@ -274,6 +274,10 @@ export function build_getter(node, state) { ); } + if (binding.kind === 'derived') { + return (binding.declaration_kind === 'var' ? b.maybe_call : b.call)(binding.node); + } + return node; } diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js index 0853757775..4678d88ec5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js @@ -3,7 +3,6 @@ /** @import { Context as ServerContext } from '../server/types.js' */ import { extract_paths, is_expression_async } from '../../../utils/ast.js'; import * as b from '#compiler/builders'; -import { get_value } from '../client/visitors/shared/declarations.js'; /** * @template {ClientContext | ServerContext} Context diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index fedabd6ac1..af8df2e32c 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -153,7 +153,6 @@ export { safe_get, tick, untrack, - exclude_from_object, deep_read, deep_read_state, active_effect @@ -171,7 +170,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback, to_array } from '../shared/utils.js'; +export { noop, fallback, to_array, exclude_from_object } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index e745f7f137..3260c24b8c 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -752,30 +752,6 @@ export function untrack(fn) { } } -/** - * @param {Record} obj - * @param {Array} keys - * @returns {Record} - */ -export function exclude_from_object(obj, keys) { - /** @type {Record} */ - var result = {}; - - for (var key in obj) { - if (!keys.includes(key)) { - result[key] = obj[key]; - } - } - - for (var symbol of Object.getOwnPropertySymbols(obj)) { - if (Object.propertyIsEnumerable.call(obj, symbol) && !keys.includes(symbol)) { - result[symbol] = obj[symbol]; - } - } - - return result; -} - /** * Possibly traverse an object and read all its properties so that they're all reactive in case this is `$state`. * Does only check first level of an object for performance reasons (heuristic should be good for 99% of all cases). diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 6ab32a0d6e..2f7ef13c06 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -23,6 +23,7 @@ import { } from '../../utils.js'; import { Renderer } from './renderer.js'; import * as e from './errors.js'; +import { ssr_context } from './context.js'; // https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 // https://infra.spec.whatwg.org/#noncharacter @@ -469,7 +470,7 @@ export { push_element, pop_element, validate_snippet_args } from './dev.js'; export { snapshot } from '../shared/clone.js'; -export { fallback, to_array } from '../shared/utils.js'; +export { fallback, to_array, exclude_from_object } from '../shared/utils.js'; export { invalid_default_snippet, @@ -486,17 +487,29 @@ export { escape_html as escape }; * @returns {(new_value?: T) => (T | void)} */ export function derived(fn) { - const get_value = once(fn); - /** - * @type {T | undefined} - */ + // deriveds created during render are memoized, + // deriveds created outside (e.g. SvelteKit `page` stuff) are not + const get_value = ssr_context === null ? fn : once(fn); + + /** @type {T | undefined} */ let updated_value; return function (new_value) { if (arguments.length === 0) { return updated_value ?? get_value(); } + updated_value = new_value; return updated_value; }; } + +/** + * @template T + * @param {()=>T} fn + */ +export function async_derived(fn) { + return Promise.resolve(fn()).then((value) => { + return () => value; + }); +} diff --git a/packages/svelte/src/internal/server/renderer.js b/packages/svelte/src/internal/server/renderer.js index 610e9bb268..cb0c6d584a 100644 --- a/packages/svelte/src/internal/server/renderer.js +++ b/packages/svelte/src/internal/server/renderer.js @@ -204,9 +204,14 @@ export class Renderer { set_ssr_context(parent); if (result instanceof Promise) { + result.finally(() => { + set_ssr_context(null); + }); + if (child.global.mode === 'sync') { e.await_invalid(); } + // just to avoid unhandled promise rejections -- we'll end up throwing in `collect_async` if something fails result.catch(() => {}); child.promise = result; @@ -621,24 +626,26 @@ export class Renderer { * @returns {Renderer} */ static #open_render(mode, component, options) { - const renderer = new Renderer( - new SSRState(mode, options.idPrefix ? options.idPrefix + '-' : '', options.csp) - ); - - renderer.push(BLOCK_OPEN); - - push(); - if (options.context) /** @type {SSRContext} */ (ssr_context).c = options.context; - /** @type {SSRContext} */ (ssr_context).r = renderer; + var previous_context = ssr_context; - // @ts-expect-error - component(renderer, options.props ?? {}); + try { + const renderer = new Renderer( + new SSRState(mode, options.idPrefix ? options.idPrefix + '-' : '', options.csp) + ); - pop(); + /** @type {SSRContext} */ + const context = { p: null, c: options.context ?? null, r: renderer }; + set_ssr_context(context); - renderer.push(BLOCK_CLOSE); + renderer.push(BLOCK_OPEN); + // @ts-expect-error + component(renderer, options.props ?? {}); + renderer.push(BLOCK_CLOSE); - return renderer; + return renderer; + } finally { + set_ssr_context(previous_context); + } } /** diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index c9cc2c2d78..393b51383e 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -118,3 +118,27 @@ export function to_array(value, n) { return array; } + +/** + * @param {Record} obj + * @param {Array} keys + * @returns {Record} + */ +export function exclude_from_object(obj, keys) { + /** @type {Record} */ + var result = {}; + + for (var key in obj) { + if (!keys.includes(key)) { + result[key] = obj[key]; + } + } + + for (var symbol of Object.getOwnPropertySymbols(obj)) { + if (Object.propertyIsEnumerable.call(obj, symbol) && !keys.includes(symbol)) { + result[symbol] = obj[symbol]; + } + } + + return result; +} diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index c5317f822e..94f09c9e8d 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -13,6 +13,7 @@ import type { CompileOptions } from '#compiler'; import { suite_with_variants, type BaseTest } from '../suite.js'; import { clear } from '../../src/internal/client/reactivity/batch.js'; import { hydrating } from '../../src/internal/client/dom/hydration.js'; +import { ssr_context } from '../../src/internal/server/context.js'; type Assert = typeof import('vitest').assert & { htmlEqual(a: string, b: string, description?: string): void; @@ -358,6 +359,10 @@ async function run_test_variant( let snapshot = undefined; if (variant === 'hydrate' || variant === 'ssr' || variant === 'async-ssr') { + if (ssr_context !== null) { + throw new Error('ssr_context was not cleared'); + } + config.before_test?.(); // ssr into target const SsrSvelteComponent = (await import(`${cwd}/_output/server/main.svelte.js`)).default; @@ -387,6 +392,10 @@ async function run_test_variant( snapshot = config.snapshot(target); } } + + if (ssr_context !== null) { + throw new Error('ssr_context was not cleared'); + } } else { target.innerHTML = ''; } diff --git a/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/_config.js new file mode 100644 index 0000000000..4525e1ea81 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + test_ssr({ assert, logs }) { + assert.deepEqual(logs, [0, 2, { count: 2 }, 0, 0, { local_count: 1 }]); + }, + + test({ assert, logs }) { + assert.deepEqual(logs, [0, 2, { count: 2 }, 0, 0, { local_count: 1 }]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/main.svelte new file mode 100644 index 0000000000..b291707849 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/main.svelte @@ -0,0 +1,21 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/state.svelte.js b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/state.svelte.js new file mode 100644 index 0000000000..df8f6f4795 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-server-memoization/state.svelte.js @@ -0,0 +1,20 @@ +let s = $state(0); +let d = $derived.by(() => { + count += 1; + return s * 2; +}); + +export let count = 0; + +export function reset() { + count = 0; + s = 0; +} + +export function increment() { + s += 1; +} + +export function get() { + return d; +} diff --git a/packages/svelte/tests/snapshot/samples/async-if-chain/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-if-chain/_expected/server/index.svelte.js index 91315edb94..1b1ab51d6f 100644 --- a/packages/svelte/tests/snapshot/samples/async-if-chain/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-if-chain/_expected/server/index.svelte.js @@ -8,7 +8,7 @@ export default function Async_if_chain($$renderer) { let foo = true; var blocking; - var $$promises = $$renderer.run([async () => blocking = await foo]); + var $$promises = $$renderer.run([async () => blocking = await $.async_derived(() => foo)]); $$renderer.async_block([$$promises[0]], ($$renderer) => { if (foo) { @@ -94,10 +94,10 @@ export default function Async_if_chain($$renderer) { $$renderer.push(` `); $$renderer.async_block([$$promises[0]], ($$renderer) => { - if (blocking > 10) { + if (blocking() > 10) { $$renderer.push(''); $$renderer.push(`foo`); - } else if (blocking > 5) { + } else if (blocking() > 5) { $$renderer.push(''); $$renderer.push(`bar`); } else { diff --git a/packages/svelte/tests/snapshot/samples/async-in-derived/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-in-derived/_expected/server/index.svelte.js index 58358b3ded..3af9f504ec 100644 --- a/packages/svelte/tests/snapshot/samples/async-in-derived/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-in-derived/_expected/server/index.svelte.js @@ -6,15 +6,15 @@ export default function Async_in_derived($$renderer, $$props) { var yes1, yes2, no1, no2; var $$promises = $$renderer.run([ - async () => yes1 = await 1, - async () => yes2 = foo(await 1), - () => no1 = (async () => { + async () => yes1 = await $.async_derived(() => 1), + async () => yes2 = await $.async_derived(async () => foo(await 1)), + () => no1 = $.derived(async () => { return await 1; - })(), + }), - () => no2 = async () => { + () => no2 = $.derived(() => async () => { return await 1; - } + }) ]); if (true) { diff --git a/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/server/index.svelte.js index e9bf215dcd..a41b58051c 100644 --- a/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/server/index.svelte.js @@ -2,13 +2,13 @@ import * as $ from 'svelte/internal/server'; export default function Await_block_scope($$renderer) { let counter = { count: 0 }; - const promise = Promise.resolve(counter); + const promise = $.derived(() => Promise.resolve(counter)); function increment() { counter.count += 1; } $$renderer.push(` `); - $.await($$renderer, promise, () => {}, (counter) => {}); + $.await($$renderer, promise(), () => {}, (counter) => {}); $$renderer.push(` ${$.escape(counter.count)}`); } \ No newline at end of file