From b1a6b8cbdaa23314066dbc36480eecd9d85620b8 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 16 Jun 2025 19:05:43 +0200 Subject: [PATCH] fix: use proxy for destructured values --- .../phases/3-transform/server/types.d.ts | 1 + .../server/visitors/CallExpression.js | 7 +- .../server/visitors/VariableDeclaration.js | 77 ++++--------------- packages/svelte/src/internal/server/index.js | 47 ++++++++++- packages/svelte/src/internal/shared/utils.js | 22 ++++++ .../_expected/server/index.svelte.js | 4 +- .../_expected/server/index.svelte.js | 44 +++++------ 7 files changed, 110 insertions(+), 92 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts index adde7480cb..c9000ca487 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/server/types.d.ts @@ -6,6 +6,7 @@ import type { ComponentAnalysis } from '../../types.js'; export interface ServerTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; + readonly is_destructured_derived?: boolean; } export interface ComponentServerTransformState extends ServerTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 35c79988b0..daafdc9295 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -31,7 +31,12 @@ export function CallExpression(node, context) { if (rune === '$derived' || rune === '$derived.by') { const fn = /** @type {Expression} */ (context.visit(node.arguments[0])); - return b.call('$.derived', rune === '$derived' ? b.thunk(fn) : fn); + + return b.call( + '$.derived', + rune === '$derived' ? b.thunk(fn) : fn, + context.state.is_destructured_derived && b.true + ); } if (rune === '$state.snapshot') { 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 32b6e62a68..5fc5a259b1 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,12 +1,12 @@ -/** @import { VariableDeclaration, VariableDeclarator, Expression, CallExpression, Pattern, Identifier, ObjectPattern, ArrayPattern, Property } from 'estree' */ +/** @import { VariableDeclaration, VariableDeclarator, Expression, CallExpression, Pattern, Identifier } from 'estree' */ /** @import { Binding } from '#compiler' */ /** @import { Context } from '../types.js' */ /** @import { ComponentAnalysis } from '../../../types.js' */ /** @import { Scope } from '../../../scope.js' */ -import { walk } from 'zimmerframe'; -import { build_fallback, extract_identifiers, extract_paths } from '../../../../utils/ast.js'; +import { build_fallback, extract_paths } from '../../../../utils/ast.js'; import * as b from '#compiler/builders'; import { get_rune } from '../../../scope.js'; +import { walk } from 'zimmerframe'; /** * @param {VariableDeclaration} node @@ -17,9 +17,6 @@ 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); @@ -84,75 +81,29 @@ export function VariableDeclaration(node, context) { continue; } - const args = /** @type {CallExpression} */ (init).arguments; - const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0; - - 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)), value) - ); - continue; - } + const value = init + ? /** @type {Expression} */ ( + context.visit(init, { + ...context.state, + is_destructured_derived: declarator.id.type !== 'Identifier' + }) + ) + : b.void0; if (declarator.id.type === 'Identifier') { - if (is_destructuring && rune === '$derived') { - add_destructured_reassign(() => value); - continue; - } - declarations.push( - b.declarator(declarator.id, rune === '$derived' ? b.thunk(value) : value) - ); + declarations.push(b.declarator(declarator.id, value)); continue; } - if (rune === '$derived') { - if (is_destructuring) { - add_destructured_reassign(() => value); - continue; - } + if (rune === '$derived' || rune === '$derived.by') { declarations.push( - b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), b.thunk(value)) + b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), 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/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 2ca85fff44..56f31e10d0 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -3,7 +3,7 @@ /** @import { Store } from '#shared' */ export { FILENAME, HMR } from '../../constants.js'; import { attr, clsx, to_class, to_style } from '../shared/attributes.js'; -import { is_promise, noop } from '../shared/utils.js'; +import { access_path_on_object, is_promise, noop } from '../shared/utils.js'; import { subscribe_to_store } from '../../store/utils.js'; import { UNINITIALIZED, @@ -18,6 +18,7 @@ import { validate_store } from '../shared/validate.js'; import { is_boolean_attribute, is_raw_text_element, is_void } from '../../utils.js'; import { reset_elements } from './dev.js'; import { Payload } from './payload.js'; +import { is } from '../client/proxy.js'; // https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 // https://infra.spec.whatwg.org/#noncharacter @@ -518,15 +519,55 @@ export { escape_html as escape }; /** * @template T * @param {()=>T} fn - * @returns {(new_value?: T) => (T | void)} + * @param {boolean} is_destructured + * @returns {((new_value?: T) => (T | void)) | Record (T | void)>} */ -export function derived(fn) { +export function derived(fn, is_destructured = false) { const get_value = once(fn); /** * @type {T | undefined} */ let updated_value; + if (is_destructured) { + /** + * + * @param {(string | symbol)[]} path + * @returns + */ + function recursive_proxy(path = []) { + return new Proxy( + /** @type {(new_value: any)=>any} */ ( + function (new_value) { + if (arguments.length === 0) { + return ( + access_path_on_object(/** @type {*} */ (updated_value), path) ?? + access_path_on_object(/** @type {*} */ (get_value()), path) + ); + } + var last_key = path[path.length - 1]; + const to_update = access_path_on_object( + /** @type {*} */ (updated_value), + path.slice(0, -1), + (current, key) => { + current[key] = {}; + } + ); + /** @type {*} */ (to_update)[last_key] = new_value; + return /** @type {*} */ (to_update)[last_key]; + } + ), + { + get(_, key) { + return recursive_proxy([...path, key]); + } + } + ); + } + updated_value = /** @type {T} */ ({}); + return recursive_proxy(); + } + return function (new_value) { if (arguments.length === 0) { return updated_value ?? get_value(); diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 10f8597520..10c218f7e6 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -116,3 +116,25 @@ export function to_array(value, n) { return array; } + +/** + * + * @param {Record} obj + * @param {(string | symbol)[]} path + * @param {(current: Record, key:string|symbol)=>void} [on_undefined] + * @returns + */ +export function access_path_on_object(obj, path, on_undefined) { + if (obj == null) return undefined; + + let current = obj; + for (const key of path) { + if (current == null) return undefined; + if (current[key] == null && on_undefined) { + on_undefined(current, key); + } + current = /** @type {*} */ (current)[key]; + } + + return current; +} 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 dd82309a2c..139dd2e132 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,7 +2,7 @@ import * as $ from 'svelte/internal/server'; export default function Await_block_scope($$payload) { let counter = { count: 0 }; - const promise = () => Promise.resolve(counter); + const promise = $.derived(() => Promise.resolve(counter)); function increment() { counter.count += 1; @@ -11,4 +11,4 @@ export default function Await_block_scope($$payload) { $$payload.out += ` `; $.await($$payload, promise?.(), () => {}, (counter) => {}); $$payload.out += ` ${$.escape(counter.count)}`; -} +} \ No newline at end of file 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 18b3379ed3..ce72a3d2fd 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 @@ -6,51 +6,49 @@ export default function Server_deriveds($$payload, $$props) { // 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 { foo, bar: [a, b, { baz }] } = $.derived(() => stuff, true); let stuff2 = [1, 2, 3]; - - let [d_1, e_1, f_1] = stuff2, - d = () => d_1, - e = () => e_1, - f = () => f_1; - + let [d, e, f] = $.derived(() => stuff2, true); let count = 0; - let double = () => count * 2; - let identifier = () => count; - let dot_by = () => () => count; + let double = $.derived(() => count * 2); + let identifier = $.derived(() => count); + let dot_by = $.derived(() => () => count); class Test { state = 0; - #der = () => this.state * 2; + #der = $.derived(() => this.state * 2); get der() { return this.#der(); } - #der_by = () => this.state; + set der($$value) { + return this.#der($$value); + } + + #der_by = $.derived(() => this.state); get der_by() { return this.#der_by(); } - #identifier = () => this.state; + set der_by($$value) { + return this.#der_by($$value); + } + + #identifier = $.derived(() => this.state); get identifier() { return this.#identifier(); } + + set identifier($$value) { + return this.#identifier($$value); + } } 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)}`; + $$payload.out += `${$.escape(foo?.())} ${$.escape(a?.())} ${$.escape(b?.())} ${$.escape(baz?.())} ${$.escape(d?.())} ${$.escape(e?.())} ${$.escape(f?.())} 0 0 ${$.escape(dot_by?.())} ${$.escape(test.der)} ${$.escape(test.der_by)} ${$.escape(test.identifier)}`; $.pop(); } \ No newline at end of file