fix: consider static attributes that are inlined in the template (#14249)

* fix: consider static attributes that are inlined in the template

* fix: use `is_inlinable_expression`

* fix: move check for inlinable expression as last

* fix: simplify and correct

* chore: accept single node in `is_inlinable_expression`

* chore: update comment

* chore: add snapshots for non static nodes

Author: paoloricciuti

.changeset/mighty-ads-appear.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: consider static attributes that are inlined in the template

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

@@ -1,5 +1,5 @@
 /** @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' */
@@ -326,3 +326,28 @@ export function can_inline_variable(binding) {
 		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

@@ -19,7 +19,12 @@ import {
 import * as b from '../../../../utils/builders.js';
 import { is_custom_element_node } from '../../../nodes.js';
 import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
-import { build_getter, can_inline_variable, 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,
@@ -584,10 +589,7 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 	const inlinable_expression =
 		attribute.value === true
 			? false // not an expression
-			: is_inlinable_expression(
-					Array.isArray(attribute.value) ? attribute.value : [attribute.value],
-					context.state
-				);
+			: is_inlinable_expression(attribute.value, context.state);
 	if (attribute.metadata.expression.has_state) {
 		if (has_call) {
 			state.init.push(build_update(update));
@@ -605,30 +607,6 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 	}
 }
 
-/**
- * @param {(AST.Text | AST.ExpressionTag)[]} nodes
- * @param {import('../types.js').ComponentClientTransformState} state
- */
-function is_inlinable_expression(nodes, state) {
-	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;
-}
-
 /**
  * Like `build_element_attribute_update_assignment` but without any special attribute treatment.
  * @param {Identifier}	node_id

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

@@ -3,6 +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 { build_template_literal, build_update } from './utils.js';
 
 /**
@@ -97,7 +98,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) {
 
 			let child_state = state;
 
-			if (is_static_element(node)) {
+			if (is_static_element(node, state)) {
 				skipped += 1;
 			} else if (node.type === 'EachBlock' && nodes.length === 1 && is_element) {
 				node.metadata.is_controlled = true;
@@ -124,10 +125,12 @@ export function process_children(nodes, initial, is_element, { visit, state }) {
 
 /**
  * @param {SvelteNode} node
+ * @param {ComponentContext["state"]} state
  */
-function is_static_element(node) {
+function is_static_element(node, state) {
 	if (node.type !== 'RegularElement') return false;
 	if (node.fragment.metadata.dynamic) return false;
+	if (node.name.includes('-')) return false; // we're setting all attributes on custom elements through properties
 
 	for (const attribute of node.attributes) {
 		if (attribute.type !== 'Attribute') {
@@ -138,10 +141,6 @@ function is_static_element(node) {
 			return false;
 		}
 
-		if (attribute.value !== true && !is_text_attribute(attribute)) {
-			return false;
-		}
-
 		if (attribute.name === 'autofocus' || attribute.name === 'muted') {
 			return false;
 		}
@@ -155,8 +154,14 @@ function is_static_element(node) {
 			return false;
 		}
 
-		if (node.name.includes('-')) {
-			return false; // we're setting all attributes on custom elements through properties
+		if (
+			attribute.value !== true &&
+			!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)
+		) {
+			return false;
 		}
 	}
 

packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/client/index.svelte.js

@@ -9,11 +9,8 @@ var root = $.template(`<picture><source srcset="${__DECLARED_ASSET_0__}" type="i
 
 export default function Inline_module_vars($$anchor) {
 	var picture = root();
-	var source = $.child(picture);
-	var source_1 = $.sibling(source, 2);
-	var source_2 = $.sibling(source_1, 2);
-	var img = $.sibling(source_2, 2);
 
+	$.next(6);
 	$.reset(picture);
 	$.append($$anchor, picture);
 }

packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js

@@ -1,7 +1,7 @@
 import "svelte/internal/disclose-version";
 import * as $ from "svelte/internal/client";
 
-var root = $.template(`<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1> </h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> <!> <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements></custom-elements></cant-skip>`, 3);
+var root = $.template(`<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1> </h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> <!> <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements></custom-elements></cant-skip> <div><input></div> <div><source></div> <select><option>a</option></select> <img src="..." alt="" loading="lazy">`, 3);
 
 export default function Skip_static_subtree($$anchor, $$props) {
 	var fragment = root();
@@ -22,6 +22,28 @@ export default function Skip_static_subtree($$anchor, $$props) {
 
 	$.set_custom_element_data(custom_elements, "with", "attributes");
 	$.reset(cant_skip);
+
+	var div = $.sibling(cant_skip, 2);
+	var input = $.child(div);
+
+	$.autofocus(input, true);
+	$.reset(div);
+
+	var div_1 = $.sibling(div, 2);
+	var source = $.child(div_1);
+
+	source.muted = true;
+	$.reset(div_1);
+
+	var select = $.sibling(div_1, 2);
+	var option = $.child(select);
+
+	option.value = null == (option.__value = "a") ? "" : "a";
+	$.reset(select);
+
+	var img = $.sibling(select, 2);
+
 	$.template_effect(() => $.set_text(text, $$props.title));
+	$.handle_lazy_img(img);
 	$.append($$anchor, fragment);
 }

packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/server/index.svelte.js

@@ -3,5 +3,5 @@ import * as $ from "svelte/internal/server";
 export default function Skip_static_subtree($$payload, $$props) {
 	let { title, content } = $$props;
 
-	$$payload.out += `<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1>${$.escape(title)}</h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> ${$.html(content)} <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements with="attributes"></custom-elements></cant-skip>`;
+	$$payload.out += `<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1>${$.escape(title)}</h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> ${$.html(content)} <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements with="attributes"></custom-elements></cant-skip> <div><input autofocus></div> <div><source muted></div> <select><option value="a">a</option></select> <img src="..." alt="" loading="lazy">`;
 }

packages/svelte/tests/snapshot/samples/skip-static-subtree/index.svelte

@@ -30,3 +30,18 @@
 <cant-skip>
 	<custom-elements with="attributes"></custom-elements>
 </cant-skip>
+
+<div>
+	<!-- svelte-ignore a11y_autofocus -->
+	<input autofocus />
+</div>
+
+<div>
+	<source muted />
+</div>
+
+<select>
+	<option value="a">a</option>
+</select>
+
+<img src="..." alt="" loading="lazy" />