Skip to content

perf: inline module variables into template #13075

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 8 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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/eighty-dragons-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

perf: inline module variables into template
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ export interface ComponentClientTransformState extends ClientTransformState {
/** Stuff that happens after the render effect (control blocks, dynamic elements, bindings, actions, etc) */
readonly after_update: Statement[];
/** The HTML template string */
readonly template: string[];
readonly template: {
quasi: string[];
expressions: Expression[];
};
readonly locations: SourceLocation[];
readonly metadata: {
namespace: Namespace;
Expand Down
40 changes: 40 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,43 @@ export function create_derived_block_argument(node, context) {
export function create_derived(state, arg) {
return b.call(state.analysis.runes ? '$.derived' : '$.derived_safe_equal', arg);
}

/**
* @param {import('./types.js').ComponentClientTransformState} state
* @param {string} quasi_to_add
*/
export function push_template_quasi(state, quasi_to_add) {
const { quasi } = state.template;
if (quasi.length === 0) {
quasi.push(quasi_to_add);
return;
}
quasi[quasi.length - 1] = quasi[quasi.length - 1].concat(quasi_to_add);
}

/**
* @param {import('./types.js').ComponentClientTransformState} state
* @param {import('estree').Expression} expression_to_add
*/
export function push_template_expression(state, expression_to_add) {
const { expressions, quasi } = state.template;
if (quasi.length === 0) {
quasi.push('');
}
expressions.push(expression_to_add);
quasi.push('');
}

/**
* Whether a variable can be referenced directly from template string.
* @param {import('#compiler').Binding | undefined} binding
* @returns {boolean}
*/
export function can_inline_variable(binding) {
return (
!!binding &&
!binding.scope.parent &&
// to prevent the need for escaping
binding.initial?.type === 'Literal'
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import * as b from '../../../../utils/builders.js';
import { create_derived_block_argument } from '../utils.js';
import { create_derived_block_argument, push_template_quasi } from '../utils.js';

/**
* @param {AST.AwaitBlock} node
* @param {ComponentContext} context
*/
export function AwaitBlock(node, context) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');

// Visit {#await <expression>} first to ensure that scopes are in the correct order
const expression = b.thunk(/** @type {Expression} */ (context.visit(node.expression)));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */

import { push_template_quasi } from '../utils.js';

/**
* @param {AST.Comment} node
* @param {ComponentContext} context
*/
export function Comment(node, context) {
// We'll only get here if comments are not filtered out, which they are unless preserveComments is true
context.state.template.push(`<!--${node.data}-->`);
push_template_quasi(context.state, `<!--${node.data}-->`);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { dev } from '../../../../state.js';
import { extract_paths, object } from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { build_getter } from '../utils.js';
import { build_getter, push_template_quasi } from '../utils.js';
import { get_value } from './shared/declarations.js';

/**
Expand All @@ -32,7 +32,7 @@ export function EachBlock(node, context) {
);

if (!each_node_meta.is_controlled) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');
}

if (each_node_meta.array_name !== null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ export function Fragment(node, context) {
init: [],
update: [],
after_update: [],
template: [],
template: {
quasi: [],
expressions: []
},
locations: [],
transform: { ...context.state.transform },
metadata: {
Expand Down Expand Up @@ -115,7 +118,12 @@ export function Fragment(node, context) {
});

/** @type {Expression[]} */
const args = [b.template([b.quasi(state.template.join(''), true)], [])];
const args = [
b.template(
state.template.quasi.map((q) => b.quasi(q, true)),
state.template.expressions
)
];

if (state.metadata.context.template_needs_import_node) {
args.push(b.literal(TEMPLATE_USE_IMPORT_NODE));
Expand Down Expand Up @@ -170,12 +178,15 @@ export function Fragment(node, context) {
flags |= TEMPLATE_USE_IMPORT_NODE;
}

if (state.template.length === 1 && state.template[0] === '<!>') {
if (state.template.quasi.length === 1 && state.template.quasi[0] === '<!>') {
// special case — we can use `$.comment` instead of creating a unique template
body.push(b.var(id, b.call('$.comment')));
} else {
add_template(template_name, [
b.template([b.quasi(state.template.join(''), true)], []),
b.template(
state.template.quasi.map((q) => b.quasi(q, true)),
state.template.expressions
),
b.literal(flags)
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
/** @import { ComponentContext } from '../types' */
import { is_ignored } from '../../../../state.js';
import * as b from '../../../../utils/builders.js';
import { push_template_quasi } from '../utils.js';

/**
* @param {AST.HtmlTag} node
* @param {ComponentContext} context
*/
export function HtmlTag(node, context) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');

// push into init, so that bindings run afterwards, which might trigger another run and override hydration
context.state.init.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import * as b from '../../../../utils/builders.js';
import { push_template_quasi } from '../utils.js';

/**
* @param {AST.IfBlock} node
* @param {ComponentContext} context
*/
export function IfBlock(node, context) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');

const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import * as b from '../../../../utils/builders.js';
import { push_template_quasi } from '../utils.js';

/**
* @param {AST.KeyBlock} node
* @param {ComponentContext} context
*/
export function KeyBlock(node, context) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');

const key = /** @type {Expression} */ (context.visit(node.expression));
const body = /** @type {Expression} */ (context.visit(node.fragment));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ import {
import * as b from '../../../../utils/builders.js';
import { is_custom_element_node } from '../../../nodes.js';
import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
import { build_getter } from '../utils.js';
import {
build_getter,
can_inline_variable,
push_template_expression,
push_template_quasi
} from '../utils.js';
import {
get_attribute_name,
build_attribute_value,
Expand Down Expand Up @@ -53,7 +58,7 @@ export function RegularElement(node, context) {
}

if (node.name === 'noscript') {
context.state.template.push('<noscript></noscript>');
push_template_quasi(context.state, '<noscript></noscript>');
return;
}

Expand All @@ -67,7 +72,7 @@ export function RegularElement(node, context) {
namespace: determine_namespace_for_children(node, context.state.metadata.namespace)
};

context.state.template.push(`<${node.name}`);
push_template_quasi(context.state, `<${node.name}`);

/** @type {Array<AST.Attribute | AST.SpreadAttribute>} */
const attributes = [];
Expand Down Expand Up @@ -241,7 +246,8 @@ export function RegularElement(node, context) {
const value = is_text_attribute(attribute) ? attribute.value[0].data : true;

if (name !== 'class' || value) {
context.state.template.push(
push_template_quasi(
context.state,
` ${attribute.name}${
is_boolean_attribute(name) && value === true
? ''
Expand Down Expand Up @@ -278,7 +284,7 @@ export function RegularElement(node, context) {
context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id)));
}

context.state.template.push('>');
push_template_quasi(context.state, '>');

/** @type {SourceLocation[]} */
const child_locations = [];
Expand Down Expand Up @@ -377,7 +383,7 @@ export function RegularElement(node, context) {
}

if (!is_void(node.name)) {
context.state.template.push(`</${node.name}>`);
push_template_quasi(context.state, `</${node.name}>`);
}
}

Expand Down Expand Up @@ -465,7 +471,7 @@ function build_element_spread_attributes(
value.type === 'Literal' &&
context.state.metadata.namespace === 'html'
) {
context.state.template.push(` is="${escape_html(value.value, true)}"`);
push_template_quasi(context.state, ` is="${escape_html(value.value, true)}"`);
continue;
}

Expand Down Expand Up @@ -607,6 +613,13 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
);
}

const { has_expression_tag, can_inline } =
attribute.value === true
? { has_expression_tag: false, can_inline: true }
: can_inline_all_nodes(
Array.isArray(attribute.value) ? attribute.value : [attribute.value],
context.state
);
if (attribute.metadata.expression.has_state) {
if (has_call) {
state.init.push(build_update(update));
Expand All @@ -615,11 +628,40 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
}
return true;
} else {
state.init.push(update);
if (has_expression_tag && can_inline) {
push_template_quasi(context.state, ` ${name}="`);
push_template_expression(context.state, value);
push_template_quasi(context.state, '"');
} else {
state.init.push(update);
}
return false;
}
}

/**
* @param {(AST.Text | AST.ExpressionTag)[]} nodes
* @param {import('../types.js').ComponentClientTransformState} state
*/
function can_inline_all_nodes(nodes, state) {
let can_inline = true;
let has_expression_tag = false;
for (let value of nodes) {
if (value.type === 'ExpressionTag') {
if (value.expression.type === 'Identifier') {
const binding = state.scope
.owner(value.expression.name)
?.declarations.get(value.expression.name);
can_inline &&= can_inline_variable(binding);
} else {
can_inline = false;
}
has_expression_tag = true;
}
}
return { can_inline, has_expression_tag };
}

/**
* Like `build_element_attribute_update_assignment` but without any special attribute treatment.
* @param {Identifier} node_id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
/** @import { ComponentContext } from '../types' */
import { unwrap_optional } from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { push_template_quasi } from '../utils.js';

/**
* @param {AST.RenderTag} node
* @param {ComponentContext} context
*/
export function RenderTag(node, context) {
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');
const callee = unwrap_optional(node.expression).callee;
const raw_args = unwrap_optional(node.expression).arguments;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import * as b from '../../../../utils/builders.js';
import { push_template_quasi } from '../utils.js';
import { build_attribute_value } from './shared/element.js';

/**
Expand All @@ -10,7 +11,7 @@ import { build_attribute_value } from './shared/element.js';
*/
export function SlotElement(node, context) {
// <slot {a}>fallback</slot> --> $.slot($$slots.default, { get a() { .. } }, () => ...fallback);
context.state.template.push('<!>');
push_template_quasi(context.state, '<!>');

/** @type {Property[]} */
const props = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { determine_namespace_for_children } from '../../utils.js';
import { push_template_quasi } from '../utils.js';
import {
build_attribute_value,
build_class_directives,
Expand All @@ -21,7 +22,7 @@ import { build_render_statement, build_update } from './shared/utils.js';
* @param {ComponentContext} context
*/
export function SvelteElement(node, context) {
context.state.template.push(`<!>`);
push_template_quasi(context.state, `<!>`);

/** @type {Array<AST.Attribute | AST.SpreadAttribute>} */
const attributes = [];
Expand Down
Loading