Skip to content

Commit d495fac

Browse files
seefeldbclaude
seefeldb
and
claude
authored
fix(transform): synthetic map params leak (#1955)
* Fix: Filter synthetic map params from outer scope derives Fixes bug where element/index/array leaked into outer scope derives. Adds special case handling for single non-synthetic dataflow with synthetic map params. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Refactor: Replace heuristics with explicit map callback tracking Replaces fragile heuristic-based detection of map callbacks with explicit marking using a shared WeakSet registry. This fixes bugs where synthetic identifiers (element, index, array) leaked into outer scope derives. **Problem:** - ClosureTransformer creates synthetic identifiers without symbols for map params - OpaqueRefJSXTransformer analyzes transformed AST and needs to distinguish: 1. Synthetic params INSIDE map callbacks (keep) 2. Synthetic params that leaked to outer scope (filter out) - Previous heuristics based on counting dataflows were brittle and error-prone **Solution:** 1. Add `mapCallbackRegistry: WeakSet<ts.Node>` to TransformationOptions 2. ClosureTransformer marks arrow functions when creating map callbacks 3. OpaqueRefJSXTransformer checks marking to detect map callback scopes 4. Filter logic: - Only synthetic dataflows → inside callback (keep all) - Mixed synthetic + non-synthetic → check if in marked callback - If not in marked callback → filter out synthetic params **Benefits:** - Direct knowledge instead of heuristics (no more magic thresholds) - Clear contract between transformers via shared registry - More maintainable - reduced from ~130 lines to ~100 lines - Fixes "element is not defined" runtime errors 🤖 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 e4dd4b5 commit d495fac

File tree

8 files changed

+175
-65
lines changed

8 files changed

+175
-65
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,9 @@ function createRecipeCallWithParams(
10151015
transformedBody,
10161016
);
10171017

1018+
// Mark this as a map callback for later transformers (e.g., OpaqueRefJSXTransformer)
1019+
context.markAsMapCallback(newCallback);
1020+
10181021
// Build a TypeNode for the callback parameter to pass as a type argument to recipe<T>()
10191022
const callbackParamTypeNode = buildCallbackParamTypeNode(
10201023
mapCall,

‎packages/ts-transformers/src/core/context.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,21 @@ export class TransformationContext {
6868
column: location.character + 1,
6969
});
7070
}
71+
72+
/**
73+
* Mark an arrow function as a map callback created by ClosureTransformer.
74+
* This allows later transformers to identify synthetic map callback scopes.
75+
*/
76+
markAsMapCallback(node: ts.Node): void {
77+
if (this.options.mapCallbackRegistry) {
78+
this.options.mapCallbackRegistry.add(node);
79+
}
80+
}
81+
82+
/**
83+
* Check if a node is a map callback created by ClosureTransformer.
84+
*/
85+
isMapCallback(node: ts.Node): boolean {
86+
return this.options.mapCallbackRegistry?.has(node) ?? false;
87+
}
7188
}

‎packages/ts-transformers/src/core/transformers.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface TransformationOptions {
88
readonly debug?: boolean;
99
readonly logger?: (message: string) => void;
1010
readonly typeRegistry?: TypeRegistry;
11+
readonly mapCallbackRegistry?: WeakSet<ts.Node>;
1112
}
1213

1314
export interface TransformationDiagnostic {

‎packages/ts-transformers/src/ct-pipeline.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export class CommonToolsTransformerPipeline extends Pipeline {
1010
constructor(options: TransformationOptions = {}) {
1111
const ops = {
1212
typeRegistry: new WeakMap(),
13+
mapCallbackRegistry: new WeakSet(),
1314
...options,
1415
};
1516
super([

‎packages/ts-transformers/src/transformers/opaque-ref/helpers.ts‎

Lines changed: 83 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -115,81 +115,100 @@ export function filterRelevantDataFlows(
115115
const syntheticDataFlows = dataFlows.filter((df) =>
116116
hasSyntheticRoot(df.expression)
117117
);
118-
const nonSyntheticDataFlows = dataFlows.filter((df) =>
119-
!hasSyntheticRoot(df.expression)
120-
);
121-
122-
// If we have both synthetic and non-synthetic dataflows, we need to determine
123-
// if we're inside or outside the map callback
124-
if (syntheticDataFlows.length > 0 && nonSyntheticDataFlows.length > 0) {
125-
// Check if any dataflow is in a scope with parameters from a map callback
126-
// If so, we're inside a callback being transformed and should keep all dataflows
127-
const isInMapCallbackScope = dataFlows.some((df) => {
128-
const scope = analysis.graph.scopes.find((s) => s.id === df.scopeId);
129-
if (!scope) return false;
130118

131-
// Check if any parameter in this scope comes from a builder or array-map
132-
return scope.parameters.some((param) => {
133-
if (!param.declaration) return false;
134-
const callKind = getOpaqueCallKindForParameter(
135-
param.declaration,
136-
context.checker,
137-
);
138-
return callKind === "builder" || callKind === "array-map";
139-
});
119+
// If we have synthetic dataflows (e.g., element, index, array from map callbacks),
120+
// these are identifiers without symbols that were created by ClosureTransformer.
121+
// We need to determine if they're being used in the correct scope or if they leaked.
122+
if (syntheticDataFlows.length > 0) {
123+
// Check if the synthetic identifiers are standard map callback parameter names
124+
const hasSyntheticMapParams = syntheticDataFlows.some((df) => {
125+
let rootExpr: ts.Expression = df.expression;
126+
while (
127+
ts.isPropertyAccessExpression(rootExpr) ||
128+
ts.isElementAccessExpression(rootExpr)
129+
) {
130+
rootExpr = rootExpr.expression;
131+
}
132+
if (ts.isIdentifier(rootExpr)) {
133+
const name = rootExpr.text;
134+
// Standard map callback parameter names created by ClosureTransformer
135+
return name === "element" || name === "index" || name === "array";
136+
}
137+
return false;
140138
});
141139

142-
// If we're inside a map callback scope, keep all dataflows
143-
if (isInMapCallbackScope) {
144-
// Keep all dataflows - we're inside a callback with both synthetic params and captures
145-
return dataFlows.filter((dataFlow) => {
146-
if (
147-
originatesFromIgnoredParameter(
148-
dataFlow.expression,
149-
dataFlow.scopeId,
150-
analysis,
151-
context.checker,
152-
)
153-
) {
154-
return false;
155-
}
156-
return true;
157-
});
158-
}
140+
if (hasSyntheticMapParams) {
141+
// We have synthetic map callback params. These could be:
142+
// 1. Inside a map callback (keep them)
143+
// 2. In outer scope where they leaked (filter them out)
159144

160-
// Heuristic: Check if the non-synthetic dataflows have symbols in the outer
161-
// scope If they reference outer-scope variables (like cells), we're at the
162-
// outer scope and should filter synthetic params.
163-
const nonSyntheticHaveOuterScopeSymbols = nonSyntheticDataFlows.every(
164-
(df) => {
165-
let rootExpr: ts.Expression = df.expression;
166-
while (
167-
ts.isPropertyAccessExpression(rootExpr) ||
168-
ts.isElementAccessExpression(rootExpr)
169-
) {
170-
rootExpr = rootExpr.expression;
171-
}
172-
if (ts.isIdentifier(rootExpr)) {
173-
const symbol = context.checker.getSymbolAtLocation(rootExpr);
174-
// If it has a symbol, it's likely from outer scope
175-
return !!symbol;
145+
const nonSyntheticDataFlows = dataFlows.filter((df) =>
146+
!hasSyntheticRoot(df.expression)
147+
);
148+
149+
// If we have ONLY synthetic dataflows, we're definitely inside a map callback
150+
if (nonSyntheticDataFlows.length === 0) {
151+
// Pure synthetic - we're inside a map callback, keep all
152+
return dataFlows.filter((dataFlow) => {
153+
if (
154+
originatesFromIgnoredParameter(
155+
dataFlow.expression,
156+
dataFlow.scopeId,
157+
analysis,
158+
context.checker,
159+
)
160+
) {
161+
return false;
162+
}
163+
return true;
164+
});
165+
}
166+
167+
// We have both synthetic and non-synthetic. This could be:
168+
// 1. Inside a map callback with captures (keep all)
169+
// 2. Outer scope with leaked synthetic params (filter synthetics)
170+
171+
// Try to find if any dataflow is from a scope with parameters that's a marked callback
172+
const isInMarkedCallback = dataFlows.some((df) => {
173+
const scope = analysis.graph.scopes.find((s) => s.id === df.scopeId);
174+
if (!scope || scope.parameters.length === 0) return false;
175+
176+
const firstParam = scope.parameters[0];
177+
if (!firstParam || !firstParam.declaration) return false;
178+
179+
let node: ts.Node | undefined = firstParam.declaration.parent;
180+
while (node) {
181+
if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) {
182+
return context.isMapCallback(node);
183+
}
184+
node = node.parent;
176185
}
177186
return false;
178-
},
179-
);
187+
});
180188

181-
// If all non-synthetic dataflows have outer-scope symbols AND we have 2 or
182-
// more of them, we're likely analyzing an outer-scope expression that
183-
// contains nested map callbacks
184-
if (
185-
nonSyntheticHaveOuterScopeSymbols && nonSyntheticDataFlows.length >= 2
186-
) {
189+
if (isInMarkedCallback) {
190+
// Inside a map callback - keep all except ignored params
191+
return dataFlows.filter((dataFlow) => {
192+
if (
193+
originatesFromIgnoredParameter(
194+
dataFlow.expression,
195+
dataFlow.scopeId,
196+
analysis,
197+
context.checker,
198+
)
199+
) {
200+
return false;
201+
}
202+
return true;
203+
});
204+
}
205+
206+
// Synthetic map params in outer scope - filter them out
187207
return dataFlows.filter((df) => !hasSyntheticRoot(df.expression));
188208
}
189-
190-
// Otherwise, we're inside a map callback with captures - keep all dataflows
191209
}
192210

211+
// No synthetic dataflows, use standard filtering
193212
return dataFlows.filter((dataFlow) => {
194213
if (
195214
originatesFromIgnoredParameter(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,4 @@ export default recipe({
6161
// @ts-ignore: Internals
6262
function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
6363
// @ts-ignore: Internals
64-
h.fragment = __ctHelpers.h.fragment;
64+
h.fragment = __ctHelpers.h.fragment;

‎packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.expected.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import * as __ctHelpers from "commontools";
2+
import { cell, recipe, UI } from "commontools";
3+
export default recipe("MapSingleCapture", (_state) => {
4+
const people = cell([
5+
{ id: "1", name: "Alice" },
6+
{ id: "2", name: "Bob" },
7+
]);
8+
return {
9+
[UI]: (<div>
10+
{__ctHelpers.derive(people, people => people.length > 0 && (<ul>
11+
{people.mapWithPattern(__ctHelpers.recipe({
12+
$schema: "https://json-schema.org/draft/2020-12/schema",
13+
type: "object",
14+
properties: {
15+
element: {
16+
$ref: "#/$defs/__object"
17+
},
18+
params: {
19+
type: "object",
20+
properties: {}
21+
}
22+
},
23+
required: ["element", "params"],
24+
$defs: {
25+
__object: {
26+
type: "object",
27+
properties: {
28+
id: {
29+
type: "string"
30+
},
31+
name: {
32+
type: "string"
33+
}
34+
},
35+
required: ["id", "name"]
36+
}
37+
}
38+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<li key={element.id}>{element.name}</li>)), {})}
39+
</ul>))}
40+
</div>),
41+
};
42+
});
43+
// @ts-ignore: Internals
44+
function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
45+
// @ts-ignore: Internals
46+
h.fragment = __ctHelpers.h.fragment;

‎packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.input.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <cts-enable />
2+
import { cell, recipe, UI } from "commontools";
3+
4+
export default recipe("MapSingleCapture", (_state) => {
5+
const people = cell([
6+
{ id: "1", name: "Alice" },
7+
{ id: "2", name: "Bob" },
8+
]);
9+
10+
return {
11+
[UI]: (
12+
<div>
13+
{people.length > 0 && (
14+
<ul>
15+
{people.map((person) => (
16+
<li key={person.id}>{person.name}</li>
17+
))}
18+
</ul>
19+
)}
20+
</div>
21+
),
22+
};
23+
});

0 commit comments

Comments
 (0)