Skip to content

feat: better code generation for let: directives in SSR mode #12611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stupid-bottles-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: better code generation for `let:` directives in SSR mode
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -22,6 +23,11 @@ export interface ClientTransformState extends TransformState {

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

export interface ComponentClientTransformState extends ClientTransformState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -79,7 +78,6 @@ const template_visitors = {
HtmlTag,
IfBlock,
KeyBlock,
LetDirective,
RegularElement,
RenderTag,
SlotElement,
Expand Down Expand Up @@ -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
};

Expand Down Expand Up @@ -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} */ (
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));

Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { BlockStatement, ExpressionStatement } from 'estree' */
/** @import { BlockStatement } from 'estree' */
/** @import { SvelteFragment } from '#compiler' */
/** @import { ComponentContext } from '../types' */

Expand All @@ -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)));
}
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -17,8 +17,8 @@ export function serialize_inline_component(node, expression, context) {
/** @type {Property[]} */
const custom_css_props = [];

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

/** @type {Record<string, TemplateNode[]>} */
const children = {};
Expand All @@ -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
Expand All @@ -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') {
Expand All @@ -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;
}
Expand All @@ -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 = [];

Expand All @@ -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')
);
}
}

Expand Down Expand Up @@ -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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 0 additions & 6 deletions packages/svelte/src/compiler/phases/3-transform/types.d.ts
Original file line number Diff line number Diff line change
@@ -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<SvelteNode, Scope>;
/**
* A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
* will be replaced with `node` (e.g. `x` -> `$.get(x)`)
*/
readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
}
3 changes: 2 additions & 1 deletion packages/svelte/src/compiler/utils/builders.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,11 @@ export function object(properties) {
}

/**
* @param {Array<ESTree.RestElement | ESTree.AssignmentProperty>} properties
* @param {Array<ESTree.RestElement | ESTree.AssignmentProperty | ESTree.Property>} properties
* @returns {ESTree.ObjectPattern}
*/
export function object_pattern(properties) {
// @ts-expect-error the types appear to be wrong
return { type: 'ObjectPattern', properties };
}

Expand Down
Loading
Loading