Skip to content

Commit d749685

Browse files
authored
fix: rework directive name handling (#9470)
* move snapshot test to a runtime test * handle dynamic cases * huh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
1 parent 73e8820 commit d749685

File tree

7 files changed

+42
-168
lines changed

7 files changed

+42
-168
lines changed

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

+17-11
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
EACH_ITEM_REACTIVE,
3535
EACH_KEYED
3636
} from '../../../../../constants.js';
37+
import { regex_is_valid_identifier } from '../../../patterns.js';
3738

3839
/**
3940
* 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
7576
}
7677

7778
/**
78-
* goes from nested.access to nested['access']
79-
* @param {string} expression
79+
* For unfortunate legacy reasons, directive names can look like this `use:a.b-c`
80+
* This turns that string into a member expression
81+
* @param {string} name
8082
*/
81-
function member_expression_id_to_literal(expression) {
83+
function parse_directive_name(name) {
8284
// this allow for accessing members of an object
83-
const splitted_expression = expression.split('.');
85+
const parts = name.split('.');
86+
let part = /** @type {string} */ (parts.shift());
8487

85-
let new_expression = splitted_expression.shift() ?? '';
88+
/** @type {import('estree').Identifier | import('estree').MemberExpression} */
89+
let expression = b.id(part);
8690

87-
for (let new_piece of splitted_expression) {
88-
new_expression += `['${new_piece}']`;
91+
while ((part = /** @type {string} */ (parts.shift()))) {
92+
const computed = !regex_is_valid_identifier.test(part);
93+
expression = b.member(expression, computed ? b.literal(part) : b.id(part), computed);
8994
}
90-
return new_expression;
95+
96+
return expression;
9197
}
9298

9399
/**
@@ -1697,7 +1703,7 @@ export const template_visitors = {
16971703
b.call(
16981704
'$.animate',
16991705
state.node,
1700-
b.id(member_expression_id_to_literal(node.name)),
1706+
/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
17011707
expression
17021708
)
17031709
)
@@ -1721,7 +1727,7 @@ export const template_visitors = {
17211727
b.call(
17221728
type,
17231729
state.node,
1724-
b.id(member_expression_id_to_literal(node.name)),
1730+
/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
17251731
expression,
17261732
node.modifiers.includes('global') ? b.true : b.false
17271733
)
@@ -2445,7 +2451,7 @@ export const template_visitors = {
24452451
b.arrow(
24462452
params,
24472453
b.call(
2448-
serialize_get_binding(b.id(member_expression_id_to_literal(node.name)), state),
2454+
/** @type {import('estree').Expression} */ (visit(parse_directive_name(node.name))),
24492455
...params
24502456
)
24512457
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { test } from '../../test';
2+
3+
// no need to compare the rendered HTML — we only care
4+
// that the generated code is valid
5+
export default test({});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<script>
2+
/**
3+
* @param {Element} [node]
4+
* @param {any} [options]
5+
*/
6+
const fn = (node, options) => ({});
7+
8+
let a = { b: { 'c-d': fn } };
9+
10+
let directive = $derived(a);
11+
</script>
12+
13+
<!-- these will yield TypeScript errors, because it looks like e.g. `nested.with - string`,
14+
in other words a number. Relatedly, people should not do this. It is stupid. -->
15+
<div use:directive.b.c-d />
16+
<div transition:directive.b.c-d />
17+
<div animate:directive.b.c-d />
18+
<div in:directive.b.c-d />
19+
<div out:directive.b.c-d />

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

-5
This file was deleted.

Diff for: packages/svelte/tests/snapshot/samples/directives-with-member-access/_expected/client/index.svelte.js

-108
This file was deleted.

Diff for: packages/svelte/tests/snapshot/samples/directives-with-member-access/index.svelte

-40
This file was deleted.

Diff for: packages/svelte/tests/snapshot/test.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,11 @@ import { VERSION } from 'svelte/compiler';
77

88
interface SnapshotTest extends BaseTest {
99
compileOptions?: Partial<import('#compiler').CompileOptions>;
10-
skip_if_ssr?: boolean;
1110
}
1211

1312
const { test, run } = suite<SnapshotTest>(async (config, cwd) => {
1413
compile_directory(cwd, 'client', config.compileOptions);
15-
if (!config.skip_if_ssr) {
16-
compile_directory(cwd, 'server', config.compileOptions);
17-
}
14+
compile_directory(cwd, 'server', config.compileOptions);
1815

1916
// run `UPDATE_SNAPSHOTS=true pnpm test snapshot` to update snapshot tests
2017
if (process.env.UPDATE_SNAPSHOTS) {

0 commit comments

Comments
 (0)