fix: rework directive name handling (#9470)

* move snapshot test to a runtime test

* handle dynamic cases

* huh

---------

Co-authored-by: Rich Harris <[email protected]>

Author: Rich-Harris

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

@@ -34,6 +34,7 @@ import {
 	EACH_ITEM_REACTIVE,
 	EACH_KEYED
 } from '../../../../../constants.js';
+import { regex_is_valid_identifier } from '../../../patterns.js';
 
 /**
  * Serializes each style directive into something like `$.style(element, style_property, value)`
@@ -75,19 +76,24 @@ function serialize_style_directives(style_directives, element_id, context, is_at
 }
 
 /**
- * goes from nested.access to nested['access']
- * @param {string} expression
+ * For unfortunate legacy reasons, directive names can look like this `use:a.b-c`
+ * This turns that string into a member expression
+ * @param {string} name
  */
-function member_expression_id_to_literal(expression) {
+function parse_directive_name(name) {
 	// this allow for accessing members of an object
-	const splitted_expression = expression.split('.');
+	const parts = name.split('.');
+	let part = /** @type {string} */ (parts.shift());
 
-	let new_expression = splitted_expression.shift() ?? '';
+	/** @type {import('estree').Identifier | import('estree').MemberExpression} */
+	let expression = b.id(part);
 
-	for (let new_piece of splitted_expression) {
-		new_expression += `['${new_piece}']`;
+	while ((part = /** @type {string} */ (parts.shift()))) {
+		const computed = !regex_is_valid_identifier.test(part);
+		expression = b.member(expression, computed ? b.literal(part) : b.id(part), computed);
 	}
-	return new_expression;
+
+	return expression;
 }
 
 /**
@@ -1697,7 +1703,7 @@ export const template_visitors = {
 				b.call(
 					'$.animate',
 					state.node,
-					b.id(member_expression_id_to_literal(node.name)),
+					/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
 					expression
 				)
 			)
@@ -1721,7 +1727,7 @@ export const template_visitors = {
 				b.call(
 					type,
 					state.node,
-					b.id(member_expression_id_to_literal(node.name)),
+					/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
 					expression,
 					node.modifiers.includes('global') ? b.true : b.false
 				)
@@ -2445,7 +2451,7 @@ export const template_visitors = {
 			b.arrow(
 				params,
 				b.call(
-					serialize_get_binding(b.id(member_expression_id_to_literal(node.name)), state),
+					/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
 					...params
 				)
 			)

packages/svelte/tests/runtime-runes/samples/directives-with-member-access/_config.js

@@ -0,0 +1,5 @@
+import { test } from '../../test';
+
+// no need to compare the rendered HTML — we only care
+// that the generated code is valid
+export default test({});

packages/svelte/tests/runtime-runes/samples/directives-with-member-access/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	/**
+	 * @param {Element} [node]
+	 * @param {any} [options]
+	 */
+	const fn = (node, options) => ({});
+
+	let a = { b: { 'c-d': fn } };
+
+	let directive = $derived(a);
+</script>
+
+<!-- these will yield TypeScript errors, because it looks like e.g. `nested.with - string`,
+     in other words a number. Relatedly, people should not do this. It is stupid. -->
+<div use:directive.b.c-d />
+<div transition:directive.b.c-d />
+<div animate:directive.b.c-d />
+<div in:directive.b.c-d />
+<div out:directive.b.c-d />

packages/svelte/tests/snapshot/samples/directives-with-member-access/_config.js

@@ -7,14 +7,11 @@ import { VERSION } from 'svelte/compiler';
 
 interface SnapshotTest extends BaseTest {
 	compileOptions?: Partial<import('#compiler').CompileOptions>;
-	skip_if_ssr?: boolean;
 }
 
 const { test, run } = suite<SnapshotTest>(async (config, cwd) => {
 	compile_directory(cwd, 'client', config.compileOptions);
-	if (!config.skip_if_ssr) {
-		compile_directory(cwd, 'server', config.compileOptions);
-	}
+	compile_directory(cwd, 'server', config.compileOptions);
 
 	// run `UPDATE_SNAPSHOTS=true pnpm test snapshot` to update snapshot tests
 	if (process.env.UPDATE_SNAPSHOTS) {