chore: remove template expression inlining

Author: trueadm

.changeset/calm-mice-perform.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: remove template expression inlining

packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js

@@ -1,7 +1,7 @@
 /** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression } from 'estree' */
 /** @import { AST, DelegatedEvent, SvelteNode } from '#compiler' */
 /** @import { Context } from '../types' */
-import { is_boolean_attribute, is_capture_event, is_delegated } from '../../../../utils.js';
+import { is_capture_event, is_delegated } from '../../../../utils.js';
 import {
 	get_attribute_chunks,
 	get_attribute_expression,
@@ -16,28 +16,14 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
 export function Attribute(node, context) {
 	context.next();
 
-	const parent = /** @type {SvelteNode} */ (context.path.at(-1));
-
-	if (parent.type === 'RegularElement') {
-		// special case <option value="" />
-		if (node.name === 'value' && parent.name === 'option') {
-			mark_subtree_dynamic(context.path);
-		}
-
-		// special case <img loading="lazy" />
-		if (node.name === 'loading' && parent.name === 'img') {
+	// special case
+	if (node.name === 'value') {
+		const parent = /** @type {SvelteNode} */ (context.path.at(-1));
+		if (parent.type === 'RegularElement' && parent.name === 'option') {
 			mark_subtree_dynamic(context.path);
 		}
 	}
 
-	if (node.name.startsWith('on')) {
-		mark_subtree_dynamic(context.path);
-	}
-
-	if (parent.type === 'RegularElement' && is_boolean_attribute(node.name.toLowerCase())) {
-		node.metadata.expression.can_inline = false;
-	}
-
 	if (node.value !== true) {
 		for (const chunk of get_attribute_chunks(node.value)) {
 			if (chunk.type !== 'ExpressionTag') continue;
@@ -51,7 +37,6 @@ export function Attribute(node, context) {
 
 			node.metadata.expression.has_state ||= chunk.metadata.expression.has_state;
 			node.metadata.expression.has_call ||= chunk.metadata.expression.has_call;
-			node.metadata.expression.can_inline &&= chunk.metadata.expression.can_inline;
 		}
 
 		if (is_event_attribute(node)) {

packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js

@@ -179,8 +179,6 @@ export function CallExpression(node, context) {
 		if (!is_pure(node.callee, context) || context.state.expression.dependencies.size > 0) {
 			context.state.expression.has_call = true;
 			context.state.expression.has_state = true;
-			context.state.expression.can_inline = false;
-			mark_subtree_dynamic(context.path);
 		}
 	}
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js

@@ -2,6 +2,7 @@
 /** @import { Context } from '../types' */
 import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js';
 import * as e from '../../../errors.js';
+import { mark_subtree_dynamic } from './shared/fragment.js';
 
 /**
  * @param {AST.ExpressionTag} node
@@ -14,5 +15,9 @@ export function ExpressionTag(node, context) {
 		}
 	}
 
+	// TODO ideally we wouldn't do this here, we'd just do it on encountering
+	// an `Identifier` within the tag. But we currently need to handle `{42}` etc
+	mark_subtree_dynamic(context.path);
+
 	context.next({ ...context.state, expression: node.metadata.expression });
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

@@ -1,4 +1,5 @@
 /** @import { Expression, Identifier } from 'estree' */
+/** @import { EachBlock } from '#compiler' */
 /** @import { Context } from '../types' */
 import is_reference from 'is-reference';
 import { should_proxy } from '../../3-transform/client/utils.js';
@@ -19,6 +20,8 @@ export function Identifier(node, context) {
 		return;
 	}
 
+	mark_subtree_dynamic(context.path);
+
 	// If we are using arguments outside of a function, then throw an error
 	if (
 		node.name === 'arguments' &&
@@ -84,12 +87,6 @@ export function Identifier(node, context) {
 		}
 	}
 
-	// no binding means global, and we can't inline e.g. `<span>{location}</span>`
-	// because it could change between component renders. if there _is_ a
-	// binding and it is outside module scope, the expression cannot
-	// be inlined (TODO allow inlining in more cases - e.g. primitive consts)
-	let can_inline = !!binding && !binding.scope.parent && binding.kind === 'normal';
-
 	if (binding) {
 		if (context.state.expression) {
 			context.state.expression.dependencies.add(binding);
@@ -125,12 +122,4 @@ export function Identifier(node, context) {
 			w.reactive_declaration_module_script_dependency(node);
 		}
 	}
-
-	if (!can_inline) {
-		if (context.state.expression) {
-			context.state.expression.can_inline = false;
-		}
-
-		mark_subtree_dynamic(context.path);
-	}
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js

@@ -20,9 +20,6 @@ export function MemberExpression(node, context) {
 
 	if (context.state.expression && !is_pure(node, context)) {
 		context.state.expression.has_state = true;
-		context.state.expression.can_inline = false;
-
-		mark_subtree_dynamic(context.path);
 	}
 
 	if (!is_safe_identifier(node, context.state.scope)) {

packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js

@@ -10,7 +10,6 @@ export function TaggedTemplateExpression(node, context) {
 	if (context.state.expression && !is_pure(node.tag, context)) {
 		context.state.expression.has_call = true;
 		context.state.expression.has_state = true;
-		context.state.expression.can_inline = false;
 	}
 
 	if (node.tag.type === 'Identifier') {

packages/svelte/src/compiler/phases/3-transform/client/utils.js

@@ -1,18 +1,18 @@
 /** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression, Identifier, Pattern, PrivateIdentifier, Statement } from 'estree' */
-/** @import { Binding, SvelteNode } from '#compiler' */
+/** @import { AST, Binding, SvelteNode } from '#compiler' */
 /** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */
 /** @import { Analysis } from '../../types.js' */
 /** @import { Scope } from '../../scope.js' */
+import * as b from '../../../utils/builders.js';
+import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js';
 import {
-	PROPS_IS_BINDABLE,
-	PROPS_IS_IMMUTABLE,
 	PROPS_IS_LAZY_INITIAL,
+	PROPS_IS_IMMUTABLE,
 	PROPS_IS_RUNES,
-	PROPS_IS_UPDATED
+	PROPS_IS_UPDATED,
+	PROPS_IS_BINDABLE
 } from '../../../../constants.js';
 import { dev } from '../../../state.js';
-import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js';
-import * as b from '../../../utils/builders.js';
 import { get_value } from './visitors/shared/declarations.js';
 
 /**
@@ -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);
 }
+
+/**
+ * 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 &&
+		// in a `<script module>` block
+		!binding.scope.parent &&
+		// to prevent the need for escaping
+		binding.initial?.type === 'Literal'
+	);
+}
+
+/**
+ * @param {(AST.Text | AST.ExpressionTag) | (AST.Text | AST.ExpressionTag)[]} node_or_nodes
+ * @param {import('./types.js').ComponentClientTransformState} state
+ */
+export function is_inlinable_expression(node_or_nodes, state) {
+	let nodes = Array.isArray(node_or_nodes) ? node_or_nodes : [node_or_nodes];
+	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);
+				if (!can_inline_variable(binding)) {
+					return false;
+				}
+			} else {
+				return false;
+			}
+			has_expression_tag = true;
+		}
+	}
+	return has_expression_tag;
+}

packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js

@@ -1,4 +1,4 @@
-/** @import { Expression, Identifier, Statement, TemplateElement } from 'estree' */
+/** @import { Expression, Identifier, Statement } from 'estree' */
 /** @import { AST, Namespace } from '#compiler' */
 /** @import { SourceLocation } from '#shared' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../types' */
@@ -141,14 +141,14 @@ export function Fragment(node, context) {
 		const id = b.id(context.state.scope.generate('fragment'));
 
 		const use_space_template =
-			trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') &&
-			trimmed.some((node) => node.type === 'ExpressionTag' && !node.metadata.expression.can_inline);
+			trimmed.some((node) => node.type === 'ExpressionTag') &&
+			trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag');
 
 		if (use_space_template) {
 			// special case — we can use `$.text` instead of creating a unique template
 			const id = b.id(context.state.scope.generate('text'));
 
-			process_children(trimmed, () => id, null, {
+			process_children(trimmed, () => id, false, {
 				...context,
 				state
 			});
@@ -158,12 +158,12 @@ export function Fragment(node, context) {
 		} else {
 			if (is_standalone) {
 				// no need to create a template, we can just use the existing block's anchor
-				process_children(trimmed, () => b.id('$$anchor'), null, { ...context, state });
+				process_children(trimmed, () => b.id('$$anchor'), false, { ...context, state });
 			} else {
 				/** @type {(is_text: boolean) => Expression} */
 				const expression = (is_text) => b.call('$.first_child', id, is_text && b.true);
 
-				process_children(trimmed, expression, null, { ...context, state });
+				process_children(trimmed, expression, false, { ...context, state });
 
 				let flags = TEMPLATE_FRAGMENT;
 
@@ -212,34 +212,12 @@ function join_template(items) {
 	let quasi = b.quasi('');
 	const template = b.template([quasi], []);
 
-	/**
-	 * @param {Expression} expression
-	 */
-	function push(expression) {
-		if (expression.type === 'TemplateLiteral') {
-			for (let i = 0; i < expression.expressions.length; i += 1) {
-				const q = expression.quasis[i];
-				const e = expression.expressions[i];
-
-				quasi.value.cooked += /** @type {string} */ (q.value.cooked);
-				push(e);
-			}
-
-			const last = /** @type {TemplateElement} */ (expression.quasis.at(-1));
-			quasi.value.cooked += /** @type {string} */ (last.value.cooked);
-		} else if (expression.type === 'Literal') {
-			/** @type {string} */ (quasi.value.cooked) += expression.value;
-		} else {
-			template.expressions.push(expression);
-			template.quasis.push((quasi = b.quasi('')));
-		}
-	}
-
 	for (const item of items) {
 		if (typeof item === 'string') {
 			quasi.value.cooked += item;
 		} else {
-			push(item);
+			template.expressions.push(item);
+			template.quasis.push((quasi = b.quasi('')));
 		}
 	}