Skip to content

Commit 945fce1

Browse files
JoviDeCroockJoviDeCroock
committed
Improve signal name detection
Assisted-By: devx/fd8c6a94-abf3-49a5-a874-456585b423f2
1 parent edabac6 commit 945fce1

5 files changed

Lines changed: 370 additions & 70 deletions

File tree

‎.changeset/bright-signals-name.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@preact/signals-preact-transform": patch
3+
"@preact/signals-react-transform": patch
4+
---
5+
6+
Improve generated debug names for signals created outside variable declarations.

‎packages/preact-transform/src/index.ts‎

Lines changed: 119 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,119 @@ function isSignalCall(path: NodePath<BabelTypes.CallExpression>): boolean {
3333
return false;
3434
}
3535

36-
function getVariableNameFromDeclarator(
36+
function getStaticName(node: BabelTypes.Node, computed = false): string | null {
37+
if (!computed && node.type === "Identifier") {
38+
return node.name;
39+
} else if (!computed && node.type === "PrivateName") {
40+
return `#${node.id.name}`;
41+
} else if (node.type === "StringLiteral" || node.type === "NumericLiteral") {
42+
return String(node.value);
43+
}
44+
45+
return null;
46+
}
47+
48+
function hasComputedKey(node: BabelTypes.Node): boolean {
49+
return "computed" in node && node.computed === true;
50+
}
51+
52+
function getAssignmentName(
53+
node: BabelTypes.AssignmentExpression["left"]
54+
): string | null {
55+
if (node.type === "Identifier") {
56+
return node.name;
57+
} else if (node.type === "MemberExpression") {
58+
return getStaticName(node.property, node.computed);
59+
}
60+
61+
return null;
62+
}
63+
64+
function getFunctionExpressionName(path: NodePath): string | null {
65+
const parentPath = path.parentPath;
66+
if (!parentPath) return null;
67+
68+
if (parentPath.isVariableDeclarator()) {
69+
return parentPath.node.id.type === "Identifier"
70+
? parentPath.node.id.name
71+
: null;
72+
} else if (parentPath.isAssignmentExpression()) {
73+
return getAssignmentName(parentPath.node.left);
74+
} else if (
75+
parentPath.isObjectProperty() ||
76+
parentPath.isClassProperty() ||
77+
parentPath.isClassPrivateProperty()
78+
) {
79+
return getStaticName(parentPath.node.key, hasComputedKey(parentPath.node));
80+
}
81+
82+
return null;
83+
}
84+
85+
function getSignalNameFromContext(
3786
path: NodePath<BabelTypes.CallExpression>
3887
): string | null {
39-
// Walk up the AST to find a variable declarator
40-
let currentPath: NodePath | null = path;
88+
let currentPath: NodePath | null = path.parentPath;
4189
while (currentPath) {
4290
if (
91+
currentPath.isArrowFunctionExpression() ||
92+
(currentPath.isFunctionExpression() && !currentPath.node.id)
93+
) {
94+
const name = getFunctionExpressionName(currentPath);
95+
if (name) return name;
96+
break;
97+
} else if (
4398
currentPath.isVariableDeclarator() &&
4499
currentPath.node.id.type === "Identifier"
45100
) {
46101
return currentPath.node.id.name;
102+
} else if (currentPath.isAssignmentExpression()) {
103+
const name = getAssignmentName(currentPath.node.left);
104+
if (name) return name;
105+
break;
106+
} else if (currentPath.isObjectProperty()) {
107+
const name = getStaticName(
108+
currentPath.node.key,
109+
hasComputedKey(currentPath.node)
110+
);
111+
if (name) return name;
112+
break;
113+
} else if (
114+
currentPath.isClassProperty() ||
115+
currentPath.isClassPrivateProperty()
116+
) {
117+
const name = getStaticName(
118+
currentPath.node.key,
119+
hasComputedKey(currentPath.node)
120+
);
121+
if (name) return name;
122+
break;
123+
} else if (
124+
(currentPath.isFunctionDeclaration() ||
125+
currentPath.isFunctionExpression()) &&
126+
currentPath.node.id
127+
) {
128+
return currentPath.node.id.name;
129+
} else if (
130+
currentPath.isObjectMethod() ||
131+
currentPath.isClassMethod() ||
132+
currentPath.isClassPrivateMethod()
133+
) {
134+
const name = getStaticName(
135+
currentPath.node.key,
136+
hasComputedKey(currentPath.node)
137+
);
138+
if (name) return name;
139+
break;
47140
}
48141
currentPath = currentPath.parentPath;
49142
}
50-
return null;
143+
144+
const callee = path.get("callee");
145+
return callee.isIdentifier() ? callee.node.name : null;
51146
}
52147

53-
function hasNameInOptions(
148+
function shouldSkipNameInjection(
54149
t: typeof BabelTypes,
55150
args: NodePath<
56151
| BabelTypes.Expression
@@ -59,24 +154,21 @@ function hasNameInOptions(
59154
| BabelTypes.ArgumentPlaceholder
60155
>[]
61156
): boolean {
62-
// Check if there's a second argument with a name property
63-
if (args.length >= 2) {
64-
const optionsArg = args[1];
65-
if (optionsArg.isObjectExpression()) {
66-
return optionsArg.node.properties.some(prop => {
67-
if (t.isObjectProperty(prop) && !prop.computed) {
68-
if (t.isIdentifier(prop.key, { name: "name" })) {
69-
return true;
70-
}
71-
if (t.isStringLiteral(prop.key) && prop.key.value === "name") {
72-
return true;
73-
}
74-
}
75-
return false;
76-
});
77-
}
157+
if (args.length < 2) return false;
158+
159+
const optionsArg = args[1];
160+
if (!optionsArg.isObjectExpression()) {
161+
// Non-literal options cannot be safely extended without changing semantics.
162+
return true;
78163
}
79-
return false;
164+
165+
return optionsArg.node.properties.some(prop => {
166+
if (t.isSpreadElement(prop)) return true;
167+
if (!t.isObjectProperty(prop)) return false;
168+
169+
const key = getStaticName(prop.key, prop.computed);
170+
return key === "name" || (key === null && prop.computed);
171+
});
80172
}
81173

82174
function injectSignalName(
@@ -112,19 +204,12 @@ function injectSignalName(
112204
]);
113205
path.node.arguments.push(nameOption);
114206
} else if (args.length >= 2) {
115-
// Two or more arguments, modify existing options object
207+
// Two or more arguments, modify existing literal options
116208
const optionsArg = args[1];
117209
if (optionsArg.isObjectExpression()) {
118-
// Add name property to existing options object
119210
optionsArg.node.properties.push(
120211
t.objectProperty(t.identifier("name"), name)
121212
);
122-
} else {
123-
// Replace second argument with options object containing name
124-
const nameOption = t.objectExpression([
125-
t.objectProperty(t.identifier("name"), name),
126-
]);
127-
args[1].replaceWith(nameOption);
128213
}
129214
}
130215
}
@@ -143,10 +228,10 @@ export default function signalsTransform(
143228
const args = path.get("arguments");
144229

145230
// Only inject name if it doesn't already have one
146-
if (!hasNameInOptions(t, args)) {
147-
const variableName = getVariableNameFromDeclarator(path);
148-
if (variableName) {
149-
injectSignalName(t, path, variableName, this.filename);
231+
if (!shouldSkipNameInjection(t, args)) {
232+
const signalName = getSignalNameFromContext(path);
233+
if (signalName) {
234+
injectSignalName(t, path, signalName, this.filename);
150235
}
151236
}
152237
}

‎packages/preact-transform/test/node/index.test.tsx‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ describe("Preact Signals Babel Transform", () => {
101101
function MyComponent() {
102102
const count = signal(0, { name: "myCounter" });
103103
const data = useSignal(null, { name: "userData", watched: () => {} });
104+
const total = computed(() => 1, { ["name"]: "total" });
104105
return <div>{count.value}</div>;
105106
}
106107
`;
@@ -114,6 +115,9 @@ describe("Preact Signals Babel Transform", () => {
114115
name: "userData",
115116
watched: () => {},
116117
});
118+
const total = computed(() => 1, {
119+
["name"]: "total",
120+
});
117121
return <div>{count.value}</div>;
118122
}
119123
`;
@@ -140,5 +144,62 @@ describe("Preact Signals Babel Transform", () => {
140144

141145
await runDebugTest(inputCode, expectedOutput, "Component.js");
142146
});
147+
148+
it("derives names from surrounding syntax", () => {
149+
const inputCode = [
150+
"class Store {",
151+
" count = signal(0);",
152+
" #selected = signal(false);",
153+
" constructor() {",
154+
' this.status = signal("idle");',
155+
' this["message"] = computed(() => "");',
156+
" }",
157+
" load() {",
158+
" return computed(() => this.status.value);",
159+
" }",
160+
"}",
161+
"const model = {",
162+
" enabled: signal(true),",
163+
' "label": computed(() => "label"),',
164+
' 0: signal("zero"),',
165+
"};",
166+
"function createVisible() {",
167+
" return signal(true);",
168+
"}",
169+
"consume(signal(false));",
170+
"const createSelected = () => signal(true);",
171+
"const rows = values.map(() => signal(0));",
172+
'const key = "dynamic";',
173+
"const dynamic = {[key]: computed(() => key)};",
174+
"const options = getOptions();",
175+
"const preserved = signal(0, options);",
176+
"const spread = signal(0, {...options});",
177+
"[first] = [signal(0)];",
178+
].join("\n");
179+
180+
const output = transformCode(inputCode, DEBUG_OPTIONS, "Models.js");
181+
182+
for (const expectedName of [
183+
"count (Models.js:2)",
184+
"#selected (Models.js:3)",
185+
"status (Models.js:5)",
186+
"message (Models.js:6)",
187+
"load (Models.js:9)",
188+
"enabled (Models.js:13)",
189+
"label (Models.js:14)",
190+
"0 (Models.js:15)",
191+
"createVisible (Models.js:18)",
192+
"signal (Models.js:20)",
193+
"createSelected (Models.js:21)",
194+
"signal (Models.js:22)",
195+
"computed (Models.js:24)",
196+
"signal (Models.js:28)",
197+
]) {
198+
expect(output).toContain(`name: "${expectedName}"`);
199+
}
200+
expect(output).toContain("const preserved = signal(0, options);");
201+
expect(output).not.toContain("preserved (Models.js:26)");
202+
expect(output).not.toContain("spread (Models.js:27)");
203+
});
143204
});
144205
});

0 commit comments

Comments
 (0)