Skip to content

Commit 4982203

Browse files
mathpirateclaude
andcommitted
fix: wrap synthetic element access with opaque refs in derive
Fixes a bug where element access expressions like `tagCounts[element]` inside map callbacks weren't being wrapped in derive() calls. The issue occurred because these expressions are synthetic (created by the ClosureTransformer) and the dataflow analyzer wasn't properly handling synthetic element access with dynamic opaque indices. Changes: - Extract isStaticElementAccess() helper to share logic between synthetic and non-synthetic code paths - Add proper handling for synthetic element access expressions: - Static indices (e.g., element[0]) preserve merged analysis - Dynamic indices with opaque refs (e.g., tagCounts[element]) set requiresRewrite=true to trigger derive wrapper - Update test expectations to use original variable names instead of _v1, _v2 (matches pattern in other test fixtures) - Fix HTML-encoded directive in test input file 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b0e9aa2 commit 4982203

File tree

4 files changed

+32
-9
lines changed

4 files changed

+32
-9
lines changed

packages/ts-transformers/src/ast/dataflow.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ export function createDataFlowAnalyzer(
199199
};
200200
};
201201

202+
// Helper: Check if an element access expression has a static (literal) index
203+
const isStaticElementAccess = (expression: ts.ElementAccessExpression): boolean => {
204+
const argumentExpression = expression.argumentExpression;
205+
return argumentExpression !== undefined &&
206+
ts.isExpression(argumentExpression) &&
207+
(ts.isLiteralExpression(argumentExpression) ||
208+
ts.isNoSubstitutionTemplateLiteral(argumentExpression));
209+
};
210+
202211
const analyzeExpression = (
203212
expression: ts.Expression,
204213
scope: DataFlowScopeInternal,
@@ -467,8 +476,25 @@ export function createDataFlowAnalyzer(
467476
};
468477
}
469478

470-
// For JSX elements, arrow functions, and other expression containers, preserve requiresRewrite from children
471-
// This matches the non-synthetic code paths for these node types
479+
// Element access expressions: static indices don't need derive wrapping,
480+
// but dynamic indices with opaque refs do (e.g., tagCounts[element])
481+
if (ts.isElementAccessExpression(expression)) {
482+
const isStaticIndex = isStaticElementAccess(expression);
483+
484+
if (isStaticIndex) {
485+
// Static index like element[0] - preserve merged analysis
486+
return merged;
487+
} else if (merged.containsOpaqueRef) {
488+
// Dynamic index with opaque refs - requires derive wrapper
489+
return {
490+
...merged,
491+
requiresRewrite: true,
492+
};
493+
}
494+
return merged;
495+
}
496+
497+
// JSX elements, arrow functions, and other containers preserve requiresRewrite from children
472498
if (
473499
ts.isJsxElement(expression) ||
474500
ts.isJsxFragment(expression) ||
@@ -747,10 +773,7 @@ export function createDataFlowAnalyzer(
747773
? analyzeExpression(argumentExpression, scope, context)
748774
: emptyAnalysis();
749775

750-
const isStaticIndex = argumentExpression &&
751-
ts.isExpression(argumentExpression) &&
752-
(ts.isLiteralExpression(argumentExpression) ||
753-
ts.isNoSubstitutionTemplateLiteral(argumentExpression));
776+
const isStaticIndex = isStaticElementAccess(expression);
754777

755778
if (isStaticIndex) {
756779
const result = mergeAnalyses(target, argument);

packages/ts-transformers/test/fixtures/closures/map-element-access-opaque.expected.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export default recipe({
4949
},
5050
required: ["element", "params"]
5151
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: { tagCounts } }) => (<span>
52-
{element}: {__ctHelpers.derive({ tagCounts, element }, ({ tagCounts: _v1, element: _v2 }) => _v1[_v2])}
52+
{element}: {__ctHelpers.derive({ tagCounts, element }, ({ tagCounts: tagCounts, element: element }) => tagCounts[element])}
5353
</span>)), { tagCounts: state.tagCounts })}
5454
</div>),
5555
};

packages/ts-transformers/test/fixtures/jsx-expressions/element-access-both-opaque.expected.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export default recipe("ElementAccessBothOpaque", (_state) => {
77
[UI]: (<div>
88
<h3>Element Access with Both OpaqueRefs</h3>
99
{/* Both items and index are OpaqueRefs */}
10-
<p>Selected item: {__ctHelpers.derive({ items, index }, ({ items: _v1, index: _v2 }) => _v1[_v2])}</p>
10+
<p>Selected item: {__ctHelpers.derive({ items, index }, ({ items: items, index: index }) => items[index])}</p>
1111
</div>),
1212
};
1313
});

packages/ts-transformers/test/fixtures/jsx-expressions/element-access-both-opaque.input.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/// &lt;cts-enable />
1+
/// <cts-enable />
22
import { cell, recipe, UI } from "commontools";
33

44
export default recipe("ElementAccessBothOpaque", (_state) => {

0 commit comments

Comments
 (0)