Skip to content

Commit 260e93a

Browse files
seefeldbclaude
seefeldb
and
claude
authored
fix(transformation): Capture outer-scope variables in shorthand property assignments (#1954)
* Fix: Capture outer-scope variables in shorthand property assignments Fixes bug where variables in shorthand assignments inside map callbacks weren't captured. Filters ShorthandPropertyAssignment from declarations and checks if identifiers match callback parameters. * Fix: Support array binding patterns in closure parameter detection Array-destructured map parameters (e.g., `([item]) => ({ item })`) were triggering the `realDeclarations.length === 0` path but the branch never recognized ArrayBindingPattern names. This caused identifiers like `item` to be mis-classified as outer captures, resulting in `params: { item }` with `item` undefined at runtime. This fix treats array binding patterns the same way as object binding patterns by checking if identifiers match array-destructured parameter names. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 321a5e3 commit 260e93a

File tree

5 files changed

+325
-2
lines changed

5 files changed

+325
-2
lines changed

‎packages/ts-transformers/src/closures/transformer.ts‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,49 @@ function shouldCaptureIdentifier(
169169
return undefined;
170170
}
171171

172-
// Check if ALL declarations are within the callback
173-
const allDeclaredInside = declarations.every((decl) =>
172+
// Filter out shorthand property assignments - they're not real declarations,
173+
// they're just syntactic sugar that references the actual declaration elsewhere
174+
const realDeclarations = declarations.filter((decl) =>
175+
!ts.isShorthandPropertyAssignment(decl)
176+
);
177+
178+
// If all we have are shorthand property assignments, check if this identifier
179+
// is actually a parameter of the callback itself
180+
if (realDeclarations.length === 0) {
181+
// Check if there's a parameter with this name in the callback
182+
const isCallbackParam = func.parameters.some((param) => {
183+
if (ts.isIdentifier(param.name) && param.name.text === node.text) {
184+
return true;
185+
}
186+
// Also check destructured parameters
187+
if (ts.isObjectBindingPattern(param.name)) {
188+
return param.name.elements.some((element) =>
189+
ts.isBindingElement(element) &&
190+
ts.isIdentifier(element.name) &&
191+
element.name.text === node.text
192+
);
193+
}
194+
// Also check array binding patterns (e.g., ([item]) => ...)
195+
if (ts.isArrayBindingPattern(param.name)) {
196+
return param.name.elements.some((element) =>
197+
ts.isBindingElement(element) &&
198+
ts.isIdentifier(element.name) &&
199+
element.name.text === node.text
200+
);
201+
}
202+
return false;
203+
});
204+
205+
if (isCallbackParam) {
206+
return undefined; // Don't capture - it's just referencing a callback parameter
207+
}
208+
209+
// Not a callback parameter, must be from outer scope
210+
return node;
211+
}
212+
213+
// Check if ALL real declarations are within the callback
214+
const allDeclaredInside = realDeclarations.every((decl) =>
174215
isDeclaredWithinCallback(decl, func)
175216
);
176217

‎packages/ts-transformers/test/fixtures/closures/map-array-destructure-shorthand.expected.tsx‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import * as __ctHelpers from "commontools";
2+
import { recipe, UI } from "commontools";
3+
type ItemTuple = [
4+
item: string,
5+
count: number
6+
];
7+
interface State {
8+
items: ItemTuple[];
9+
}
10+
export default recipe({
11+
$schema: "https://json-schema.org/draft/2020-12/schema",
12+
type: "object",
13+
properties: {
14+
items: {
15+
type: "array",
16+
items: {
17+
$ref: "#/$defs/ItemTuple"
18+
}
19+
}
20+
},
21+
required: ["items"],
22+
$defs: {
23+
ItemTuple: {
24+
type: "array",
25+
items: {
26+
anyOf: [{
27+
type: "string"
28+
}, {
29+
type: "number"
30+
}]
31+
}
32+
}
33+
}
34+
} as const satisfies __ctHelpers.JSONSchema, ({ items }) => {
35+
return {
36+
[UI]: (<div>
37+
{/* Array destructured parameter - without fix, 'item' would be
38+
incorrectly captured in params due to shorthand usage in JSX */}
39+
{items.mapWithPattern(__ctHelpers.recipe({
40+
$schema: "https://json-schema.org/draft/2020-12/schema",
41+
type: "object",
42+
properties: {
43+
element: {
44+
$ref: "#/$defs/ItemTuple"
45+
},
46+
params: {
47+
type: "object",
48+
properties: {}
49+
}
50+
},
51+
required: ["element", "params"],
52+
$defs: {
53+
ItemTuple: {
54+
type: "array",
55+
items: {
56+
anyOf: [{
57+
type: "string"
58+
}, {
59+
type: "number"
60+
}]
61+
}
62+
}
63+
}
64+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<div data-item={element[0]}>{element[0]}</div>)), {})}
65+
66+
{/* Multiple array destructured params */}
67+
{items.mapWithPattern(__ctHelpers.recipe({
68+
$schema: "https://json-schema.org/draft/2020-12/schema",
69+
type: "object",
70+
properties: {
71+
element: {
72+
$ref: "#/$defs/ItemTuple"
73+
},
74+
params: {
75+
type: "object",
76+
properties: {}
77+
}
78+
},
79+
required: ["element", "params"],
80+
$defs: {
81+
ItemTuple: {
82+
type: "array",
83+
items: {
84+
anyOf: [{
85+
type: "string"
86+
}, {
87+
type: "number"
88+
}]
89+
}
90+
}
91+
}
92+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<div key={element[0]}>
93+
{element[0]}: {element[1]}
94+
</div>)), {})}
95+
</div>),
96+
};
97+
});
98+
// @ts-ignore: Internals
99+
function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
100+
// @ts-ignore: Internals
101+
h.fragment = __ctHelpers.h.fragment;

‎packages/ts-transformers/test/fixtures/closures/map-array-destructure-shorthand.input.tsx‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <cts-enable />
2+
import { recipe, UI } from "commontools";
3+
4+
type ItemTuple = [item: string, count: number];
5+
6+
interface State {
7+
items: ItemTuple[];
8+
}
9+
10+
export default recipe<State>("ArrayDestructureShorthand", ({ items }) => {
11+
return {
12+
[UI]: (
13+
<div>
14+
{/* Array destructured parameter - without fix, 'item' would be
15+
incorrectly captured in params due to shorthand usage in JSX */}
16+
{items.map(([item]) => (
17+
<div data-item={item}>{item}</div>
18+
))}
19+
20+
{/* Multiple array destructured params */}
21+
{items.map(([item, count]) => (
22+
<div key={item}>
23+
{item}: {count}
24+
</div>
25+
))}
26+
</div>
27+
),
28+
};
29+
});

‎packages/ts-transformers/test/fixtures/closures/map-capture-cell-param.expected.tsx‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import * as __ctHelpers from "commontools";
2+
import { Cell, Default, handler, recipe, UI } from "commontools";
3+
interface Item {
4+
text: Default<string, "">;
5+
}
6+
interface InputSchema {
7+
items: Default<Item[], [
8+
]>;
9+
}
10+
const removeItem = handler(true as const satisfies __ctHelpers.JSONSchema, {
11+
$schema: "https://json-schema.org/draft/2020-12/schema",
12+
type: "object",
13+
properties: {
14+
items: {
15+
type: "array",
16+
items: {
17+
$ref: "#/$defs/Item"
18+
},
19+
asCell: true
20+
},
21+
index: {
22+
type: "number"
23+
}
24+
},
25+
required: ["items", "index"],
26+
$defs: {
27+
Item: {
28+
type: "object",
29+
properties: {
30+
text: {
31+
type: "string",
32+
default: ""
33+
}
34+
},
35+
required: ["text"]
36+
}
37+
}
38+
} as const satisfies __ctHelpers.JSONSchema, (_, _2) => {
39+
// Not relevant for repro
40+
});
41+
export default recipe({
42+
$schema: "https://json-schema.org/draft/2020-12/schema",
43+
type: "object",
44+
properties: {
45+
items: {
46+
type: "array",
47+
items: {
48+
$ref: "#/$defs/Item"
49+
},
50+
default: []
51+
}
52+
},
53+
required: ["items"],
54+
$defs: {
55+
Item: {
56+
type: "object",
57+
properties: {
58+
text: {
59+
type: "string",
60+
default: ""
61+
}
62+
},
63+
required: ["text"]
64+
}
65+
}
66+
} as const satisfies __ctHelpers.JSONSchema, ({ items }) => {
67+
return {
68+
[UI]: (<ul>
69+
{items.mapWithPattern(__ctHelpers.recipe({
70+
$schema: "https://json-schema.org/draft/2020-12/schema",
71+
type: "object",
72+
properties: {
73+
element: {
74+
$ref: "#/$defs/Item"
75+
},
76+
index: {
77+
type: "number"
78+
},
79+
params: {
80+
type: "object",
81+
properties: {
82+
items: {
83+
type: "array",
84+
items: {
85+
$ref: "#/$defs/Item"
86+
},
87+
asOpaque: true
88+
}
89+
},
90+
required: ["items"]
91+
}
92+
},
93+
required: ["element", "params"],
94+
$defs: {
95+
Item: {
96+
type: "object",
97+
properties: {
98+
text: {
99+
type: "string",
100+
default: ""
101+
}
102+
},
103+
required: ["text"]
104+
}
105+
}
106+
} as const satisfies __ctHelpers.JSONSchema, ({ element, index, params: { items } }) => (<li key={index}>
107+
<ct-button onClick={removeItem({ items, index })}>
108+
Remove
109+
</ct-button>
110+
</li>)), { items: items })}
111+
</ul>),
112+
};
113+
});
114+
// @ts-ignore: Internals
115+
function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
116+
// @ts-ignore: Internals
117+
h.fragment = __ctHelpers.h.fragment;

‎packages/ts-transformers/test/fixtures/closures/map-capture-cell-param.input.tsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/// <cts-enable />
2+
import { Cell, Default, handler, recipe, UI } from "commontools";
3+
4+
interface Item {
5+
text: Default<string, "">;
6+
}
7+
8+
interface InputSchema {
9+
items: Default<Item[], []>;
10+
}
11+
12+
const removeItem = handler<unknown, { items: Cell<Item[]>; index: number }>(
13+
(_, _2) => {
14+
// Not relevant for repro
15+
},
16+
);
17+
18+
export default recipe<InputSchema>(
19+
"Simple List with Remove",
20+
({ items }) => {
21+
return {
22+
[UI]: (
23+
<ul>
24+
{items.map((_, index) => (
25+
<li key={index}>
26+
<ct-button onClick={removeItem({ items, index })}>
27+
Remove
28+
</ct-button>
29+
</li>
30+
))}
31+
</ul>
32+
),
33+
};
34+
},
35+
);

0 commit comments

Comments
 (0)