fix: avoid marking subtree as dynamic for inlined attributes

Author: paoloricciuti

.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/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 { is_inlinable_expression } from '../../utils.js';
 import { mark_subtree_dynamic } from './shared/fragment.js';
 
 /**
@@ -15,9 +16,17 @@ 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);
+	const attribute_parent = context.path.find((parent) => parent.type === 'Attribute');
+	/**
+	 * if the expression tag is part 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 (attribute_parent && !is_inlinable_expression(node, context.state.scope)) {
+		// 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

@@ -7,6 +7,7 @@ import * as e from '../../../errors.js';
 import * as w from '../../../warnings.js';
 import { is_rune } from '../../../../utils.js';
 import { mark_subtree_dynamic } from './shared/fragment.js';
+import { is_inlinable_expression } from '../../utils.js';
 
 /**
  * @param {Identifier} node
@@ -20,7 +21,21 @@ export function Identifier(node, context) {
 		return;
 	}
 
-	mark_subtree_dynamic(context.path);
+	const expression_tag_parent = context.path.find((parent) => parent.type === 'ExpressionTag');
+	const attribute_parent = context.path.find((parent) => parent.type === 'Attribute');
+
+	/**
+	 * 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 (
+		!attribute_parent ||
+		!expression_tag_parent ||
+		!is_inlinable_expression(expression_tag_parent, context.state.scope)
+	) {
+		mark_subtree_dynamic(context.path);
+	}
 
 	// If we are using arguments outside of a function, then throw an error
 	if (

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/RegularElement.js

@@ -3,35 +3,28 @@
 /** @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_expression } 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,
@@ -40,7 +33,6 @@ import {
 	build_update_assignment,
 	get_states_and_calls
 } from './shared/utils.js';
-import { visit_event_attribute } from './shared/events.js';
 
 /**
  * @param {AST.RegularElement} node
@@ -589,7 +581,7 @@ 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);
+			: is_inlinable_expression(attribute.value, context.state.scope);
 	if (attribute.metadata.expression.has_state) {
 		if (has_call) {
 			state.init.push(build_update(update));

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

@@ -3,7 +3,7 @@
 /** @import { ComponentContext } from '../../types' */
 import { is_event_attribute, is_text_attribute } from '../../../../../utils/ast.js';
 import * as b from '../../../../../utils/builders.js';
-import { is_inlinable_expression } from '../../utils.js';
+import { is_inlinable_expression } from '../../../../utils.js';
 import { build_template_literal, build_update } from './utils.js';
 
 /**
@@ -159,7 +159,7 @@ function is_static_element(node, state) {
 			!is_text_attribute(attribute) &&
 			// If the attribute is not a text attribute but is inlinable we will directly inline it in the
 			// the template so before returning false we need to check that the attribute is not inlinable
-			!is_inlinable_expression(attribute.value, state)
+			!is_inlinable_expression(attribute.value, state.scope)
 		) {
 			return false;
 		}

packages/svelte/src/compiler/phases/utils.js

@@ -0,0 +1,39 @@
+/** @import { AST, Binding } from '#compiler' */
+
+/**
+ * Whether a variable can be referenced directly from template string.
+ * @param {Binding | undefined} binding
+ * @returns {boolean}
+ */
+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('./scope.js').Scope} scope
+ */
+export function is_inlinable_expression(node_or_nodes, scope) {
+	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 = 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/tests/snapshot/samples/inline-module-vars/_expected/client/index.svelte.js

@@ -10,7 +10,5 @@ var root = $.template(`<picture><source srcset="${__DECLARED_ASSET_0__}" type="i
 export default function Inline_module_vars($$anchor) {
 	var picture = root();
 
-	$.next(6);
-	$.reset(picture);
 	$.append($$anchor, picture);
 }