From dec07f00c16119271035f3b8908d099d70626612 Mon Sep 17 00:00:00 2001
From: paoloricciuti <ricciutipaolo@gmail.com>
Date: Tue, 12 Nov 2024 17:14:03 +0100
Subject: [PATCH] feat: inline dom expression too

---
 .../2-analyze/visitors/ExpressionTag.js       | 12 ----
 .../phases/2-analyze/visitors/Identifier.js   |  2 +-
 .../client/visitors/RegularElement.js         | 37 ++++++++----
 .../client/visitors/shared/fragment.js        | 16 +++++-
 .../client/visitors/shared/utils.js           | 12 +++-
 packages/svelte/src/compiler/phases/utils.js  | 56 +++++++++++++++----
 6 files changed, 96 insertions(+), 39 deletions(-)

diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js
index 86a2f6a9e1..018b04eeea 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js
@@ -16,17 +16,5 @@ export function ExpressionTag(node, context) {
 		}
 	}
 
-	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(attribute_parent, 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 });
 }
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js
index c0509a5414..f0f9b1e1fb 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js
@@ -28,7 +28,7 @@ export function Identifier(node, context) {
 	 * 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(attribute_parent, context.state.scope)) {
+	if (!attribute_parent || !is_inlinable_expression(attribute_parent.value, context.state.scope)) {
 		mark_subtree_dynamic(context.path);
 	}
 
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 649762994a..5cfc0a1496 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
@@ -31,6 +31,7 @@ import {
 	build_template_literal,
 	build_update,
 	build_update_assignment,
+	escape_template_quasis,
 	get_states_and_calls
 } from './shared/utils.js';
 
@@ -360,22 +361,30 @@ 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_expression(trimmed, context.state.scope)) {
+			// escaping every quasi if it's a template literal
+			if (value.type === 'TemplateLiteral') {
+				escape_template_quasis(value);
+			}
+			state.template.push(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.some((node) => node.type !== 'Text') &&
+			(!trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') ||
+				!is_inlinable_expression(trimmed, context.state.scope));
 
 		// The same applies if it's a `<template>` element, since we need to
 		// set the value of `hydrate_node` to `node.content`
@@ -578,7 +587,11 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 		);
 	}
 
-	const inlinable_expression = is_inlinable_expression(attribute, context.state.scope);
+	// we need to special case textarea value because it's not an actual attribute
+	const inlinable_expression =
+		is_inlinable_expression(attribute.value, context.state.scope) &&
+		attribute.name !== 'value' &&
+		element.name !== 'textarea';
 	if (attribute.metadata.expression.has_state) {
 		if (has_call) {
 			state.init.push(build_update(update));
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js
index 2934df81f8..d69c606247 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js
@@ -4,7 +4,7 @@
 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';
+import { build_template_literal, build_update, escape_template_quasis } from './utils.js';
 
 /**
  * Processes an array of template nodes, joining sibling text/expression nodes
@@ -83,7 +83,17 @@ export function process_children(nodes, initial, is_element, { visit, state }) {
 		} else if (has_state && !within_bound_contenteditable) {
 			state.update.push(update);
 		} else {
-			state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value)));
+			// if the expression is inlinable we just push it to the template
+			if (is_inlinable_expression(sequence, state.scope)) {
+				// escaping every quasi if it's a template literal
+				if (value.type === 'TemplateLiteral') {
+					escape_template_quasis(value);
+				}
+				state.template.push(value);
+			} else {
+				// else we programmatically set the value
+				state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value)));
+			}
 		}
 	}
 
@@ -159,7 +169,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, state.scope)
+			!is_inlinable_expression(attribute.value, state.scope)
 		) {
 			return false;
 		}
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 21b902d5c2..e4cc1d2407 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
@@ -1,4 +1,4 @@
-/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement, Super } from 'estree' */
+/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement, Super, TemplateLiteral } from 'estree' */
 /** @import { AST, SvelteNode } from '#compiler' */
 /** @import { ComponentClientTransformState } from '../../types' */
 import { walk } from 'zimmerframe';
@@ -9,6 +9,7 @@ import { regex_is_valid_identifier } from '../../../../patterns.js';
 import { create_derived } from '../../utils.js';
 import is_reference from 'is-reference';
 import { locator } from '../../../../../state.js';
+import { escape_html } from '../../../../../../escaping.js';
 
 /**
  * @param {Array<AST.Text | AST.ExpressionTag>} values
@@ -31,6 +32,15 @@ export function get_states_and_calls(values) {
 
 	return { states, calls };
 }
+/**
+ * Escape the html in every quesi in the template literal
+ * @param {TemplateLiteral} template
+ */
+export function escape_template_quasis(template) {
+	for (let quasi of template.quasis) {
+		quasi.value.raw = escape_html(quasi.value.raw);
+	}
+}
 
 /**
  * @param {Array<AST.Text | AST.ExpressionTag>} values
diff --git a/packages/svelte/src/compiler/phases/utils.js b/packages/svelte/src/compiler/phases/utils.js
index 116f03ca91..9b2b0b68ca 100644
--- a/packages/svelte/src/compiler/phases/utils.js
+++ b/packages/svelte/src/compiler/phases/utils.js
@@ -1,5 +1,31 @@
 /** @import { AST, Binding } from '#compiler' */
 /** @import { Scope } from './scope' */
+/** @import * as ESTree from 'estree' */
+
+import { walk } from 'zimmerframe';
+
+/**
+ * @param {ESTree.Expression} expr
+ */
+export function extract_identifiers(expr) {
+	/** @type {ESTree.Identifier[]} */
+	let nodes = [];
+
+	walk(
+		expr,
+		{},
+		{
+			Identifier(node, { path }) {
+				const parent = path.at(-1);
+				if (parent?.type !== 'MemberExpression' || parent.property !== node || parent.computed) {
+					nodes.push(node);
+				}
+			}
+		}
+	);
+
+	return nodes;
+}
 
 /**
  * Whether a variable can be referenced directly from template string.
@@ -17,21 +43,31 @@ function can_inline_variable(binding) {
 }
 
 /**
- * @param {AST.Attribute} attribute
+ * @param {AST.Attribute["value"]} value
  * @param {Scope} scope
  */
-export function is_inlinable_expression(attribute, scope) {
-	if (attribute.value === true) return false; // not an expression
-	let nodes = Array.isArray(attribute.value) ? attribute.value : [attribute.value];
+export function is_inlinable_expression(value, scope) {
+	if (value === true) return false; // not an expression
+	let nodes = Array.isArray(value) ? value : [value];
 	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 {
+			const identifiers = extract_identifiers(value.expression);
+			// if not every identifier is inlinable we bail
+			if (
+				identifiers.length > 0 &&
+				identifiers.some((id) => {
+					const binding = scope.owner(id.name)?.declarations.get(id.name);
+					return !can_inline_variable(binding);
+				})
+			) {
+				return false;
+			} else if (
+				// we need to special case null and boolean values because
+				// we want to set them programmatically
+				value.expression.type === 'Literal' &&
+				(value.expression.value === null || typeof value.expression.value === 'boolean')
+			) {
 				return false;
 			}
 			has_expression_tag = true;