From 56315df67961fd4caa26116940415cc2f34fe3ee Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Wed, 28 Feb 2024 15:05:45 +0100
Subject: [PATCH] feat: allow arbitrary call expressions for render tags
 (#10656)

closes #9582
---
 .changeset/ten-jokes-divide.md                |  5 +++
 packages/svelte/src/compiler/errors.js        |  4 +-
 .../src/compiler/phases/1-parse/state/tag.js  | 10 +++--
 .../compiler/phases/2-analyze/validation.js   | 18 ++++++--
 .../3-transform/client/visitors/template.js   | 24 ++++++----
 .../3-transform/server/transform-server.js    | 24 ++++++++--
 .../svelte/src/compiler/types/template.d.ts   |  8 ++--
 packages/svelte/src/compiler/utils/ast.js     |  9 ++++
 packages/svelte/src/internal/client/render.js |  6 ++-
 .../svelte/src/internal/client/validate.js    |  2 +-
 .../render-tag-invalid-call/_config.js        |  8 ++++
 .../render-tag-invalid-call/main.svelte       |  1 +
 .../samples/snippets/output.json              | 44 +++++++++++++------
 .../samples/snippet-expressions/_config.js    | 42 ++++++++++++++++++
 .../samples/snippet-expressions/child.svelte  | 21 +++++++++
 .../samples/snippet-expressions/main.svelte   | 17 +++++++
 packages/svelte/types/index.d.ts              |  5 +--
 17 files changed, 206 insertions(+), 42 deletions(-)
 create mode 100644 .changeset/ten-jokes-divide.md
 create mode 100644 packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/_config.js
 create mode 100644 packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/main.svelte
 create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-expressions/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-expressions/child.svelte
 create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-expressions/main.svelte

diff --git a/.changeset/ten-jokes-divide.md b/.changeset/ten-jokes-divide.md
new file mode 100644
index 0000000000..179357fb29
--- /dev/null
+++ b/.changeset/ten-jokes-divide.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+feat: allow arbitrary call expressions and optional chaining for snippets
diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js
index f2207c9336..213b689d3e 100644
--- a/packages/svelte/src/compiler/errors.js
+++ b/packages/svelte/src/compiler/errors.js
@@ -89,8 +89,10 @@ const parse = {
 	'duplicate-style-element': () => `A component can have a single top-level <style> element`,
 	'duplicate-script-element': () =>
 		`A component can have a single top-level <script> element and/or a single top-level <script context="module"> element`,
-	'invalid-render-expression': () => 'expected an identifier followed by (...)',
+	'invalid-render-expression': () => '{@render ...} tags can only contain call expressions',
 	'invalid-render-arguments': () => 'expected at most one argument',
+	'invalid-render-call': () =>
+		'Calling a snippet function using apply, bind or call is not allowed',
 	'invalid-render-spread-argument': () => 'cannot use spread arguments in {@render ...} tags',
 	'invalid-snippet-rest-parameter': () =>
 		'snippets do not support rest parameters; use an array instead'
diff --git a/packages/svelte/src/compiler/phases/1-parse/state/tag.js b/packages/svelte/src/compiler/phases/1-parse/state/tag.js
index 27c3b7e9af..a457a82059 100644
--- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js
+++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js
@@ -577,7 +577,12 @@ function special(parser) {
 
 		const expression = read_expression(parser);
 
-		if (expression.type !== 'CallExpression' || expression.callee.type !== 'Identifier') {
+		if (
+			expression.type !== 'CallExpression' &&
+			(expression.type !== 'ChainExpression' ||
+				expression.expression.type !== 'CallExpression' ||
+				!expression.expression.optional)
+		) {
 			error(expression, 'invalid-render-expression');
 		}
 
@@ -589,8 +594,7 @@ function special(parser) {
 			type: 'RenderTag',
 			start,
 			end: parser.index,
-			expression: expression.callee,
-			arguments: expression.arguments
+			expression: expression
 		});
 	}
 }
diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js
index 55d523ffdc..ddb0d1d584 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/validation.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js
@@ -9,7 +9,8 @@ import {
 	get_parent,
 	is_expression_attribute,
 	is_text_attribute,
-	object
+	object,
+	unwrap_optional
 } from '../../utils/ast.js';
 import { warn } from '../../warnings.js';
 import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
@@ -487,11 +488,22 @@ const validation = {
 		});
 	},
 	RenderTag(node, context) {
-		for (const arg of node.arguments) {
+		const raw_args = unwrap_optional(node.expression).arguments;
+		for (const arg of raw_args) {
 			if (arg.type === 'SpreadElement') {
 				error(arg, 'invalid-render-spread-argument');
 			}
 		}
+
+		const callee = unwrap_optional(node.expression).callee;
+		if (
+			callee.type === 'MemberExpression' &&
+			callee.property.type === 'Identifier' &&
+			['bind', 'apply', 'call'].includes(callee.property.name)
+		) {
+			error(node, 'invalid-render-call');
+		}
+
 		const is_inside_textarea = context.path.find((n) => {
 			return (
 				n.type === 'SvelteElement' &&
@@ -505,7 +517,7 @@ const validation = {
 				node,
 				'invalid-tag-placement',
 				'inside <textarea> or <svelte:element this="textarea">',
-				node.expression.name
+				'render'
 			);
 		}
 	},
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
index 0bd8d9235d..f54113375e 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
@@ -3,7 +3,8 @@ import {
 	extract_paths,
 	is_event_attribute,
 	is_text_attribute,
-	object
+	object,
+	unwrap_optional
 } from '../../../../utils/ast.js';
 import { binding_properties } from '../../../bindings.js';
 import {
@@ -1864,18 +1865,18 @@ export const template_visitors = {
 	},
 	RenderTag(node, context) {
 		context.state.template.push('<!>');
-		const binding = context.state.scope.get(node.expression.name);
-		const is_reactive = binding?.kind !== 'normal' || node.expression.type !== 'Identifier';
+		const callee = unwrap_optional(node.expression).callee;
+		const raw_args = unwrap_optional(node.expression).arguments;
+		const is_reactive =
+			callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';
 
 		/** @type {import('estree').Expression[]} */
 		const args = [context.state.node];
-		for (const arg of node.arguments) {
+		for (const arg of raw_args) {
 			args.push(b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg))));
 		}
 
-		let snippet_function = /** @type {import('estree').Expression} */ (
-			context.visit(node.expression)
-		);
+		let snippet_function = /** @type {import('estree').Expression} */ (context.visit(callee));
 		if (context.state.options.dev) {
 			snippet_function = b.call('$.validate_snippet', snippet_function);
 		}
@@ -1885,7 +1886,14 @@ export const template_visitors = {
 				b.stmt(b.call('$.snippet_effect', b.thunk(snippet_function), ...args))
 			);
 		} else {
-			context.state.after_update.push(b.stmt(b.call(snippet_function, ...args)));
+			context.state.after_update.push(
+				b.stmt(
+					(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
+						snippet_function,
+						...args
+					)
+				)
+			);
 		}
 	},
 	AnimateDirective(node, { state, visit }) {
diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
index 168ed14f08..a173a25281 100644
--- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
+++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
@@ -1,6 +1,11 @@
 import { walk } from 'zimmerframe';
 import { set_scope, get_rune } from '../../scope.js';
-import { extract_identifiers, extract_paths, is_event_attribute } from '../../../utils/ast.js';
+import {
+	extract_identifiers,
+	extract_paths,
+	is_event_attribute,
+	unwrap_optional
+} from '../../../utils/ast.js';
 import * as b from '../../../utils/builders.js';
 import is_reference from 'is-reference';
 import {
@@ -1161,17 +1166,28 @@ const template_visitors = {
 		state.init.push(anchor);
 		state.template.push(t_expression(anchor_id));
 
-		const expression = /** @type {import('estree').Expression} */ (context.visit(node.expression));
+		const callee = unwrap_optional(node.expression).callee;
+		const raw_args = unwrap_optional(node.expression).arguments;
+
+		const expression = /** @type {import('estree').Expression} */ (context.visit(callee));
 		const snippet_function = state.options.dev
 			? b.call('$.validate_snippet', expression)
 			: expression;
 
-		const snippet_args = node.arguments.map((arg) => {
+		const snippet_args = raw_args.map((arg) => {
 			return /** @type {import('estree').Expression} */ (context.visit(arg));
 		});
 
 		state.template.push(
-			t_statement(b.stmt(b.call(snippet_function, b.id('$$payload'), ...snippet_args)))
+			t_statement(
+				b.stmt(
+					(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
+						snippet_function,
+						b.id('$$payload'),
+						...snippet_args
+					)
+				)
+			)
 		);
 
 		state.template.push(t_expression(anchor_id));
diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts
index ec8319e575..5d794cee09 100644
--- a/packages/svelte/src/compiler/types/template.d.ts
+++ b/packages/svelte/src/compiler/types/template.d.ts
@@ -13,7 +13,10 @@ import type {
 	ObjectExpression,
 	Pattern,
 	Program,
-	SpreadElement
+	SpreadElement,
+	CallExpression,
+	ChainExpression,
+	SimpleCallExpression
 } from 'estree';
 import type { Atrule, Rule } from './css';
 
@@ -151,8 +154,7 @@ export interface DebugTag extends BaseNode {
 /** A `{@render foo(...)} tag */
 export interface RenderTag extends BaseNode {
 	type: 'RenderTag';
-	expression: Identifier;
-	arguments: Array<Expression | SpreadElement>;
+	expression: SimpleCallExpression | (ChainExpression & { expression: SimpleCallExpression });
 }
 
 type Tag = ExpressionTag | HtmlTag | ConstTag | DebugTag | RenderTag;
diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js
index 464c2081f3..0ff09079bc 100644
--- a/packages/svelte/src/compiler/utils/ast.js
+++ b/packages/svelte/src/compiler/utils/ast.js
@@ -361,3 +361,12 @@ export function is_simple_expression(node) {
 
 	return false;
 }
+
+/**
+ * @template {import('estree').SimpleCallExpression | import('estree').MemberExpression} T
+ * @param {import('estree').ChainExpression & { expression : T } | T} node
+ * @returns {T}
+ */
+export function unwrap_optional(node) {
+	return node.type === 'ChainExpression' ? node.expression : node;
+}
diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js
index fe3db6c0eb..c956c96a4a 100644
--- a/packages/svelte/src/internal/client/render.js
+++ b/packages/svelte/src/internal/client/render.js
@@ -2684,7 +2684,7 @@ export function sanitize_slots(props) {
 }
 
 /**
- * @param {() => Function} get_snippet
+ * @param {() => Function | null | undefined} get_snippet
  * @param {Node} node
  * @param {(() => any)[]} args
  * @returns {void}
@@ -2695,7 +2695,9 @@ export function snippet_effect(get_snippet, node, ...args) {
 		// Only rerender when the snippet function itself changes,
 		// not when an eagerly-read prop inside the snippet function changes
 		const snippet = get_snippet();
-		untrack(() => snippet(node, ...args));
+		if (snippet) {
+			untrack(() => snippet(node, ...args));
+		}
 		return () => {
 			if (block.d !== null) {
 				remove(block.d);
diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js
index 83124697a8..442d91190b 100644
--- a/packages/svelte/src/internal/client/validate.js
+++ b/packages/svelte/src/internal/client/validate.js
@@ -118,7 +118,7 @@ export function add_snippet_symbol(fn) {
  * @param {any} snippet_fn
  */
 export function validate_snippet(snippet_fn) {
-	if (snippet_fn[snippet_symbol] !== true) {
+	if (snippet_fn && snippet_fn[snippet_symbol] !== true) {
 		throw new Error(
 			'The argument to `{@render ...}` must be a snippet function, not a component or some other kind of function. ' +
 				'If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}`.'
diff --git a/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/_config.js b/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/_config.js
new file mode 100644
index 0000000000..47bd37699c
--- /dev/null
+++ b/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/_config.js
@@ -0,0 +1,8 @@
+import { test } from '../../test';
+
+export default test({
+	error: {
+		code: 'invalid-render-call',
+		message: 'Calling a snippet function using apply, bind or call is not allowed'
+	}
+});
diff --git a/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/main.svelte b/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/main.svelte
new file mode 100644
index 0000000000..b299998eb8
--- /dev/null
+++ b/packages/svelte/tests/compiler-errors/samples/render-tag-invalid-call/main.svelte
@@ -0,0 +1 @@
+{@render snippet.apply(null, [1, 2, 3])}
\ No newline at end of file
diff --git a/packages/svelte/tests/parser-modern/samples/snippets/output.json b/packages/svelte/tests/parser-modern/samples/snippets/output.json
index c8f6b2bcda..a1b96c588c 100644
--- a/packages/svelte/tests/parser-modern/samples/snippets/output.json
+++ b/packages/svelte/tests/parser-modern/samples/snippets/output.json
@@ -131,9 +131,9 @@
 				"start": 83,
 				"end": 101,
 				"expression": {
-					"type": "Identifier",
+					"type": "CallExpression",
 					"start": 92,
-					"end": 95,
+					"end": 100,
 					"loc": {
 						"start": {
 							"line": 7,
@@ -141,29 +141,45 @@
 						},
 						"end": {
 							"line": 7,
-							"column": 12
+							"column": 17
 						}
 					},
-					"name": "foo"
-				},
-				"arguments": [
-					{
+					"callee": {
 						"type": "Identifier",
-						"start": 96,
-						"end": 99,
+						"start": 92,
+						"end": 95,
 						"loc": {
 							"start": {
 								"line": 7,
-								"column": 13
+								"column": 9
 							},
 							"end": {
 								"line": 7,
-								"column": 16
+								"column": 12
 							}
 						},
-						"name": "msg"
-					}
-				]
+						"name": "foo"
+					},
+					"arguments": [
+						{
+							"type": "Identifier",
+							"start": 96,
+							"end": 99,
+							"loc": {
+								"start": {
+									"line": 7,
+									"column": 13
+								},
+								"end": {
+									"line": 7,
+									"column": 16
+								}
+							},
+							"name": "msg"
+						}
+					],
+					"optional": false
+				}
 			}
 		],
 		"transparent": false
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-expressions/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/_config.js
new file mode 100644
index 0000000000..3ae1bd8358
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/_config.js
@@ -0,0 +1,42 @@
+import { test } from '../../test';
+
+export default test({
+	html: `
+		<p>foo</p>
+		<hr>
+		<p>foo</p>
+		<hr>
+		<p>foo</p>
+		<hr>
+		<p>foo</p>
+		<hr>
+		<p>bar</p>
+		<hr>
+		<hr>
+		<button>toggle</button>
+	`,
+
+	async test({ assert, target }) {
+		const btn = target.querySelector('button');
+		await btn?.click();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<p>bar</p>
+				<hr>
+				<p>bar</p>
+				<hr>
+				<p>foo</p>
+				<hr>
+				<p>foo</p>
+				<hr>
+				<p>foo</p>
+				<hr>
+				<p>foo</p>
+				<hr>
+				<p>foo</p>
+				<button>toggle</button>
+			`
+		);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-expressions/child.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/child.svelte
new file mode 100644
index 0000000000..2713f7d989
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/child.svelte
@@ -0,0 +1,21 @@
+<script>
+	let { snippets, snippet, optional } = $props();
+
+	function getOptional() {
+		return optional;
+	}
+</script>
+
+{@render snippets[snippet]()}
+<hr>
+{@render snippets?.[snippet]?.()}
+<hr>
+{@render snippets.foo()}
+<hr>
+{@render snippets.foo?.()}
+<hr>
+{@render (optional ?? snippets.bar)()}
+<hr>
+{@render optional?.()}
+<hr>
+{@render getOptional()?.()}
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-expressions/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/main.svelte
new file mode 100644
index 0000000000..a5e45d7b0e
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/snippet-expressions/main.svelte
@@ -0,0 +1,17 @@
+<script>
+	import Child from './child.svelte';
+	let snippet = $state('foo');
+	let show = $state(false);
+</script>
+
+{#snippet foo()}
+	<p>foo</p>
+{/snippet}
+
+{#snippet bar()}
+	<p>bar</p>
+{/snippet}
+
+<Child snippets={{foo, bar}} {snippet} optional={show ? foo : undefined} />
+
+<button on:click={() => { snippet = 'bar'; show = true; }}>toggle</button>
diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts
index 0fd664f098..3a1c3da786 100644
--- a/packages/svelte/types/index.d.ts
+++ b/packages/svelte/types/index.d.ts
@@ -475,7 +475,7 @@ declare module 'svelte/animate' {
 }
 
 declare module 'svelte/compiler' {
-	import type { AssignmentExpression, ClassDeclaration, Expression, FunctionDeclaration, Identifier, ImportDeclaration, ArrayExpression, MemberExpression, ObjectExpression, Pattern, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, FunctionExpression, Node, Program, SpreadElement } from 'estree';
+	import type { AssignmentExpression, ClassDeclaration, Expression, FunctionDeclaration, Identifier, ImportDeclaration, ArrayExpression, MemberExpression, ObjectExpression, Pattern, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, FunctionExpression, Node, Program, ChainExpression, SimpleCallExpression } from 'estree';
 	import type { Location } from 'locate-character';
 	import type { SourceMap } from 'magic-string';
 	import type { Context } from 'zimmerframe';
@@ -1197,8 +1197,7 @@ declare module 'svelte/compiler' {
 	/** A `{@render foo(...)} tag */
 	interface RenderTag extends BaseNode {
 		type: 'RenderTag';
-		expression: Identifier;
-		arguments: Array<Expression | SpreadElement>;
+		expression: SimpleCallExpression | (ChainExpression & { expression: SimpleCallExpression });
 	}
 
 	type Tag = ExpressionTag | HtmlTag | ConstTag | DebugTag | RenderTag;