From eb530c82c4e8870194cb919be4bc76600f0e0231 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 30 Jun 2025 16:04:44 -0400 Subject: [PATCH 1/2] chore: move stuff from `analysis` into global compiler state (#16268) * break out locator stuff from the rest of global state * move some stuff around * tighten up * make runes globally available * make component_name globally available * unused --- packages/svelte/src/compiler/index.js | 14 ++++---- packages/svelte/src/compiler/migrate/index.js | 5 ++- .../src/compiler/phases/1-parse/index.js | 3 ++ .../src/compiler/phases/2-analyze/index.js | 17 +++++++++ .../svelte/src/compiler/phases/types.d.ts | 3 ++ packages/svelte/src/compiler/state.js | 35 +++++++++++++------ 6 files changed, 57 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/compiler/index.js b/packages/svelte/src/compiler/index.js index 429c6b4a6f..9ba23c1485 100644 --- a/packages/svelte/src/compiler/index.js +++ b/packages/svelte/src/compiler/index.js @@ -20,9 +20,8 @@ export { default as preprocess } from './preprocess/index.js'; */ export function compile(source, options) { source = remove_bom(source); - state.reset_warning_filter(options.warningFilter); + state.reset_warnings(options.warningFilter); const validated = validate_component_options(options, ''); - state.reset(source, validated); let parsed = _parse(source); @@ -64,9 +63,8 @@ export function compile(source, options) { */ export function compileModule(source, options) { source = remove_bom(source); - state.reset_warning_filter(options.warningFilter); + state.reset_warnings(options.warningFilter); const validated = validate_module_options(options, ''); - state.reset(source, validated); const analysis = analyze_module(source, validated); return transform_module(analysis, source, validated); @@ -96,6 +94,7 @@ export function compileModule(source, options) { * @returns {Record} */ +// TODO 6.0 remove unused `filename` /** * The parse function parses a component, returning only its abstract syntax tree. * @@ -104,14 +103,15 @@ export function compileModule(source, options) { * * The `loose` option, available since 5.13.0, tries to always return an AST even if the input will not successfully compile. * + * The `filename` option is unused and will be removed in Svelte 6.0. + * * @param {string} source * @param {{ filename?: string; rootDir?: string; modern?: boolean; loose?: boolean }} [options] * @returns {AST.Root | LegacyRoot} */ -export function parse(source, { filename, rootDir, modern, loose } = {}) { +export function parse(source, { modern, loose } = {}) { source = remove_bom(source); - state.reset_warning_filter(() => false); - state.reset(source, { filename: filename ?? '(unknown)', rootDir }); + state.reset_warnings(() => false); const ast = _parse(source, loose); return to_public_ast(source, ast, modern); diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 5ca9adb98b..fdc9734f85 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -9,7 +9,7 @@ import { parse } from '../phases/1-parse/index.js'; import { regex_valid_component_name } from '../phases/1-parse/state/element.js'; import { analyze_component } from '../phases/2-analyze/index.js'; import { get_rune } from '../phases/scope.js'; -import { reset, reset_warning_filter } from '../state.js'; +import { reset, reset_warnings } from '../state.js'; import { extract_identifiers, extract_all_identifiers_from_expression, @@ -134,8 +134,7 @@ export function migrate(source, { filename, use_ts } = {}) { return start + style_placeholder + end; }); - reset_warning_filter(() => false); - reset(source, { filename: filename ?? '(unknown)' }); + reset_warnings(() => false); let parsed = parse(source); diff --git a/packages/svelte/src/compiler/phases/1-parse/index.js b/packages/svelte/src/compiler/phases/1-parse/index.js index b8ae8199eb..77cc2bf3fa 100644 --- a/packages/svelte/src/compiler/phases/1-parse/index.js +++ b/packages/svelte/src/compiler/phases/1-parse/index.js @@ -9,6 +9,7 @@ import { create_fragment } from './utils/create.js'; import read_options from './read/options.js'; import { is_reserved } from '../../../utils.js'; import { disallow_children } from '../2-analyze/visitors/shared/special-element.js'; +import * as state from '../../state.js'; const regex_position_indicator = / \(\d+:\d+\)$/; @@ -301,6 +302,8 @@ export class Parser { * @returns {AST.Root} */ export function parse(template, loose = false) { + state.set_source(template); + const parser = new Parser(template, loose); return parser.root; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index be1f1ee5bb..d73e273ec2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -76,6 +76,7 @@ import { UseDirective } from './visitors/UseDirective.js'; import { VariableDeclarator } from './visitors/VariableDeclarator.js'; import is_reference from 'is-reference'; import { mark_subtree_dynamic } from './visitors/shared/fragment.js'; +import * as state from '../../state.js'; /** * @type {Visitors} @@ -240,6 +241,7 @@ export function analyze_module(source, options) { /** @type {AST.JSComment[]} */ const comments = []; + state.set_source(source); const ast = parse(source, comments, false, false); const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, null); @@ -269,6 +271,13 @@ export function analyze_module(source, options) { classes: new Map() }; + state.reset({ + dev: options.dev, + filename: options.filename, + rootDir: options.rootDir, + runes: true + }); + walk( /** @type {Node} */ (ast), { @@ -506,6 +515,14 @@ export function analyze_component(root, source, options) { snippets: new Set() }; + state.reset({ + component_name: analysis.name, + dev: options.dev, + filename: options.filename, + rootDir: options.rootDir, + runes: true + }); + if (!runes) { // every exported `let` or `var` declaration becomes a prop, everything else becomes an export for (const node of instance.ast.body) { diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 661f363991..dba2559a17 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -34,7 +34,9 @@ export interface ReactiveStatement { */ export interface Analysis { module: Js; + /** @deprecated use `component_name` from `state.js` instead */ name: string; // TODO should this be filename? it's used in `compileModule` as well as `compile` + /** @deprecated use `runes` from `state.js` instead */ runes: boolean; immutable: boolean; tracing: boolean; @@ -90,6 +92,7 @@ export interface ComponentAnalysis extends Analysis { keyframes: string[]; has_global: boolean; }; + /** @deprecated use `source` from `state.js` instead */ source: string; undefined_exports: Map; /** diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index 1db3db917f..9095651ced 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -16,6 +16,8 @@ export let warnings = []; */ export let filename; +export let component_name = ''; + /** * The original source code * @type {string} @@ -28,8 +30,16 @@ export let source; */ export let dev; +export let runes = false; + export let locator = getLocator('', { offsetLine: 1 }); +/** @param {string} value */ +export function set_source(value) { + source = value; + locator = getLocator(source, { offsetLine: 1 }); +} + /** * @param {AST.SvelteNode & { start?: number | undefined }} node */ @@ -71,8 +81,9 @@ export function pop_ignore() { * * @param {(warning: Warning) => boolean} fn */ -export function reset_warning_filter(fn = () => true) { +export function reset_warnings(fn = () => true) { warning_filter = fn; + warnings = []; } /** @@ -85,23 +96,27 @@ export function is_ignored(node, code) { } /** - * @param {string} _source - * @param {{ dev?: boolean; filename: string; rootDir?: string }} options + * @param {{ + * dev: boolean; + * filename: string; + * component_name?: string; + * rootDir?: string; + * runes: boolean; + * }} state */ -export function reset(_source, options) { - source = _source; - const root_dir = options.rootDir?.replace(/\\/g, '/'); - filename = options.filename.replace(/\\/g, '/'); +export function reset(state) { + const root_dir = state.rootDir?.replace(/\\/g, '/'); + filename = state.filename.replace(/\\/g, '/'); - dev = !!options.dev; + dev = state.dev; + runes = state.runes; + component_name = state.component_name ?? '(unknown)'; if (typeof root_dir === 'string' && filename.startsWith(root_dir)) { // make filename relative to rootDir filename = filename.replace(root_dir, '').replace(/^[/\\]/, ''); } - locator = getLocator(source, { offsetLine: 1 }); - warnings = []; ignore_stack = []; ignore_map.clear(); } From d427ffd8b96c5a5509ee602bd77d58a90f24e784 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 1 Jul 2025 10:37:37 -0400 Subject: [PATCH 2/2] chore: encapsulate expression memoization (#16269) * chore: encapsulate expression memoization * add comment * tweak * use b.id --- .../3-transform/client/transform-client.js | 4 +- .../phases/3-transform/client/types.d.ts | 5 +- .../3-transform/client/visitors/Fragment.js | 4 +- .../client/visitors/RegularElement.js | 61 ++++++++++--------- .../client/visitors/SvelteElement.js | 6 +- .../client/visitors/shared/element.js | 37 +++++------ .../client/visitors/shared/utils.js | 52 ++++++++++++---- 7 files changed, 99 insertions(+), 70 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index e85a35cf8e..a9c0651e0f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -168,9 +168,9 @@ export function client_component(analysis, options) { // these are set inside the `Fragment` visitor, and cannot be used until then init: /** @type {any} */ (null), update: /** @type {any} */ (null), - expressions: /** @type {any} */ (null), after_update: /** @type {any} */ (null), - template: /** @type {any} */ (null) + template: /** @type {any} */ (null), + memoizer: /** @type {any} */ (null) }; const module = /** @type {ESTree.Program} */ ( 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 2388ee1b00..cf5c942268 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 @@ -12,6 +12,7 @@ import type { AST, Namespace, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; import type { Template } from './transform-template/template.js'; +import type { Memoizer } from './visitors/shared/utils.js'; export interface ClientTransformState extends TransformState { /** @@ -49,8 +50,8 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly update: Statement[]; /** Stuff that happens after the render effect (control blocks, dynamic elements, bindings, actions, etc) */ readonly after_update: Statement[]; - /** Expressions used inside the render effect */ - readonly expressions: Expression[]; + /** Memoized expressions */ + readonly memoizer: Memoizer; /** The HTML template string */ readonly template: Template; readonly metadata: { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js index 4825184d31..7cd2bd90ed 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js @@ -6,7 +6,7 @@ import * as b from '#compiler/builders'; import { clean_nodes, infer_namespace } from '../../utils.js'; import { transform_template } from '../transform-template/index.js'; import { process_children } from './shared/fragment.js'; -import { build_render_statement } from './shared/utils.js'; +import { build_render_statement, Memoizer } from './shared/utils.js'; import { Template } from '../transform-template/template.js'; /** @@ -64,8 +64,8 @@ export function Fragment(node, context) { ...context.state, init: [], update: [], - expressions: [], after_update: [], + memoizer: new Memoizer(), template: new Template(), transform: { ...context.state.transform }, metadata: { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index ae8680f594..eed2a75506 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -22,7 +22,7 @@ import { build_set_style } from './shared/element.js'; import { process_children } from './shared/fragment.js'; -import { build_render_statement, build_template_chunk, get_expression_id } from './shared/utils.js'; +import { build_render_statement, build_template_chunk, Memoizer } from './shared/utils.js'; import { visit_event_attribute } from './shared/events.js'; /** @@ -253,8 +253,7 @@ export function RegularElement(node, context) { const { value, has_state } = build_attribute_value( attribute.value, context, - (value, metadata) => - metadata.has_call ? get_expression_id(context.state.expressions, value) : value + (value, metadata) => (metadata.has_call ? context.state.memoizer.add(value) : value) ); const update = build_element_attribute_update(node, node_id, name, value, attributes); @@ -455,11 +454,15 @@ function setup_select_synchronization(value_binding, context) { /** * @param {AST.ClassDirective[]} class_directives - * @param {Expression[]} expressions * @param {ComponentContext} context + * @param {Memoizer} memoizer * @return {ObjectExpression | Identifier} */ -export function build_class_directives_object(class_directives, expressions, context) { +export function build_class_directives_object( + class_directives, + context, + memoizer = context.state.memoizer +) { let properties = []; let has_call_or_state = false; @@ -471,38 +474,40 @@ export function build_class_directives_object(class_directives, expressions, con const directives = b.object(properties); - return has_call_or_state ? get_expression_id(expressions, directives) : directives; + return has_call_or_state ? memoizer.add(directives) : directives; } /** * @param {AST.StyleDirective[]} style_directives - * @param {Expression[]} expressions * @param {ComponentContext} context - * @return {ObjectExpression | ArrayExpression}} + * @param {Memoizer} memoizer + * @return {ObjectExpression | ArrayExpression | Identifier}} */ -export function build_style_directives_object(style_directives, expressions, context) { - let normal_properties = []; - let important_properties = []; +export function build_style_directives_object( + style_directives, + context, + memoizer = context.state.memoizer +) { + const normal = b.object([]); + const important = b.object([]); - for (const directive of style_directives) { + let has_call_or_state = false; + + for (const d of style_directives) { const expression = - directive.value === true - ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) - : build_attribute_value(directive.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(expressions, value) : value - ).value; - const property = b.init(directive.name, expression); - - if (directive.modifiers.includes('important')) { - important_properties.push(property); - } else { - normal_properties.push(property); - } + d.value === true + ? build_getter(b.id(d.name), context.state) + : build_attribute_value(d.value, context).value; + + const object = d.modifiers.includes('important') ? important : normal; + object.properties.push(b.init(d.name, expression)); + + has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state; } - return important_properties.length - ? b.array([b.object(normal_properties), b.object(important_properties)]) - : b.object(normal_properties); + const directives = important.properties.length ? b.array([normal, important]) : normal; + + return has_call_or_state ? memoizer.add(directives) : directives; } /** @@ -624,7 +629,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont element === 'select' && attribute.value !== true && !is_text_attribute(attribute); const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(state.expressions, value) : value + metadata.has_call ? state.memoizer.add(value) : value ); const evaluated = context.state.scope.evaluate(value); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index 7381553dbe..6af5f5770e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -10,7 +10,7 @@ import { build_attribute_effect, build_set_class } from './shared/element.js'; -import { build_render_statement } from './shared/utils.js'; +import { build_render_statement, Memoizer } from './shared/utils.js'; /** * @param {AST.SvelteElement} node @@ -46,8 +46,8 @@ export function SvelteElement(node, context) { node: element_id, init: [], update: [], - expressions: [], - after_update: [] + after_update: [], + memoizer: new Memoizer() } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index 10f942b7d4..d119b0457b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -7,7 +7,7 @@ import { is_ignored } from '../../../../../state.js'; import { is_event_attribute } from '../../../../../utils/ast.js'; import * as b from '#compiler/builders'; import { build_class_directives_object, build_style_directives_object } from '../RegularElement.js'; -import { build_expression, build_template_chunk, get_expression_id } from './utils.js'; +import { build_expression, build_template_chunk, Memoizer } from './utils.js'; /** * @param {Array} attributes @@ -28,18 +28,12 @@ export function build_attribute_effect( /** @type {ObjectExpression['properties']} */ const values = []; - /** @type {Expression[]} */ - const expressions = []; - - /** @param {Expression} value */ - function memoize(value) { - return b.id(`$${expressions.push(value) - 1}`); - } + const memoizer = new Memoizer(); for (const attribute of attributes) { if (attribute.type === 'Attribute') { const { value } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call ? memoize(value) : value + metadata.has_call ? memoizer.add(value) : value ); if ( @@ -57,7 +51,7 @@ export function build_attribute_effect( let value = /** @type {Expression} */ (context.visit(attribute)); if (attribute.metadata.expression.has_call) { - value = memoize(value); + value = memoizer.add(value); } values.push(b.spread(value)); @@ -69,7 +63,7 @@ export function build_attribute_effect( b.prop( 'init', b.array([b.id('$.CLASS')]), - build_class_directives_object(class_directives, expressions, context) + build_class_directives_object(class_directives, context, memoizer) ) ); } @@ -79,21 +73,20 @@ export function build_attribute_effect( b.prop( 'init', b.array([b.id('$.STYLE')]), - build_style_directives_object(style_directives, expressions, context) + build_style_directives_object(style_directives, context, memoizer) ) ); } + const ids = memoizer.apply(); + context.state.init.push( b.stmt( b.call( '$.attribute_effect', element_id, - b.arrow( - expressions.map((_, i) => b.id(`$${i}`)), - b.object(values) - ), - expressions.length > 0 && b.array(expressions.map((expression) => b.thunk(expression))), + b.arrow(ids, b.object(values)), + memoizer.sync_values(), element.metadata.scoped && context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), @@ -158,7 +151,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c value = b.call('$.clsx', value); } - return metadata.has_call ? get_expression_id(context.state.expressions, value) : value; + return metadata.has_call ? context.state.memoizer.add(value) : value; }); /** @type {Identifier | undefined} */ @@ -171,7 +164,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c let next; if (class_directives.length) { - next = build_class_directives_object(class_directives, context.state.expressions, context); + next = build_class_directives_object(class_directives, context); has_state ||= class_directives.some((d) => d.metadata.expression.has_state); if (has_state) { @@ -226,7 +219,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c */ export function build_set_style(node_id, attribute, style_directives, context) { let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(context.state.expressions, value) : value + metadata.has_call ? context.state.memoizer.add(value) : value ); /** @type {Identifier | undefined} */ @@ -235,11 +228,11 @@ export function build_set_style(node_id, attribute, style_directives, context) { /** @type {ObjectExpression | Identifier | undefined} */ let prev; - /** @type {ArrayExpression | ObjectExpression | undefined} */ + /** @type {Expression | undefined} */ let next; if (style_directives.length) { - next = build_style_directives_object(style_directives, context.state.expressions, context); + next = build_style_directives_object(style_directives, context); has_state ||= style_directives.some((d) => d.metadata.expression.has_state); if (has_state) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 0bd9bfb128..b80466ccc9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -21,12 +21,41 @@ export function memoize_expression(state, value) { } /** - * Pushes `value` into `expressions` and returns a new id - * @param {Expression[]} expressions - * @param {Expression} value + * A utility for extracting complex expressions (such as call expressions) + * from templates and replacing them with `$0`, `$1` etc */ -export function get_expression_id(expressions, value) { - return b.id(`$${expressions.push(value) - 1}`); +export class Memoizer { + /** @type {Array<{ id: Identifier, expression: Expression }>} */ + #sync = []; + + /** + * @param {Expression} expression + */ + add(expression) { + const id = b.id('#'); // filled in later + + this.#sync.push({ id, expression }); + + return id; + } + + apply() { + return this.#sync.map((memo, i) => { + memo.id.name = `$${i}`; + return memo.id; + }); + } + + deriveds(runes = true) { + return this.#sync.map((memo) => + b.let(memo.id, b.call(runes ? '$.derived' : '$.derived_safe_equal', b.thunk(memo.expression))) + ); + } + + sync_values() { + if (this.#sync.length === 0) return; + return b.array(this.#sync.map((memo) => b.thunk(memo.expression))); + } } /** @@ -40,8 +69,7 @@ export function build_template_chunk( values, context, state = context.state, - memoize = (value, metadata) => - metadata.has_call ? get_expression_id(state.expressions, value) : value + memoize = (value, metadata) => (metadata.has_call ? state.memoizer.add(value) : value) ) { /** @type {Expression[]} */ const expressions = []; @@ -128,18 +156,20 @@ export function build_template_chunk( * @param {ComponentClientTransformState} state */ export function build_render_statement(state) { + const ids = state.memoizer.apply(); + const values = state.memoizer.sync_values(); + return b.stmt( b.call( '$.template_effect', b.arrow( - state.expressions.map((_, i) => b.id(`$${i}`)), + ids, state.update.length === 1 && state.update[0].type === 'ExpressionStatement' ? state.update[0].expression : b.block(state.update) ), - state.expressions.length > 0 && - b.array(state.expressions.map((expression) => b.thunk(expression))), - state.expressions.length > 0 && !state.analysis.runes && b.id('$.derived_safe_equal') + values, + values && !state.analysis.runes && b.id('$.derived_safe_equal') ) ); }