fix: avoid marking subtree as dynamic for inlined attributes

.changeset/spotty-sheep-fetch.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: avoid marking subtree as dynamic for inlined attributes

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

@@ -24,6 +24,10 @@ export function Attribute(node, context) {
 		}
 	}
 
+	if (node.name.startsWith('on')) {
+		mark_subtree_dynamic(context.path);
+	}
+
 	if (node.value !== true) {
 		for (const chunk of get_attribute_chunks(node.value)) {
 			if (chunk.type !== 'ExpressionTag') continue;

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

@@ -178,6 +178,7 @@ 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;
 		}
 	}
 }

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

@@ -2,7 +2,6 @@
 /** @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
@@ -15,9 +14,5 @@ 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,5 +1,4 @@
 /** @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';
@@ -20,8 +19,6 @@ 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' &&
@@ -87,6 +84,13 @@ 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);
@@ -122,4 +126,17 @@ export function Identifier(node, context) {
 			w.reactive_declaration_module_script_dependency(node);
 		}
 	}
+
+	if (!can_inline && context.state.expression) {
+		context.state.expression.can_inline = false;
+	}
+
+	/**
+	 * if the identifier is part of an expression tag of an attribute we want to check if it's inlinable
+	 * before marking the subtree as dynamic. This is because if it's inlinable it will be inlined in the template
+	 * directly making the whole thing actually static.
+	 */
+	if (!can_inline || !context.path.find((node) => node.type === 'Attribute')) {
+		mark_subtree_dynamic(context.path);
+	}
 }

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

@@ -19,6 +19,7 @@ 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;
 	}
 
 	if (!is_safe_identifier(node, context.state.scope)) {

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

@@ -10,6 +10,7 @@ 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 { AST, Binding, SvelteNode } from '#compiler' */
+/** @import { 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_LAZY_INITIAL,
+	PROPS_IS_BINDABLE,
 	PROPS_IS_IMMUTABLE,
+	PROPS_IS_LAZY_INITIAL,
 	PROPS_IS_RUNES,
-	PROPS_IS_UPDATED,
-	PROPS_IS_BINDABLE
+	PROPS_IS_UPDATED
 } 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,43 +311,3 @@ 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 } from 'estree' */
+/** @import { Expression, Identifier, Statement, TemplateElement } from 'estree' */
 /** @import { AST, Namespace } from '#compiler' */
 /** @import { SourceLocation } from '#shared' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../types' */
@@ -141,8 +141,8 @@ export function Fragment(node, context) {
 		const id = b.id(context.state.scope.generate('fragment'));
 
 		const use_space_template =
-			trimmed.some((node) => node.type === 'ExpressionTag') &&
-			trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag');
+			trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') &&
+			trimmed.some((node) => node.type === 'ExpressionTag' && !node.metadata.expression.can_inline);
 
 		if (use_space_template) {
 			// special case — we can use `$.text` instead of creating a unique template
@@ -212,12 +212,32 @@ 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 {
+			template.expressions.push(expression);
+			template.quasis.push((quasi = b.quasi('')));
+		}
+	}
+
 	for (const item of items) {
 		if (typeof item === 'string') {
 			quasi.value.cooked += item;
 		} else {
-			template.expressions.push(item);
-			template.quasis.push((quasi = b.quasi('')));
+			push(item);
 		}
 	}
 

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

@@ -3,44 +3,37 @@
 /** @import { SourceLocation } from '#shared' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../types' */
 /** @import { Scope } from '../../../scope' */
+import { escape_html } from '../../../../../escaping.js';
 import {
 	is_boolean_attribute,
 	is_dom_property,
 	is_load_error_element,
 	is_void
 } from '../../../../../utils.js';
-import { escape_html } from '../../../../../escaping.js';
 import { dev, is_ignored, locator } from '../../../../state.js';
-import {
-	get_attribute_expression,
-	is_event_attribute,
-	is_text_attribute
-} from '../../../../utils/ast.js';
+import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js';
 import * as b from '../../../../utils/builders.js';
 import { is_custom_element_node } from '../../../nodes.js';
+import { is_inlinable_attribute, is_inlinable_sequence } from '../../../utils.js';
 import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
+import { build_getter, create_derived } from '../utils.js';
 import {
-	build_getter,
-	can_inline_variable,
-	create_derived,
-	is_inlinable_expression
-} from '../utils.js';
-import {
-	get_attribute_name,
 	build_attribute_value,
 	build_class_directives,
+	build_set_attributes,
 	build_style_directives,
-	build_set_attributes
+	get_attribute_name
 } from './shared/element.js';
+import { visit_event_attribute } from './shared/events.js';
 import { process_children } from './shared/fragment.js';
 import {
 	build_render_statement,
 	build_template_literal,
 	build_update,
 	build_update_assignment,
+	escape_inline_expression,
 	get_states_and_calls
 } from './shared/utils.js';
-import { visit_event_attribute } from './shared/events.js';
 
 /**
  * @param {AST.RegularElement} node
@@ -368,22 +361,27 @@ export function RegularElement(node, context) {
 		get_states_and_calls(trimmed);
 
 	if (states_and_calls && states_and_calls.states === 0) {
-		child_state.init.push(
-			b.stmt(
-				b.assignment(
-					'=',
-					b.member(context.state.node, 'textContent'),
-					build_template_literal(trimmed, context.visit, child_state).value
-				)
-			)
-		);
+		let { value } = build_template_literal(trimmed, context.visit, child_state);
+		// if the expression is inlinable we just push it to the template
+		if (is_inlinable_sequence(trimmed)) {
+			state.template.push(escape_inline_expression(value));
+		} else {
+			// else we programmatically set the value
+			child_state.init.push(
+				b.stmt(b.assignment('=', b.member(context.state.node, 'textContent'), value))
+			);
+		}
 	} else {
 		/** @type {Expression} */
 		let arg = context.state.node;
 
 		// If `hydrate_node` is set inside the element, we need to reset it
-		// after the element has been hydrated
-		let needs_reset = trimmed.some((node) => node.type !== 'Text');
+		// after the element has been hydrated (we don't need to reset if it's been inlined)
+		let needs_reset = !trimmed.every(
+			(node) =>
+				node.type === 'Text' ||
+				(node.type === 'ExpressionTag' && node.metadata.expression.can_inline)
+		);
 
 		// The same applies if it's a `<template>` element, since we need to
 		// set the value of `hydrate_node` to `node.content`
@@ -586,25 +584,30 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 		);
 	}
 
-	const inlinable_expression =
-		attribute.value === true
-			? false // not an expression
-			: is_inlinable_expression(attribute.value, context.state);
 	if (attribute.metadata.expression.has_state) {
 		if (has_call) {
 			state.init.push(build_update(update));
 		} else {
 			state.update.push(update);
 		}
 		return true;
+	}
+
+	// we need to special case textarea value because it's not an actual attribute
+	const can_inline =
+		(attribute.name !== 'value' || element.name !== 'textarea') &&
+		is_inlinable_attribute(attribute);
+
+	if (can_inline) {
+		// TODO would be great if we could avoid `$.attr` when we know the value
+		context.state.template.push(
+			b.call('$.attr', b.literal(name), value, is_boolean_attribute(name) && b.true)
+		);
 	} else {
-		if (inlinable_expression) {
-			context.state.template.push(` ${name}="`, value, '"');
-		} else {
-			state.init.push(update);
-		}
-		return false;
+		state.init.push(update);
 	}
+
+	return false;
 }
 
 /**