Skip to content

Commit 321a5e3

Browse files
seefeldbclaude
seefeldb
and
claude
authored
Fix: Filter synthetic identifiers from outer scope derives (#1953)
* Fix: Filter synthetic identifiers from outer scope derives This fixes a bug where synthetic identifiers created by ClosureTransformer were leaking into derive calls in outer scopes, causing ReferenceError at runtime. The fix adds heuristic filtering in filterRelevantDataFlows() to detect when synthetic identifiers (those without symbol information) are being incorrectly included in outer-scope expressions. The heuristic: - Identifies synthetic vs non-synthetic dataflows by checking for symbol info - When mixing both: if non-synthetic flows all have outer-scope symbols (2+), we're at outer scope → filter synthetic ones - Otherwise we're inside a callback with captures → keep everything This approach is robust as it relies on symbol presence rather than hardcoded identifier names, so it won't break if user code happens to use variables named "element", "index", etc. Also adds test case for nested map within conditional to prevent regression and updates expected output for map-nested-conditional to correctly derive element properties. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(ts-transformers): keep synthetic params in callbacks with multiple captures The heuristic for filtering synthetic dataflows was too aggressive. It treated callbacks that capture 2+ outer variables as outer-scope expressions, which caused it to drop synthetic map parameter dataflows. This broke derive calls when a callback referenced multiple captured identifiers plus the synthetic parameter. Example that was breaking: ```tsx items.map((item, index) => ( <div> {item.name} - {cell1.value} - {cell2.value} - {index} </div> )) ``` The heuristic saw 2 outer-scope symbols (cell1, cell2) and incorrectly filtered out the synthetic parameters (item, index). Fix: - Check if dataflows belong to a map callback scope before applying the "2+ outer captures" heuristic - If in a map callback scope, keep all dataflows (synthetic and captures) - Only filter out ignored parameters - This allows callbacks to legitimately capture multiple outer variables while still using synthetic map parameters 🤖 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 78cf029 commit 321a5e3

File tree

3 files changed

+171
-0
lines changed

3 files changed

+171
-0
lines changed

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ function originatesFromIgnoredParameter(
4141
const inner = (expr: ts.Expression): boolean => {
4242
if (ts.isIdentifier(expr)) {
4343
const symbol = checker.getSymbolAtLocation(expr);
44+
45+
// Don't filter identifiers without symbols here - they might be synthetic
46+
// identifiers created by transformers (like map callback parameters), or
47+
// they might be legitimate identifiers that lost their symbols. Let
48+
// filterRelevantDataFlows handle this with more context about all the
49+
// dataflows being analyzed together.
50+
if (!symbol) {
51+
return false;
52+
}
53+
4454
return isIgnoredSymbol(symbol);
4555
}
4656
if (
@@ -85,6 +95,101 @@ export function filterRelevantDataFlows(
8595
analysis: DataFlowAnalysis,
8696
context: TransformationContext,
8797
): NormalizedDataFlow[] {
98+
// Check if we have identifiers without symbols (synthetic identifiers created by transformers)
99+
const hasSyntheticRoot = (expr: ts.Expression): boolean => {
100+
let current = expr;
101+
while (
102+
ts.isPropertyAccessExpression(current) ||
103+
ts.isElementAccessExpression(current)
104+
) {
105+
current = current.expression;
106+
}
107+
if (ts.isIdentifier(current)) {
108+
const symbol = context.checker.getSymbolAtLocation(current);
109+
// No symbol means it's likely a synthetic identifier
110+
return !symbol;
111+
}
112+
return false;
113+
};
114+
115+
const syntheticDataFlows = dataFlows.filter((df) =>
116+
hasSyntheticRoot(df.expression)
117+
);
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;
130+
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+
});
140+
});
141+
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+
}
159+
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;
176+
}
177+
return false;
178+
},
179+
);
180+
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+
) {
187+
return dataFlows.filter((df) => !hasSyntheticRoot(df.expression));
188+
}
189+
190+
// Otherwise, we're inside a map callback with captures - keep all dataflows
191+
}
192+
88193
return dataFlows.filter((dataFlow) => {
89194
if (
90195
originatesFromIgnoredParameter(

‎packages/ts-transformers/test/fixtures/jsx-expressions/map-nested-conditional.expected.tsx‎

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

‎packages/ts-transformers/test/fixtures/jsx-expressions/map-nested-conditional.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("MapNestedConditional", (_state) => {
5+
const items = cell([{ name: "apple" }, { name: "banana" }]);
6+
const showList = cell(true);
7+
8+
return {
9+
[UI]: (
10+
<div>
11+
{showList && (
12+
<div>
13+
{items.map((item) => (
14+
<div>
15+
{item.name && <span>{item.name}</span>}
16+
</div>
17+
))}
18+
</div>
19+
)}
20+
</div>
21+
),
22+
};
23+
});

0 commit comments

Comments
 (0)