From e66416bec779ba4fba58c1bc670e119052489baf Mon Sep 17 00:00:00 2001
From: Rich Harris <rich.harris@vercel.com>
Date: Sat, 3 Aug 2024 22:18:33 -0400
Subject: [PATCH] feat: more efficient code generation when referencing globals
 (#12712)

* feat: more efficient code generation when referencing globals

* update test
---
 .changeset/red-kings-draw.md                  |  5 +++
 .../2-analyze/visitors/CallExpression.js      |  4 +--
 .../2-analyze/visitors/MemberExpression.js    |  4 +--
 .../visitors/TaggedTemplateExpression.js      |  4 +--
 .../phases/2-analyze/visitors/shared/utils.js | 27 ++++++++--------
 .../samples/effect-tracking/main.svelte       |  6 ++--
 .../purity/_expected/client/index.svelte.js   | 32 +++++++++++++++++++
 .../purity/_expected/server/index.svelte.js   | 12 +++++++
 .../snapshot/samples/purity/index.svelte      | 12 +++++++
 9 files changed, 83 insertions(+), 23 deletions(-)
 create mode 100644 .changeset/red-kings-draw.md
 create mode 100644 packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js
 create mode 100644 packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js
 create mode 100644 packages/svelte/tests/snapshot/samples/purity/index.svelte

diff --git a/.changeset/red-kings-draw.md b/.changeset/red-kings-draw.md
new file mode 100644
index 0000000000..56ce4afff3
--- /dev/null
+++ b/.changeset/red-kings-draw.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: more efficient code generation when referencing globals
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 98b9a3d7ec..57cdae0c66 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js
@@ -4,7 +4,7 @@
 import { get_rune } from '../../scope.js';
 import * as e from '../../../errors.js';
 import { get_parent, unwrap_optional } from '../../../utils/ast.js';
-import { is_known_safe_call, is_safe_identifier } from './shared/utils.js';
+import { is_pure, is_safe_identifier } from './shared/utils.js';
 
 /**
  * @param {CallExpression} node
@@ -150,7 +150,7 @@ export function CallExpression(node, context) {
 			break;
 	}
 
-	if (context.state.expression && !is_known_safe_call(node.callee, context)) {
+	if (context.state.expression && !is_pure(node.callee, context)) {
 		context.state.expression.has_call = true;
 		context.state.expression.has_state = true;
 	}
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 633275b63a..245a164c71 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js
@@ -1,7 +1,7 @@
 /** @import { MemberExpression } from 'estree' */
 /** @import { Context } from '../types' */
 import * as e from '../../../errors.js';
-import { is_safe_identifier } from './shared/utils.js';
+import { is_pure, is_safe_identifier } from './shared/utils.js';
 
 /**
  * @param {MemberExpression} node
@@ -15,7 +15,7 @@ export function MemberExpression(node, context) {
 		}
 	}
 
-	if (context.state.expression) {
+	if (context.state.expression && !is_pure(node, context)) {
 		context.state.expression.has_state = true;
 	}
 
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 b079b67512..eacb8a342a 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js
@@ -1,13 +1,13 @@
 /** @import { TaggedTemplateExpression, VariableDeclarator } from 'estree' */
 /** @import { Context } from '../types' */
-import { is_known_safe_call } from './shared/utils.js';
+import { is_pure } from './shared/utils.js';
 
 /**
  * @param {TaggedTemplateExpression} node
  * @param {Context} context
  */
 export function TaggedTemplateExpression(node, context) {
-	if (context.state.expression && !is_known_safe_call(node.tag, context)) {
+	if (context.state.expression && !is_pure(node.tag, context)) {
 		context.state.expression.has_call = true;
 		context.state.expression.has_state = true;
 	}
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js
index 2980ac3cbe..963882d396 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js
@@ -4,7 +4,7 @@
 /** @import { Scope } from '../../../scope' */
 /** @import { NodeLike } from '../../../../errors.js' */
 import * as e from '../../../../errors.js';
-import { extract_identifiers } from '../../../../utils/ast.js';
+import { extract_identifiers, object } from '../../../../utils/ast.js';
 import * as w from '../../../../warnings.js';
 
 /**
@@ -167,24 +167,23 @@ export function is_safe_identifier(expression, scope) {
 }
 
 /**
- * @param {Expression | Super} callee
+ * @param {Expression | Super} node
  * @param {Context} context
  * @returns {boolean}
  */
-export function is_known_safe_call(callee, context) {
-	// String / Number / BigInt / Boolean casting calls
-	if (callee.type === 'Identifier') {
-		const name = callee.name;
-		const binding = context.state.scope.get(name);
-		if (
-			binding === null &&
-			(name === 'BigInt' || name === 'String' || name === 'Number' || name === 'Boolean')
-		) {
-			return true;
-		}
+export function is_pure(node, context) {
+	if (node.type !== 'Identifier' && node.type !== 'MemberExpression') {
+		return false;
 	}
 
-	// TODO add more cases
+	const left = object(node);
+	if (!left) return false;
+
+	if (left.type === 'Identifier') {
+		const binding = context.state.scope.get(left.name);
+		if (binding === null) return true; // globals are assumed to be safe
+	}
 
+	// TODO add more cases (safe Svelte imports, etc)
 	return false;
 }
diff --git a/packages/svelte/tests/runtime-runes/samples/effect-tracking/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-tracking/main.svelte
index bb48958880..1e2ea446a5 100644
--- a/packages/svelte/tests/runtime-runes/samples/effect-tracking/main.svelte
+++ b/packages/svelte/tests/runtime-runes/samples/effect-tracking/main.svelte
@@ -1,5 +1,5 @@
 <script>
-	import {untrack} from 'svelte';
+	import { untrack } from 'svelte';
 
 	const foo = $effect.tracking();
 	let bar = $state(false);
@@ -10,5 +10,5 @@
 
 <p>{foo}</p>
 <p>{bar}</p>
-<p>{$effect.tracking()}</p>
-<p>{untrack(() => $effect.tracking())}</p>
+<p>{(bar, $effect.tracking())}</p>
+<p>{untrack(() => (bar, $effect.tracking()))}</p>
diff --git a/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js
new file mode 100644
index 0000000000..2863caba49
--- /dev/null
+++ b/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js
@@ -0,0 +1,32 @@
+import "svelte/internal/disclose-version";
+import * as $ from "svelte/internal/client";
+
+var root = $.template(`<p> </p> <p> </p> <!>`, 1);
+
+export default function Purity($$anchor) {
+	let min = 0;
+	let max = 100;
+	let number = 50;
+	let value = 'hello';
+	var fragment = root();
+	var p = $.first_child(fragment);
+	var text = $.child(p);
+
+	text.nodeValue = Math.max(min, Math.min(max, number));
+	$.reset(p);
+
+	var p_1 = $.sibling($.sibling(p, true));
+	var text_1 = $.child(p_1);
+
+	text_1.nodeValue = location.href;
+	$.reset(p_1);
+
+	var node = $.sibling($.sibling(p_1, true));
+
+	Child(node, {
+		prop: encodeURIComponent(value),
+		$$legacy: true
+	});
+
+	$.append($$anchor, fragment);
+}
\ No newline at end of file
diff --git a/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js
new file mode 100644
index 0000000000..3528e8d1b4
--- /dev/null
+++ b/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js
@@ -0,0 +1,12 @@
+import * as $ from "svelte/internal/server";
+
+export default function Purity($$payload) {
+	let min = 0;
+	let max = 100;
+	let number = 50;
+	let value = 'hello';
+
+	$$payload.out += `<p>${$.escape(Math.max(min, Math.min(max, number)))}</p> <p>${$.escape(location.href)}</p> `;
+	Child($$payload, { prop: encodeURIComponent(value) });
+	$$payload.out += `<!---->`;
+}
\ No newline at end of file
diff --git a/packages/svelte/tests/snapshot/samples/purity/index.svelte b/packages/svelte/tests/snapshot/samples/purity/index.svelte
new file mode 100644
index 0000000000..79db5dc7b1
--- /dev/null
+++ b/packages/svelte/tests/snapshot/samples/purity/index.svelte
@@ -0,0 +1,12 @@
+<script>
+	let min = 0;
+	let max = 100;
+	let number = 50;
+
+	let value = 'hello';
+</script>
+
+<p>{Math.max(min, Math.min(max, number))}</p>
+<p>{location.href}</p>
+
+<Child prop={encodeURIComponent(value)} />