From b3eeee08b69f39e034f200c707b11829a8517768 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 10 Jan 2025 21:36:49 +0100 Subject: [PATCH 1/4] fix: thunkify deriveds on the server --- .changeset/curvy-toes-warn.md | 5 ++ .../3-transform/server/visitors/ClassBody.js | 3 +- .../server/visitors/VariableDeclaration.js | 60 +++++++++++++++++-- .../server/visitors/shared/utils.js | 4 ++ .../_expected/server/index.svelte.js | 4 +- .../samples/server-deriveds/_config.js | 5 ++ .../_expected/server/index.svelte.js | 59 ++++++++++++++++++ .../samples/server-deriveds/index.svelte | 25 ++++++++ packages/svelte/tests/snapshot/test.ts | 6 +- 9 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 .changeset/curvy-toes-warn.md create mode 100644 packages/svelte/tests/snapshot/samples/server-deriveds/_config.js create mode 100644 packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/server-deriveds/index.svelte diff --git a/.changeset/curvy-toes-warn.md b/.changeset/curvy-toes-warn.md new file mode 100644 index 000000000000..07f1c7dcd468 --- /dev/null +++ b/.changeset/curvy-toes-warn.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: thunkify deriveds on the server diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js index 365084a28486..b03e1b5ab516 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/ClassBody.js @@ -85,8 +85,7 @@ export function ClassBody(node, context) { const init = /** @type {Expression} **/ ( context.visit(definition.value.arguments[0], child_state) ); - const value = - field.kind === 'derived_by' ? b.call('$.once', init) : b.call('$.once', b.thunk(init)); + const value = field.kind === 'derived_by' ? init : b.thunk(init); if (is_private) { body.push(b.prop_def(field.id, value)); 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 31de811ac76f..a530b52f7677 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 @@ -1,11 +1,11 @@ -/** @import { VariableDeclaration, VariableDeclarator, Expression, CallExpression, Pattern, Identifier } from 'estree' */ +/** @import { VariableDeclaration, VariableDeclarator, Expression, CallExpression, Pattern, Identifier, ObjectPattern, ArrayPattern, Property } from 'estree' */ /** @import { Binding } from '#compiler' */ /** @import { Context } from '../types.js' */ /** @import { Scope } from '../../../scope.js' */ -import { build_fallback, extract_paths } from '../../../../utils/ast.js'; +import { walk } from 'zimmerframe'; +import { build_fallback, extract_identifiers, extract_paths } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; import { get_rune } from '../../../scope.js'; -import { walk } from 'zimmerframe'; /** * @param {VariableDeclaration} node @@ -16,6 +16,9 @@ export function VariableDeclaration(node, context) { const declarations = []; if (context.state.analysis.runes) { + /** @type {VariableDeclarator[]} */ + const destructured_reassigns = []; + for (const declarator of node.declarations) { const init = declarator.init; const rune = get_rune(init, context.state.scope); @@ -73,27 +76,72 @@ export function VariableDeclaration(node, context) { const value = args.length === 0 ? b.id('undefined') : /** @type {Expression} */ (context.visit(args[0])); + const is_destructuring = + declarator.id.type === 'ObjectPattern' || declarator.id.type === 'ArrayPattern'; + + /** + * + * @param {()=>Expression} get_generated_init + */ + function add_destructured_reassign(get_generated_init) { + // to keep everything that the user destructure as a function we need to change the original + // assignment to a generated value and then reassign a variable with the original name + if (declarator.id.type === 'ObjectPattern' || declarator.id.type === 'ArrayPattern') { + const id = /** @type {ObjectPattern | ArrayPattern} */ (context.visit(declarator.id)); + const modified = walk( + /**@type {Identifier|Property}*/ (/**@type {unknown}*/ (id)), + {}, + { + Identifier(id, { path }) { + const parent = path.at(-1); + // we only want the identifiers for the value + if (parent?.type === 'Property' && parent.value !== id) return; + const generated = context.state.scope.generate(id.name); + destructured_reassigns.push(b.declarator(b.id(id.name), b.thunk(b.id(generated)))); + return b.id(generated); + } + } + ); + declarations.push(b.declarator(/**@type {Pattern}*/ (modified), get_generated_init())); + } + } + if (rune === '$derived.by') { + if (is_destructuring) { + add_destructured_reassign(() => b.call(value)); + continue; + } declarations.push( - b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), b.call(value)) + b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), value) ); continue; } if (declarator.id.type === 'Identifier') { - declarations.push(b.declarator(declarator.id, value)); + if (is_destructuring && rune === '$derived') { + add_destructured_reassign(() => value); + continue; + } + declarations.push( + b.declarator(declarator.id, rune === '$derived' ? b.thunk(value) : value) + ); continue; } if (rune === '$derived') { + if (is_destructuring) { + add_destructured_reassign(() => value); + continue; + } declarations.push( - b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), value) + b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), b.thunk(value)) ); continue; } declarations.push(...create_state_declarators(declarator, context.state.scope, value)); } + declarations.push(...destructured_reassigns); } else { for (const declarator of node.declarations) { const bindings = /** @type {Binding[]} */ (context.state.scope.get_bindings(declarator)); 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 2c6aa2f316aa..1a4898a3d7cb 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 @@ -237,6 +237,10 @@ export function build_getter(node, state) { b.literal(node.name), build_getter(store_id, state) ); + } else if (binding.kind === 'derived') { + // we need a maybe_call because in case of `var` + // the user might use the variable before the initialization + return b.maybe_call(node.name); } return node; 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 012789a5509b..34e26e7fa096 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($$payload) { let counter = { count: 0 }; - const promise = Promise.resolve(counter); + const promise = () => Promise.resolve(counter); function increment() { counter.count += 1; } $$payload.out += ` `; - $.await(promise, () => {}, (counter) => {}, () => {}); + $.await(promise?.(), () => {}, (counter) => {}, () => {}); $$payload.out += ` ${$.escape(counter.count)}`; } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/server-deriveds/_config.js b/packages/svelte/tests/snapshot/samples/server-deriveds/_config.js new file mode 100644 index 000000000000..2a7b882b86ea --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/server-deriveds/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + mode: ['server'] +}); diff --git a/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js new file mode 100644 index 000000000000..009dd90f5b57 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js @@ -0,0 +1,59 @@ +import * as $ from "svelte/internal/server"; + +export default function Server_deriveds($$payload, $$props) { + $.push(); + + // destructuring stuff on the server needs a bit more code + // so that every identifier is a function + let stuff = { + foo: true, + bar: [1, 2, { baz: 'baz' }] + }; + + let { + foo: foo_1, + bar: [a_1, b_1, { baz: baz_1 }] + } = stuff, + foo = () => foo_1, + a = () => a_1, + b = () => b_1, + baz = () => baz_1; + + let stuff2 = [1, 2, 3]; + + let [d_1, e_1, f_1] = stuff2, + d = () => d_1, + e = () => e_1, + f = () => f_1; + + let count = 0; + let double = () => count * 2; + let identifier = () => count; + let dot_by = () => () => count; + + class Test { + state = 0; + #der = () => this.state * 2; + + get der() { + return this.#der(); + } + + #der_by = () => this.state; + + get der_by() { + return this.#der_by(); + } + + #identifier = () => this.state; + + get identifier() { + return this.#identifier(); + } + } + + const test = new Test(); + + $$payload.out += `${$.escape(foo?.())} ${$.escape(a?.())} ${$.escape(b?.())} ${$.escape(baz?.())} ${$.escape(d?.())} ${$.escape(e?.())} ${$.escape(f?.())} ${$.escape(double?.())} ${$.escape(identifier?.())} ${$.escape(dot_by?.())} ${$.escape(test.der)} ${$.escape(test.der_by)} ${$.escape(test.identifier)}`; + $.pop(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/server-deriveds/index.svelte b/packages/svelte/tests/snapshot/samples/server-deriveds/index.svelte new file mode 100644 index 000000000000..ab97da6bf8fc --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/server-deriveds/index.svelte @@ -0,0 +1,25 @@ + + +{foo} {a} {b} {baz} {d} {e} {f} {double} {identifier} {dot_by} {test.der} {test.der_by} {test.identifier} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/test.ts b/packages/svelte/tests/snapshot/test.ts index 88cf9193c3f7..f0f626f7915e 100644 --- a/packages/svelte/tests/snapshot/test.ts +++ b/packages/svelte/tests/snapshot/test.ts @@ -7,11 +7,13 @@ import { VERSION } from 'svelte/compiler'; interface SnapshotTest extends BaseTest { compileOptions?: Partial; + mode?: ('client' | 'server')[]; } const { test, run } = suite(async (config, cwd) => { - await compile_directory(cwd, 'client', config.compileOptions); - await compile_directory(cwd, 'server', config.compileOptions); + for (const mode of config?.mode ?? ['server', 'client']) { + await compile_directory(cwd, mode, config.compileOptions); + } // run `UPDATE_SNAPSHOTS=true pnpm test snapshot` to update snapshot tests if (process.env.UPDATE_SNAPSHOTS) { From 4b22a097091100ddf633adf5987f4695f0911f7c Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 10 Jan 2025 21:59:45 +0100 Subject: [PATCH 2/4] chore: install before generating snapshots --- .../server-deriveds/_expected/server/index.svelte.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js index 009dd90f5b57..18b3379ed311 100644 --- a/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/server-deriveds/_expected/server/index.svelte.js @@ -1,14 +1,11 @@ -import * as $ from "svelte/internal/server"; +import * as $ from 'svelte/internal/server'; export default function Server_deriveds($$payload, $$props) { $.push(); // destructuring stuff on the server needs a bit more code // so that every identifier is a function - let stuff = { - foo: true, - bar: [1, 2, { baz: 'baz' }] - }; + let stuff = { foo: true, bar: [1, 2, { baz: 'baz' }] }; let { foo: foo_1, From a3f7ae8c739c17fddd3dd14e7341f4a65436f130 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 17 Apr 2025 15:45:31 -0400 Subject: [PATCH 3/4] dont transform twice --- .../phases/3-transform/server/visitors/shared/element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index b0bcb8fd6fbb..aac15cc03df6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -169,7 +169,7 @@ export function build_element_attributes(node, context) { type: 'ExpressionTag', start: -1, end: -1, - expression, + expression: attribute.expression, metadata: { expression: create_expression_metadata() } From 961c36bdf50e645790dd8e42217883db8aee0793 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 17 Apr 2025 16:01:08 -0400 Subject: [PATCH 4/4] fix --- .../3-transform/server/visitors/shared/element.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index aac15cc03df6..b5f6fb38063a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -163,13 +163,26 @@ export function build_element_attributes(node, context) { ]) ); } else { + /** @type {Expression} */ + let expression = attribute.expression; + + if (attribute.type === 'BindDirective' && expression.type === 'SequenceExpression') { + const getter = expression.expressions[0]; + expression = + getter.type === 'ArrowFunctionExpression' && + getter.params.length === 0 && + getter.body.type !== 'BlockStatement' + ? getter.body + : b.call(getter); + } + attributes.push( create_attribute(attribute.name, -1, -1, [ { type: 'ExpressionTag', start: -1, end: -1, - expression: attribute.expression, + expression, metadata: { expression: create_expression_metadata() }