From 57f197705131bc5c17b1c5d7ae978e556b37f7b7 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Thu, 28 Dec 2023 06:30:30 -0800 Subject: [PATCH] inline into template attributes --- .changeset/twelve-peaches-jam.md | 2 +- .../phases/3-transform/client/types.d.ts | 8 +- .../phases/3-transform/client/utils.js | 18 +++ .../client/visitors/javascript-runes.js | 17 ++- .../3-transform/client/visitors/template.js | 117 +++++++++++++----- .../_expected/client/index.svelte.js | 6 +- .../_expected/server/index.svelte.js | 4 +- .../samples/hoist-unmodified-var/index.svelte | 4 +- 8 files changed, 127 insertions(+), 49 deletions(-) diff --git a/.changeset/twelve-peaches-jam.md b/.changeset/twelve-peaches-jam.md index db5cf85bb5..062448dbd5 100644 --- a/.changeset/twelve-peaches-jam.md +++ b/.changeset/twelve-peaches-jam.md @@ -2,4 +2,4 @@ 'svelte': patch --- -perf: hoist variables which are not mutated or reassigned +perf: hoist primitives which are not mutated or reassigned and inline into template attributes 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 d94e1f9fe6..2de307c4d6 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'; @@ -45,7 +46,10 @@ export interface ComponentClientTransformState extends ClientTransformState { /** Stuff that happens after the render effect (bindings, actions) */ readonly after_update: Statement[]; /** The HTML template string */ - readonly template: string[]; + readonly template: { + quasi: string[]; + expressions: Expression[]; + }; readonly metadata: { namespace: Namespace; /** `true` if the HTML template needs to be instantiated with `importNode` */ diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index b2928105e1..eac059e3b6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -532,3 +532,21 @@ export function should_proxy_or_freeze(node) { return true; } + +/** + * @param {import('#compiler').Binding | undefined} binding + * @param {import('#compiler').SvelteNode[]} path + * @returns {boolean} + */ +export function is_hoistable_declaration(binding, path) { + const is_top_level = path.at(-1)?.type === 'Program'; + // TODO: allow object expressions that are not passed to functions or components as props + // and expressions as long as they do not reference non-hoistable variables + return ( + is_top_level && + !!binding && + !binding.mutated && + !binding.reassigned && + binding?.initial?.type === 'Literal' + ); +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index eb8452bbf3..66cee37e41 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -2,7 +2,12 @@ import { get_rune } from '../../../scope.js'; import { is_hoistable_function, transform_inspect_rune } from '../../utils.js'; import * as b from '../../../../utils/builders.js'; import * as assert from '../../../../utils/assert.js'; -import { create_state_declarators, get_prop_source, should_proxy_or_freeze } from '../utils.js'; +import { + create_state_declarators, + get_prop_source, + is_hoistable_declaration, + should_proxy_or_freeze +} from '../utils.js'; import { unwrap_ts_expression } from '../../../../utils/ast.js'; /** @type {import('../types.js').ComponentVisitors} */ @@ -161,15 +166,7 @@ export const javascript_visitors_runes = { if (!rune && init != null && declarator.id.type === 'Identifier') { const is_top_level = path.at(-1)?.type === 'Program'; const binding = state.scope.owner(declarator.id.name)?.declarations.get(declarator.id.name); - // TODO: allow object expressions that are not passed to functions or components as props - // and expressions as long as they do not reference non-hoistable variables - if ( - is_top_level && - binding && - !binding.mutated && - !binding.reassigned && - binding?.initial?.type === 'Literal' - ) { + if (is_hoistable_declaration(binding, path)) { state.hoisted.push(b.declaration('const', declarator.id, init)); continue; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 30461ca169..a05ffae3a0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -19,6 +19,7 @@ import { error } from '../../../../errors.js'; import { function_visitor, get_assignment_value, + is_hoistable_declaration, serialize_get_binding, serialize_set_binding } from '../utils.js'; @@ -35,6 +36,33 @@ import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js'; +/** + * @param {import('../types.js').ComponentClientTransformState} state + * @param {string} quasi_to_add + * @returns + */ +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 + */ +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(''); +} + /** * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element * @param {import('#compiler').Attribute} attribute @@ -536,7 +564,18 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu } }; - if (attribute.metadata.dynamic) { + let is_in_hoistable = false; + if (Array.isArray(attribute.value)) { + for (let value of attribute.value) { + if (value.type === 'ExpressionTag' && value.expression.type === 'Identifier') { + const binding = context.state.scope + .owner(value.expression.name) + ?.declarations.get(value.expression.name); + is_in_hoistable ||= is_hoistable_declaration(binding, context.path); + } + } + } + if (attribute.metadata.dynamic && !is_in_hoistable) { const id = state.scope.generate(`${node_id.name}_${name}`); serialize_update_assignment( state, @@ -548,7 +587,9 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu ); return true; } else { - state.init.push(assign(grouped_value).grouped); + push_template_quasi(context.state, ` ${name}="`); + push_template_expression(context.state, grouped_value); + push_template_quasi(context.state, `"`); return false; } } @@ -1042,7 +1083,10 @@ function create_block(parent, name, nodes, context) { update: [], update_effects: [], after_update: [], - template: [], + template: { + quasi: [], + expressions: [] + }, metadata: { template_needs_import_node: false, namespace, @@ -1067,7 +1111,16 @@ function create_block(parent, name, nodes, context) { const callee = namespace === 'svg' ? '$.svg_template' : '$.template'; context.state.hoisted.push( - b.var(template_name, b.call(callee, b.template([b.quasi(state.template.join(''), true)], []))) + b.var( + template_name, + b.call( + callee, + b.template( + state.template.quasi.map((quasi) => b.quasi(quasi, true)), + state.template.expressions + ) + ) + ) ); body.push( @@ -1095,10 +1148,10 @@ function create_block(parent, name, nodes, context) { state }); - const template = state.template[0]; + const quasi = state.template.quasi[0]; - if (state.template.length === 1 && (template === ' ' || template === '')) { - if (template === ' ') { + if (state.template.quasi.length === 1 && (quasi === ' ' || quasi === '')) { + if (quasi === ' ') { body.push(b.var(node_id, b.call('$.space', b.id('$$anchor'))), ...state.init); close = b.stmt(b.call('$.close', b.id('$$anchor'), node_id)); } else { @@ -1115,7 +1168,14 @@ function create_block(parent, name, nodes, context) { state.hoisted.push( b.var( template_name, - b.call(callee, b.template([b.quasi(state.template.join(''), true)], []), b.true) + b.call( + callee, + b.template( + state.template.quasi.map((quasi) => b.quasi(quasi, true)), + state.template.expressions + ), + b.true + ) ) ); @@ -1433,11 +1493,11 @@ function process_children(nodes, parent, { visit, state }) { } if (node.type === 'Text') { - state.template.push(node.raw); + push_template_quasi(state, node.raw); return; } - state.template.push(' '); + push_template_quasi(state, ' '); const text_id = get_node_id(expression, state, 'text'); const singular = b.stmt( @@ -1479,7 +1539,7 @@ function process_children(nodes, parent, { visit, state }) { return; } - state.template.push(' '); + push_template_quasi(state, ' '); const text_id = get_node_id(expression, state, 'text'); const contains_call_expression = sequence.some( @@ -1659,10 +1719,10 @@ export const template_visitors = { }, 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(``); + push_template_quasi(context.state, ``); }, 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( @@ -1750,7 +1810,7 @@ export const template_visitors = { ); }, RenderTag(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); const binding = context.state.scope.get(node.expression.name); const is_reactive = binding?.kind !== 'normal' || node.expression.type !== 'Identifier'; @@ -1823,7 +1883,7 @@ export const template_visitors = { }, RegularElement(node, context) { if (node.name === 'noscript') { - context.state.template.push(''); + push_template_quasi(context.state, ''); return; } @@ -1833,7 +1893,7 @@ export const template_visitors = { namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) }; - context.state.template.push(`<${node.name}`); + push_template_quasi(context.state, `<${node.name}`); /** @type {Array} */ const attributes = []; @@ -1974,7 +2034,8 @@ export const template_visitors = { if (name !== 'class' || literal_value) { // TODO namespace=foreign probably doesn't want to do template stuff at all and instead use programmatic methods // to create the elements it needs. - context.state.template.push( + push_template_quasi( + context.state, ` ${attribute.name}${ DOMBooleanAttributes.includes(name) && literal_value === true ? '' @@ -1997,7 +2058,7 @@ export const template_visitors = { serialize_class_directives(class_directives, node_id, context, is_attributes_reactive); serialize_style_directives(style_directives, node_id, context, is_attributes_reactive); - context.state.template.push('>'); + push_template_quasi(context.state, '>'); /** @type {import('../types').ComponentClientTransformState} */ const state = { @@ -2037,11 +2098,11 @@ export const template_visitors = { ); if (!VoidElements.includes(node.name)) { - context.state.template.push(``); + push_template_quasi(context.state, ``); } }, SvelteElement(node, context) { - context.state.template.push(``); + push_template_quasi(context.state, ``); /** @type {Array} */ const attributes = []; @@ -2152,7 +2213,7 @@ export const template_visitors = { let each_item_is_reactive = true; if (!each_node_meta.is_controlled) { - context.state.template.push(''); + push_template_quasi(context.state, ''); } if (each_node_meta.array_name !== null) { @@ -2371,7 +2432,7 @@ export const template_visitors = { } }, IfBlock(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); const consequent = /** @type {import('estree').BlockStatement} */ ( context.visit(node.consequent) @@ -2395,7 +2456,7 @@ export const template_visitors = { ); }, AwaitBlock(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); context.state.after_update.push( b.stmt( @@ -2436,7 +2497,7 @@ export const template_visitors = { ); }, KeyBlock(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); const key = /** @type {import('estree').Expression} */ (context.visit(node.expression)); const body = /** @type {import('estree').Expression} */ (context.visit(node.fragment)); context.state.after_update.push( @@ -2767,7 +2828,7 @@ export const template_visitors = { } }, Component(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); const binding = context.state.scope.get( node.name.includes('.') ? node.name.slice(0, node.name.indexOf('.')) : node.name @@ -2795,12 +2856,12 @@ export const template_visitors = { context.state.after_update.push(component); }, SvelteSelf(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); const component = serialize_inline_component(node, context.state.analysis.name, context); context.state.after_update.push(component); }, SvelteComponent(node, context) { - context.state.template.push(''); + push_template_quasi(context.state, ''); let component = serialize_inline_component(node, '$$component', context); if (context.state.options.dev) { @@ -2895,7 +2956,7 @@ export const template_visitors = { }, SlotElement(node, context) { // fallback --> $.slot($$slots.default, { get a() { .. } }, () => ...fallback); - context.state.template.push(''); + push_template_quasi(context.state, ''); /** @type {import('estree').Property[]} */ const props = []; diff --git a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/client/index.svelte.js index 48d21db804..e83d408e1a 100644 --- a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/client/index.svelte.js @@ -3,17 +3,15 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -const count = 0; -var frag = $.template(`

`); +const boolean = false; +var frag = $.template(`

hello world

`); export default function Hoist_unmodified_var($$anchor, $$props) { $.push($$props, true); /* Init */ var p = $.open($$anchor, true, frag); - var text = $.child(p); - text.nodeValue = $.stringify(count); $.close($$anchor, p); $.pop(); } diff --git a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/server/index.svelte.js index b9ae604e22..39bd268b6f 100644 --- a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/_expected/server/index.svelte.js @@ -5,8 +5,8 @@ import * as $ from "svelte/internal/server"; export default function Hoist_unmodified_var($$payload, $$props) { $.push(true); - let count = 0; + let boolean = false; - $$payload.out += `

${$.escape_text(count)}

`; + $$payload.out += `hello world

`; $.pop(); } diff --git a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/index.svelte b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/index.svelte index fffe06f9af..91aab4b137 100644 --- a/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/index.svelte +++ b/packages/svelte/tests/snapshot/samples/hoist-unmodified-var/index.svelte @@ -1,7 +1,7 @@ -

{count}

+

hello world