From d52bfe734e796aa52539bfd7ad6cd0b11019e8ea Mon Sep 17 00:00:00 2001
From: Rich Harris <rich.harris@vercel.com>
Date: Wed, 13 Nov 2024 20:34:58 -0500
Subject: [PATCH] simplify/speedup by doing the work once, during analysis

---
 .../phases/2-analyze/visitors/Attribute.js    |  4 ++
 .../2-analyze/visitors/CallExpression.js      |  1 +
 .../phases/2-analyze/visitors/Identifier.js   | 29 ++++++---
 .../2-analyze/visitors/MemberExpression.js    |  1 +
 .../visitors/TaggedTemplateExpression.js      |  1 +
 .../3-transform/client/visitors/Fragment.js   |  2 +-
 .../client/visitors/RegularElement.js         |  6 +-
 .../client/visitors/shared/fragment.js        |  4 +-
 packages/svelte/src/compiler/phases/nodes.js  |  3 +-
 packages/svelte/src/compiler/phases/utils.js  | 62 +++----------------
 packages/svelte/src/compiler/types/index.d.ts |  2 +
 11 files changed, 44 insertions(+), 71 deletions(-)

diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js
index 2a281a1aa3..efdbe292d0 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js
+++ b/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;
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js
index 957b27ae9b..2ae32e80e1 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js
+++ b/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;
 		}
 	}
 }
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 f0f9b1e1fb..5dbdc23254 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js
@@ -23,15 +23,6 @@ export function Identifier(node, context) {
 
 	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 || !is_inlinable_expression(attribute_parent.value, context.state.scope)) {
-		mark_subtree_dynamic(context.path);
-	}
-
 	// If we are using arguments outside of a function, then throw an error
 	if (
 		node.name === 'arguments' &&
@@ -101,6 +92,13 @@ export function Identifier(node, context) {
 		if (context.state.expression) {
 			context.state.expression.dependencies.add(binding);
 			context.state.expression.has_state ||= binding.kind !== 'normal';
+
+			// if the binding is outside module scope, the expression
+			// cannot be inlined (TODO allow inlining in more cases,
+			// e.g. primitive consts)
+			if (!!binding.scope.parent) {
+				context.state.expression.can_inline = false;
+			}
 		}
 
 		if (
@@ -131,5 +129,18 @@ export function Identifier(node, context) {
 		) {
 			w.reactive_declaration_module_script_dependency(node);
 		}
+	} else if (context.state.expression) {
+		// no binding means global, and we can't inline e.g. `<span>{location}</span>`
+		// because it could change between component renders
+		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 (!attribute_parent || !is_inlinable_expression(attribute_parent.value)) {
+		mark_subtree_dynamic(context.path);
 	}
 }
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js
index 171a1106a8..adcc2da422 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js
+++ b/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)) {
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js
index eacb8a342a..724b9af311 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js
+++ b/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') {
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js
index 4d42b841f2..d0b6d0996e 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js
@@ -144,7 +144,7 @@ export function Fragment(node, context) {
 		const use_space_template =
 			trimmed.some((node) => node.type === 'ExpressionTag') &&
 			trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') &&
-			!is_inlinable_expression(trimmed, context.state.scope);
+			!is_inlinable_expression(trimmed);
 
 		if (use_space_template) {
 			// special case — we can use `$.text` instead of creating a unique template
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 e5d1d89f5f..097869104e 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
@@ -363,7 +363,7 @@ export function RegularElement(node, context) {
 	if (states_and_calls && states_and_calls.states === 0) {
 		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)) {
+		if (is_inlinable_expression(trimmed)) {
 			escape_template_quasis(value);
 			state.template.push(value);
 		} else {
@@ -381,7 +381,7 @@ export function RegularElement(node, context) {
 		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));
+				!is_inlinable_expression(trimmed));
 
 		// The same applies if it's a `<template>` element, since we need to
 		// set the value of `hydrate_node` to `node.content`
@@ -586,7 +586,7 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 
 	// 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) &&
+		is_inlinable_expression(attribute.value) &&
 		attribute.name !== 'value' &&
 		element.name !== 'textarea';
 	if (attribute.metadata.expression.has_state) {
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 f1101a1d98..9469387eb3 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
@@ -84,7 +84,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) {
 			state.update.push(update);
 		} else {
 			// if the expression is inlinable we just push it to the template
-			if (!is_text && is_inlinable_expression(sequence, state.scope)) {
+			if (!is_text && is_inlinable_expression(sequence)) {
 				escape_template_quasis(value);
 				state.template.push(value);
 			} else {
@@ -166,7 +166,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.scope)
+			!is_inlinable_expression(attribute.value)
 		) {
 			return false;
 		}
diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js
index ead525aaa1..13a54730eb 100644
--- a/packages/svelte/src/compiler/phases/nodes.js
+++ b/packages/svelte/src/compiler/phases/nodes.js
@@ -58,6 +58,7 @@ export function create_expression_metadata() {
 	return {
 		dependencies: new Set(),
 		has_state: false,
-		has_call: false
+		has_call: false,
+		can_inline: true
 	};
 }
diff --git a/packages/svelte/src/compiler/phases/utils.js b/packages/svelte/src/compiler/phases/utils.js
index a0a60f5437..d4e53635c9 100644
--- a/packages/svelte/src/compiler/phases/utils.js
+++ b/packages/svelte/src/compiler/phases/utils.js
@@ -1,68 +1,19 @@
-/** @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.
- * @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'
-	);
-}
+/** @import { AST } from '#compiler' */
 
 /**
  * @param {AST.Attribute["value"]} value
- * @param {Scope} scope
  */
-export function is_inlinable_expression(value, scope) {
+export function is_inlinable_expression(value) {
 	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') {
-			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);
-				})
-			) {
+			if (!value.metadata.expression.can_inline) {
 				return false;
-			} else if (
+			}
+
+			if (
 				// we need to special case null and boolean values because
 				// we want to set them programmatically
 				value.expression.type === 'Literal' &&
@@ -74,6 +25,7 @@ export function is_inlinable_expression(value, scope) {
 			) {
 				return false;
 			}
+
 			has_expression_tag = true;
 		}
 	}
diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts
index 8a45f2d4fd..bd4749db00 100644
--- a/packages/svelte/src/compiler/types/index.d.ts
+++ b/packages/svelte/src/compiler/types/index.d.ts
@@ -317,6 +317,8 @@ export interface ExpressionMetadata {
 	has_state: boolean;
 	/** True if the expression involves a call expression (often, it will need to be wrapped in a derived) */
 	has_call: boolean;
+	/** True if the expression can be inlined into a template */
+	can_inline: boolean;
 }
 
 export * from './template.js';