diff --git a/.changeset/stupid-bottles-lay.md b/.changeset/stupid-bottles-lay.md new file mode 100644 index 000000000000..e1276d342085 --- /dev/null +++ b/.changeset/stupid-bottles-lay.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: better code generation for `let:` directives in SSR mode diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 3dbf5dc292bf..15cc09cf8bfb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -3,7 +3,8 @@ import type { Statement, LabeledStatement, Identifier, - PrivateIdentifier + PrivateIdentifier, + Expression } from 'estree'; import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; @@ -22,6 +23,11 @@ export interface ClientTransformState extends TransformState { /** The $: calls, which will be ordered in the end */ readonly legacy_reactive_statements: Map; + /** + * A map of `[name, node]` pairs, where `Identifier` nodes matching `name` + * will be replaced with `node` (e.g. `x` -> `$.get(x)`) + */ + readonly getters: Record Expression)>; } export interface ComponentClientTransformState extends ClientTransformState { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index f18df26eb454..2cc8a5b97582 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -23,7 +23,6 @@ import { Identifier } from './visitors/Identifier.js'; import { IfBlock } from './visitors/IfBlock.js'; import { KeyBlock } from './visitors/KeyBlock.js'; import { LabeledStatementLegacy } from './visitors/LabeledStatement.js'; -import { LetDirective } from './visitors/LetDirective.js'; import { MemberExpressionRunes } from './visitors/MemberExpression.js'; import { PropertyDefinitionRunes } from './visitors/PropertyDefinition.js'; import { RegularElement } from './visitors/RegularElement.js'; @@ -79,7 +78,6 @@ const template_visitors = { HtmlTag, IfBlock, KeyBlock, - LetDirective, RegularElement, RenderTag, SlotElement, @@ -113,7 +111,6 @@ export function server_component(analysis, options) { namespace: options.namespace, preserve_whitespace: options.preserveWhitespace, private_derived: new Map(), - getters: {}, skip_hydration_boundaries: false }; @@ -422,8 +419,7 @@ export function server_module(analysis, options) { // to be present for `javascript_visitors_legacy` and so is included in module // transform state as well as component transform state legacy_reactive_statements: new Map(), - private_derived: new Map(), - getters: {} + private_derived: new Map() }; const module = /** @type {Program} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/LetDirective.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/LetDirective.js deleted file mode 100644 index 17c3260beec2..000000000000 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/LetDirective.js +++ /dev/null @@ -1,40 +0,0 @@ -/** @import { LetDirective } from '#compiler' */ -/** @import { ComponentContext } from '../types.js' */ -import * as b from '../../../../utils/builders.js'; - -/** - * @param {LetDirective} node - * @param {ComponentContext} context - */ -export function LetDirective(node, context) { - if (node.expression === null || node.expression.type === 'Identifier') { - const name = node.expression === null ? node.name : node.expression.name; - return b.const(name, b.member(b.id('$$slotProps'), b.id(node.name))); - } - - const name = context.state.scope.generate(node.name); - const bindings = context.state.scope.get_bindings(node); - - for (const binding of bindings) { - context.state.getters[binding.node.name] = b.member(b.id(name), b.id(binding.node.name)); - } - - return b.const( - name, - b.call( - b.thunk( - b.block([ - b.let( - node.expression.type === 'ObjectExpression' - ? // @ts-expect-error types don't match, but it can't contain spread elements and the structure is otherwise fine - b.object_pattern(node.expression.properties) - : // @ts-expect-error types don't match, but it can't contain spread elements and the structure is otherwise fine - b.array_pattern(node.expression.elements), - b.member(b.id('$$slotProps'), b.id(node.name)) - ), - b.return(b.object(bindings.map((binding) => b.init(binding.node.name, binding.node)))) - ]) - ) - ) - ); -} diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js index 622e8138a29b..66ad5ed72c91 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js @@ -19,7 +19,6 @@ export function RegularElement(node, context) { /** @type {ComponentServerTransformState} */ const state = { ...context.state, - getters: { ...context.state.getters }, namespace, preserve_whitespace: context.state.preserve_whitespace || diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js index d226e7bd270a..4111b378618f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js @@ -15,9 +15,6 @@ export function SlotElement(node, context) { /** @type {Expression[]} */ const spreads = []; - /** @type {ExpressionStatement[]} */ - const lets = []; - /** @type {Expression} */ let expression = b.call('$.default_slot', b.id('$$props')); @@ -30,20 +27,11 @@ export function SlotElement(node, context) { if (attribute.name === 'name') { expression = b.member(b.member_id('$$props.$$slots'), value, true, true); } else if (attribute.name !== 'slot') { - if (attribute.metadata.dynamic) { - props.push(b.get(attribute.name, [b.return(value)])); - } else { - props.push(b.init(attribute.name, value)); - } + props.push(b.init(attribute.name, value)); } - } else if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); } } - // Let bindings first, they can be used on attributes - context.state.init.push(...lets); - const props_expression = spreads.length === 0 ? b.object(props) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js index 20422b635f69..35fda9823539 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js @@ -27,7 +27,6 @@ export function SvelteElement(node, context) { const state = { ...context.state, - getteres: { ...context.state.getters }, namespace: determine_namespace_for_children(node, context.state.namespace), template: [], init: [] diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteFragment.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteFragment.js index 310cc567a976..1b73ae7cfc6d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteFragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteFragment.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, ExpressionStatement } from 'estree' */ +/** @import { BlockStatement } from 'estree' */ /** @import { SvelteFragment } from '#compiler' */ /** @import { ComponentContext } from '../types' */ @@ -7,20 +7,5 @@ * @param {ComponentContext} context */ export function SvelteFragment(node, context) { - const child_state = { - ...context.state, - getters: { ...context.state.getters } - }; - - for (const attribute of node.attributes) { - if (attribute.type === 'LetDirective') { - context.state.template.push( - /** @type {ExpressionStatement} */ (context.visit(attribute, child_state)) - ); - } - } - - const block = /** @type {BlockStatement} */ (context.visit(node.fragment, child_state)); - - context.state.template.push(block); + context.state.template.push(/** @type {BlockStatement} */ (context.visit(node.fragment))); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js index 9fb30f7d9189..19e0fd6e43bd 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js @@ -1,5 +1,5 @@ -/** @import { BlockStatement, Expression, ExpressionStatement, Property, Statement } from 'estree' */ -/** @import { Attribute, Component, SvelteComponent, SvelteSelf, TemplateNode, Text } from '#compiler' */ +/** @import { BlockStatement, Expression, Pattern, Property, Statement } from 'estree' */ +/** @import { Attribute, Component, LetDirective, SvelteComponent, SvelteSelf, TemplateNode, Text } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { empty_comment, serialize_attribute_value } from './utils.js'; import * as b from '../../../../../utils/builders.js'; @@ -17,8 +17,8 @@ export function serialize_inline_component(node, expression, context) { /** @type {Property[]} */ const custom_css_props = []; - /** @type {ExpressionStatement[]} */ - const lets = []; + /** @type {Record} */ + const lets = { default: [] }; /** @type {Record} */ const children = {}; @@ -27,7 +27,9 @@ export function serialize_inline_component(node, expression, context) { * If this component has a slot property, it is a named slot within another component. In this case * the slot scope applies to the component itself, too, and not just its children. */ - let slot_scope_applies_to_itself = false; + const slot_scope_applies_to_itself = node.attributes.some( + (node) => node.type === 'Attribute' && node.name === 'slot' + ); /** * Components may have a children prop and also have child nodes. In this case, we assume @@ -50,7 +52,9 @@ export function serialize_inline_component(node, expression, context) { } for (const attribute of node.attributes) { if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + if (!slot_scope_applies_to_itself) { + lets.default.push(attribute); + } } else if (attribute.type === 'SpreadAttribute') { props_and_spreads.push(/** @type {Expression} */ (context.visit(attribute))); } else if (attribute.type === 'Attribute') { @@ -60,10 +64,6 @@ export function serialize_inline_component(node, expression, context) { continue; } - if (attribute.name === 'slot') { - slot_scope_applies_to_itself = true; - } - if (attribute.name === 'children') { has_children_prop = true; } @@ -90,10 +90,6 @@ export function serialize_inline_component(node, expression, context) { } } - if (slot_scope_applies_to_itself) { - context.state.init.push(...lets); - } - /** @type {Statement[]} */ const snippet_declarations = []; @@ -115,13 +111,20 @@ export function serialize_inline_component(node, expression, context) { let slot_name = 'default'; if (is_element_node(child)) { - const attribute = /** @type {Attribute | undefined} */ ( + const slot = /** @type {Attribute | undefined} */ ( child.attributes.find( (attribute) => attribute.type === 'Attribute' && attribute.name === 'slot' ) ); - if (attribute !== undefined) { - slot_name = /** @type {Text[]} */ (attribute.value)[0].data; + + if (slot !== undefined) { + slot_name = /** @type {Text[]} */ (slot.value)[0].data; + + lets[slot_name] = child.attributes.filter((attribute) => attribute.type === 'LetDirective'); + } else if (child.type === 'SvelteFragment') { + lets.default.push( + ...child.attributes.filter((attribute) => attribute.type === 'LetDirective') + ); } } @@ -152,16 +155,40 @@ export function serialize_inline_component(node, expression, context) { if (block.body.length === 0) continue; - const slot_fn = b.arrow( - [b.id('$$payload'), b.id('$$slotProps')], - b.block([ - ...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), - ...block.body - ]) - ); + /** @type {Pattern[]} */ + const params = [b.id('$$payload')]; + + if (lets[slot_name].length > 0) { + const pattern = b.object_pattern( + lets[slot_name].map((node) => { + if (node.expression === null) { + return b.init(node.name, b.id(node.name)); + } + + if (node.expression.type === 'ObjectExpression') { + // @ts-expect-error it gets parsed as an `ObjectExpression` but is really an `ObjectPattern` + return b.init(node.name, b.object_pattern(node.expression.properties)); + } + + if (node.expression.type === 'ArrayExpression') { + // @ts-expect-error it gets parsed as an `ArrayExpression` but is really an `ArrayPattern` + return b.init(node.name, b.array_pattern(node.expression.elements)); + } + + return b.init(node.name, node.expression); + }) + ); + + params.push(pattern); + } + + const slot_fn = b.arrow(params, b.block(block.body)); if (slot_name === 'default' && !has_children_prop) { - if (lets.length === 0 && children.default.every((node) => node.type !== 'SvelteFragment')) { + if ( + lets.default.length === 0 && + children.default.every((node) => node.type !== 'SvelteFragment') + ) { // create `children` prop... push_prop(b.prop('init', b.id('children'), slot_fn)); 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 923cc5d51259..fbfa4497b096 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 @@ -38,9 +38,6 @@ export function serialize_element_attributes(node, context) { /** @type {StyleDirective[]} */ const style_directives = []; - /** @type {ExpressionStatement[]} */ - const lets = []; - /** @type {Expression | null} */ let content = null; @@ -185,7 +182,7 @@ export function serialize_element_attributes(node, context) { } else if (attribute.type === 'StyleDirective') { style_directives.push(attribute); } else if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + // do nothing, these are handled inside `serialize_inline_component` } else { context.visit(attribute); } @@ -212,9 +209,6 @@ export function serialize_element_attributes(node, context) { } } - // Let bindings first, they can be used on attributes - context.state.init.push(...lets); - if (has_spread) { serialize_element_spread_attributes( node, 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 1d48019de65e..94f2bd52b734 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 @@ -225,11 +225,6 @@ export function serialize_get_binding(node, state) { ); } - if (Object.hasOwn(state.getters, node.name)) { - const getter = state.getters[node.name]; - return typeof getter === 'function' ? getter(node) : getter; - } - return node; } diff --git a/packages/svelte/src/compiler/phases/3-transform/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/types.d.ts index cd0acaf995ff..a16fd73a2d2c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/types.d.ts @@ -1,16 +1,10 @@ import type { Scope } from '../scope.js'; import type { SvelteNode, ValidatedModuleCompileOptions } from '#compiler'; import type { Analysis } from '../types.js'; -import type { Expression, Identifier } from 'estree'; export interface TransformState { readonly analysis: Analysis; readonly options: ValidatedModuleCompileOptions; readonly scope: Scope; readonly scopes: Map; - /** - * A map of `[name, node]` pairs, where `Identifier` nodes matching `name` - * will be replaced with `node` (e.g. `x` -> `$.get(x)`) - */ - readonly getters: Record Expression)>; } diff --git a/packages/svelte/src/compiler/utils/builders.js b/packages/svelte/src/compiler/utils/builders.js index 85d7c9471dd3..de087ffc8c27 100644 --- a/packages/svelte/src/compiler/utils/builders.js +++ b/packages/svelte/src/compiler/utils/builders.js @@ -320,10 +320,11 @@ export function object(properties) { } /** - * @param {Array} properties + * @param {Array} properties * @returns {ESTree.ObjectPattern} */ export function object_pattern(properties) { + // @ts-expect-error the types appear to be wrong return { type: 'ObjectPattern', properties }; } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index a6d70d21de71..ee69621a8885 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -428,12 +428,10 @@ export async function value_or_fallback_async(value, fallback) { * @returns {void} */ export function slot(payload, slot_fn, slot_props, fallback_fn) { - if (slot_fn === undefined) { - if (fallback_fn !== null) { - fallback_fn(); - } - } else { + if (slot_fn !== undefined) { slot_fn(payload, slot_props); + } else { + fallback_fn?.(); } } diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js index 661c34a48d50..05b343d82161 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js @@ -13,7 +13,7 @@ export default function Function_prop_no_getter($$payload) { onmousedown: () => count += 1, onmouseup, onmouseenter: () => count = plusOne(count), - children: ($$payload, $$slotProps) => { + children: ($$payload) => { $$payload.out += `clicks: ${$.escape(count)}`; }, $$slots: { default: true }