Skip to content

Commit c66d2cf

Browse files
authored
feat: better code generation for let: directives in SSR mode (#12611)
* better code generation for slot props in SSR * simplify * remove getters mechanism from server compiler * changeset * no need to use getters in SSR mode * fix comment
1 parent beea5c3 commit c66d2cf

File tree

15 files changed

+75
-128
lines changed

15 files changed

+75
-128
lines changed

.changeset/stupid-bottles-lay.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
feat: better code generation for `let:` directives in SSR mode

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import type {
33
Statement,
44
LabeledStatement,
55
Identifier,
6-
PrivateIdentifier
6+
PrivateIdentifier,
7+
Expression
78
} from 'estree';
89
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
910
import type { TransformState } from '../types.js';
@@ -22,6 +23,11 @@ export interface ClientTransformState extends TransformState {
2223

2324
/** The $: calls, which will be ordered in the end */
2425
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
26+
/**
27+
* A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
28+
* will be replaced with `node` (e.g. `x` -> `$.get(x)`)
29+
*/
30+
readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
2531
}
2632

2733
export interface ComponentClientTransformState extends ClientTransformState {

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import { Identifier } from './visitors/Identifier.js';
2323
import { IfBlock } from './visitors/IfBlock.js';
2424
import { KeyBlock } from './visitors/KeyBlock.js';
2525
import { LabeledStatementLegacy } from './visitors/LabeledStatement.js';
26-
import { LetDirective } from './visitors/LetDirective.js';
2726
import { MemberExpressionRunes } from './visitors/MemberExpression.js';
2827
import { PropertyDefinitionRunes } from './visitors/PropertyDefinition.js';
2928
import { RegularElement } from './visitors/RegularElement.js';
@@ -79,7 +78,6 @@ const template_visitors = {
7978
HtmlTag,
8079
IfBlock,
8180
KeyBlock,
82-
LetDirective,
8381
RegularElement,
8482
RenderTag,
8583
SlotElement,
@@ -113,7 +111,6 @@ export function server_component(analysis, options) {
113111
namespace: options.namespace,
114112
preserve_whitespace: options.preserveWhitespace,
115113
private_derived: new Map(),
116-
getters: {},
117114
skip_hydration_boundaries: false
118115
};
119116

@@ -422,8 +419,7 @@ export function server_module(analysis, options) {
422419
// to be present for `javascript_visitors_legacy` and so is included in module
423420
// transform state as well as component transform state
424421
legacy_reactive_statements: new Map(),
425-
private_derived: new Map(),
426-
getters: {}
422+
private_derived: new Map()
427423
};
428424

429425
const module = /** @type {Program} */ (

packages/svelte/src/compiler/phases/3-transform/server/visitors/LetDirective.js

Lines changed: 0 additions & 40 deletions
This file was deleted.

packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export function RegularElement(node, context) {
1919
/** @type {ComponentServerTransformState} */
2020
const state = {
2121
...context.state,
22-
getters: { ...context.state.getters },
2322
namespace,
2423
preserve_whitespace:
2524
context.state.preserve_whitespace ||

packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ export function SlotElement(node, context) {
1515
/** @type {Expression[]} */
1616
const spreads = [];
1717

18-
/** @type {ExpressionStatement[]} */
19-
const lets = [];
20-
2118
/** @type {Expression} */
2219
let expression = b.call('$.default_slot', b.id('$$props'));
2320

@@ -30,20 +27,11 @@ export function SlotElement(node, context) {
3027
if (attribute.name === 'name') {
3128
expression = b.member(b.member_id('$$props.$$slots'), value, true, true);
3229
} else if (attribute.name !== 'slot') {
33-
if (attribute.metadata.dynamic) {
34-
props.push(b.get(attribute.name, [b.return(value)]));
35-
} else {
36-
props.push(b.init(attribute.name, value));
37-
}
30+
props.push(b.init(attribute.name, value));
3831
}
39-
} else if (attribute.type === 'LetDirective') {
40-
lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute)));
4132
}
4233
}
4334

44-
// Let bindings first, they can be used on attributes
45-
context.state.init.push(...lets);
46-
4735
const props_expression =
4836
spreads.length === 0
4937
? b.object(props)

packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ export function SvelteElement(node, context) {
2727

2828
const state = {
2929
...context.state,
30-
getteres: { ...context.state.getters },
3130
namespace: determine_namespace_for_children(node, context.state.namespace),
3231
template: [],
3332
init: []
Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { BlockStatement, ExpressionStatement } from 'estree' */
1+
/** @import { BlockStatement } from 'estree' */
22
/** @import { SvelteFragment } from '#compiler' */
33
/** @import { ComponentContext } from '../types' */
44

@@ -7,20 +7,5 @@
77
* @param {ComponentContext} context
88
*/
99
export function SvelteFragment(node, context) {
10-
const child_state = {
11-
...context.state,
12-
getters: { ...context.state.getters }
13-
};
14-
15-
for (const attribute of node.attributes) {
16-
if (attribute.type === 'LetDirective') {
17-
context.state.template.push(
18-
/** @type {ExpressionStatement} */ (context.visit(attribute, child_state))
19-
);
20-
}
21-
}
22-
23-
const block = /** @type {BlockStatement} */ (context.visit(node.fragment, child_state));
24-
25-
context.state.template.push(block);
10+
context.state.template.push(/** @type {BlockStatement} */ (context.visit(node.fragment)));
2611
}

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
/** @import { BlockStatement, Expression, ExpressionStatement, Property, Statement } from 'estree' */
2-
/** @import { Attribute, Component, SvelteComponent, SvelteSelf, TemplateNode, Text } from '#compiler' */
1+
/** @import { BlockStatement, Expression, Pattern, Property, Statement } from 'estree' */
2+
/** @import { Attribute, Component, LetDirective, SvelteComponent, SvelteSelf, TemplateNode, Text } from '#compiler' */
33
/** @import { ComponentContext } from '../../types.js' */
44
import { empty_comment, serialize_attribute_value } from './utils.js';
55
import * as b from '../../../../../utils/builders.js';
@@ -17,8 +17,8 @@ export function serialize_inline_component(node, expression, context) {
1717
/** @type {Property[]} */
1818
const custom_css_props = [];
1919

20-
/** @type {ExpressionStatement[]} */
21-
const lets = [];
20+
/** @type {Record<string, LetDirective[]>} */
21+
const lets = { default: [] };
2222

2323
/** @type {Record<string, TemplateNode[]>} */
2424
const children = {};
@@ -27,7 +27,9 @@ export function serialize_inline_component(node, expression, context) {
2727
* If this component has a slot property, it is a named slot within another component. In this case
2828
* the slot scope applies to the component itself, too, and not just its children.
2929
*/
30-
let slot_scope_applies_to_itself = false;
30+
const slot_scope_applies_to_itself = node.attributes.some(
31+
(node) => node.type === 'Attribute' && node.name === 'slot'
32+
);
3133

3234
/**
3335
* 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) {
5052
}
5153
for (const attribute of node.attributes) {
5254
if (attribute.type === 'LetDirective') {
53-
lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute)));
55+
if (!slot_scope_applies_to_itself) {
56+
lets.default.push(attribute);
57+
}
5458
} else if (attribute.type === 'SpreadAttribute') {
5559
props_and_spreads.push(/** @type {Expression} */ (context.visit(attribute)));
5660
} else if (attribute.type === 'Attribute') {
@@ -60,10 +64,6 @@ export function serialize_inline_component(node, expression, context) {
6064
continue;
6165
}
6266

63-
if (attribute.name === 'slot') {
64-
slot_scope_applies_to_itself = true;
65-
}
66-
6767
if (attribute.name === 'children') {
6868
has_children_prop = true;
6969
}
@@ -90,10 +90,6 @@ export function serialize_inline_component(node, expression, context) {
9090
}
9191
}
9292

93-
if (slot_scope_applies_to_itself) {
94-
context.state.init.push(...lets);
95-
}
96-
9793
/** @type {Statement[]} */
9894
const snippet_declarations = [];
9995

@@ -115,13 +111,20 @@ export function serialize_inline_component(node, expression, context) {
115111

116112
let slot_name = 'default';
117113
if (is_element_node(child)) {
118-
const attribute = /** @type {Attribute | undefined} */ (
114+
const slot = /** @type {Attribute | undefined} */ (
119115
child.attributes.find(
120116
(attribute) => attribute.type === 'Attribute' && attribute.name === 'slot'
121117
)
122118
);
123-
if (attribute !== undefined) {
124-
slot_name = /** @type {Text[]} */ (attribute.value)[0].data;
119+
120+
if (slot !== undefined) {
121+
slot_name = /** @type {Text[]} */ (slot.value)[0].data;
122+
123+
lets[slot_name] = child.attributes.filter((attribute) => attribute.type === 'LetDirective');
124+
} else if (child.type === 'SvelteFragment') {
125+
lets.default.push(
126+
...child.attributes.filter((attribute) => attribute.type === 'LetDirective')
127+
);
125128
}
126129
}
127130

@@ -152,16 +155,40 @@ export function serialize_inline_component(node, expression, context) {
152155

153156
if (block.body.length === 0) continue;
154157

155-
const slot_fn = b.arrow(
156-
[b.id('$$payload'), b.id('$$slotProps')],
157-
b.block([
158-
...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []),
159-
...block.body
160-
])
161-
);
158+
/** @type {Pattern[]} */
159+
const params = [b.id('$$payload')];
160+
161+
if (lets[slot_name].length > 0) {
162+
const pattern = b.object_pattern(
163+
lets[slot_name].map((node) => {
164+
if (node.expression === null) {
165+
return b.init(node.name, b.id(node.name));
166+
}
167+
168+
if (node.expression.type === 'ObjectExpression') {
169+
// @ts-expect-error it gets parsed as an `ObjectExpression` but is really an `ObjectPattern`
170+
return b.init(node.name, b.object_pattern(node.expression.properties));
171+
}
172+
173+
if (node.expression.type === 'ArrayExpression') {
174+
// @ts-expect-error it gets parsed as an `ArrayExpression` but is really an `ArrayPattern`
175+
return b.init(node.name, b.array_pattern(node.expression.elements));
176+
}
177+
178+
return b.init(node.name, node.expression);
179+
})
180+
);
181+
182+
params.push(pattern);
183+
}
184+
185+
const slot_fn = b.arrow(params, b.block(block.body));
162186

163187
if (slot_name === 'default' && !has_children_prop) {
164-
if (lets.length === 0 && children.default.every((node) => node.type !== 'SvelteFragment')) {
188+
if (
189+
lets.default.length === 0 &&
190+
children.default.every((node) => node.type !== 'SvelteFragment')
191+
) {
165192
// create `children` prop...
166193
push_prop(b.prop('init', b.id('children'), slot_fn));
167194

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ export function serialize_element_attributes(node, context) {
3838
/** @type {StyleDirective[]} */
3939
const style_directives = [];
4040

41-
/** @type {ExpressionStatement[]} */
42-
const lets = [];
43-
4441
/** @type {Expression | null} */
4542
let content = null;
4643

@@ -185,7 +182,7 @@ export function serialize_element_attributes(node, context) {
185182
} else if (attribute.type === 'StyleDirective') {
186183
style_directives.push(attribute);
187184
} else if (attribute.type === 'LetDirective') {
188-
lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute)));
185+
// do nothing, these are handled inside `serialize_inline_component`
189186
} else {
190187
context.visit(attribute);
191188
}
@@ -212,9 +209,6 @@ export function serialize_element_attributes(node, context) {
212209
}
213210
}
214211

215-
// Let bindings first, they can be used on attributes
216-
context.state.init.push(...lets);
217-
218212
if (has_spread) {
219213
serialize_element_spread_attributes(
220214
node,

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,6 @@ export function serialize_get_binding(node, state) {
225225
);
226226
}
227227

228-
if (Object.hasOwn(state.getters, node.name)) {
229-
const getter = state.getters[node.name];
230-
return typeof getter === 'function' ? getter(node) : getter;
231-
}
232-
233228
return node;
234229
}
235230

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,10 @@
11
import type { Scope } from '../scope.js';
22
import type { SvelteNode, ValidatedModuleCompileOptions } from '#compiler';
33
import type { Analysis } from '../types.js';
4-
import type { Expression, Identifier } from 'estree';
54

65
export interface TransformState {
76
readonly analysis: Analysis;
87
readonly options: ValidatedModuleCompileOptions;
98
readonly scope: Scope;
109
readonly scopes: Map<SvelteNode, Scope>;
11-
/**
12-
* A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
13-
* will be replaced with `node` (e.g. `x` -> `$.get(x)`)
14-
*/
15-
readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
1610
}

packages/svelte/src/compiler/utils/builders.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,11 @@ export function object(properties) {
320320
}
321321

322322
/**
323-
* @param {Array<ESTree.RestElement | ESTree.AssignmentProperty>} properties
323+
* @param {Array<ESTree.RestElement | ESTree.AssignmentProperty | ESTree.Property>} properties
324324
* @returns {ESTree.ObjectPattern}
325325
*/
326326
export function object_pattern(properties) {
327+
// @ts-expect-error the types appear to be wrong
327328
return { type: 'ObjectPattern', properties };
328329
}
329330

0 commit comments

Comments
 (0)