Feature/map closures hierarchical params v2

[mathpirate]

WIP, not quite ready for review yet

Implement new approach to rewriting map closures and opaque ref JSX expressions. Build a capture tree and a hierarchical params object so that the transformed bodies of map callbacks and opaque ref JSX expressions can be unchanged.

---

Summary by cubic

Introduces hierarchical params and a capture tree to rewrite map closures and OpaqueRef JSX while keeping callback bodies unchanged. Preserves original variable names, groups captures by root, standardizes derive to use destructured grouped params, and handles optional chaining, computed property keys, and name collisions.

• New Features

  • Build a capture tree for closures and OpaqueRef JSX to group captures by root and path.
  • Generate a hierarchical params object and destructure it in callbacks to keep bodies unchanged.
  • Derive accepts grouped capture objects and uses destructured params.
  • Support optional chaining and computed property keys in captured expressions.

• Refactors

  • Centralized capture-tree and identifier utils; unique-name generation/reuse prevents collisions and unbound names.
  • Simplified closure and opaque-ref transformers; removed flattening and avoided element/index shadowing.
  • Updated docs and tests to match the hierarchical params spec.

^{Written for commit 6d21b5f. Summary will update automatically on new commits.}

[cubic-dev-ai]

14 issues found across 48 files

Prompt for AI agents (all 14 issues)

Understand the root cause of the following 14 issues and fix them.


<file name="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-arithmetic-operations.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-arithmetic-operations.expected.tsx:32">
The derive callback receives the entire `{ state: { … } }` object, so `state.count` is undefined; destructure the `state` property before using it.</violation>
</file>

<file name="packages/ts-transformers/src/transformers/builtins/derive.ts">

<violation number="1" location="packages/ts-transformers/src/transformers/builtins/derive.ts:132">
Fallback refs mapped via refToParamName can end up replaced with identifiers that are no longer bound because register(entry.paramName) renames them when a root name already used the same text. Please keep the map in sync with any renaming so the lambda binds the identifier that replaceOpaqueRefsWithParams emits.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/recipe-with-cells.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/recipe-with-cells.expected.tsx:17">
The derive callback still expects `cell` to be the captured object, but the call now passes `{ cell: { value: cell.value } }`, so `cell.value` is undefined at runtime. Destructure the params to keep the original body working.</violation>

<violation number="2" location="packages/ts-transformers/test/fixtures/jsx-expressions/recipe-with-cells.expected.tsx:20">
Like the previous derive call, this callback still assumes `cell` refers directly to the captured object, but the params object wraps it under `cell`. Destructure the params before using `cell.value`.</violation>
</file>

<file name="packages/ts-transformers/src/utils/identifiers.ts">

<violation number="1" location="packages/ts-transformers/src/utils/identifiers.ts:43">
sanitizeIdentifierCandidate can enter an infinite loop when the provided fallback starts with an invalid identifier character because the loop never changes the leading character.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx:157">
`derive` now receives an object shaped `{ state: { ... } }`, but the callback still expects `state` to be the original state. Without destructuring  (`({ state }) =&gt; …`), each callback will read `state.user`/`state.config` from the wrapper object and crash because `state` lacks those properties.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/recipe-statements-vs-jsx.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/recipe-statements-vs-jsx.expected.tsx:63">
The derive callback now receives the whole capture object (`{ state: {...} }`), so `state =&gt; state.value + 1` reads `state.value` from the wrong level and returns NaN. Destructure the callback parameter (and apply the same fix to the other new derive calls in this file).</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-conditional-rendering.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-conditional-rendering.expected.tsx:44">
The derive callback still receives the whole capture object `{ state: { … } }`, so `state.count` is undefined and the ternary always resolves to the fallback; destructure `{ state }` (or equivalent) before using the original body so the logic remains correct.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-string-operations.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-string-operations.expected.tsx:38">
The callback here receives the entire capture object `{ state: { ... } }`, so `state.title` is undefined. Destructure `{ state }` (or adjust the body) before using its properties.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/map-nested-conditional.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/map-nested-conditional.expected.tsx:15">
The derive callback is reading item.name from the wrong level; it receives { item: { name } } so item.name is always undefined, breaking the conditional span. Destructure the callback parameter to access the nested item first.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/element-access-complex.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/element-access-complex.expected.tsx:118">
The derive callback now receives an object shaped { state: { … } }, but the parameter is used as if matrix/row/col were top-level properties, so state.matrix is undefined and the access fails. Destructure { state } (or update the body) before using it.</violation>
</file>

<file name="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx">

<violation number="1" location="packages/ts-transformers/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx:66">
The derive callback still treats the input as flat (`state.items`), but the captured data is now nested under `state`. This makes `state.items` undefined and will throw before rendering; destructure the nested `state` object before accessing it.</violation>
</file>

<file name="packages/ts-transformers/src/utils/capture-tree.ts">

<violation number="1" location="packages/ts-transformers/src/utils/capture-tree.ts:124">
Captures that use optional chaining are rebuilt as plain property accesses, so a capture like maybe?.value becomes maybe.value. If maybe is nullish this now throws instead of returning undefined, breaking runtime behavior.</violation>
</file>

<file name="packages/ts-transformers/src/closures/transformer.ts">

<violation number="1" location="packages/ts-transformers/src/closures/transformer.ts:915">
If the original callback captures an outer-scope identifier named `element` while omitting the element parameter, this transformation introduces a new parameter named `element` and silently rewires the body to read that instead of the captured variable, changing the result.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

Fallback refs mapped via refToParamName can end up replaced with identifiers that are no longer bound because register(entry.paramName) renames them when a root name already used the same text. Please keep the map in sync with any renaming so the lambda binds the identifier that replaceOpaqueRefsWithParams emits.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/transformers/builtins/derive.ts at line 132:

<comment>Fallback refs mapped via refToParamName can end up replaced with identifiers that are no longer bound because register(entry.paramName) renames them when a root name already used the same text. Please keep the map in sync with any renaming so the lambda binds the identifier that replaceOpaqueRefsWithParams emits.</comment>

<file context>
@@ -35,74 +40,119 @@ export interface DeriveCallOptions {
-    )
-  );
+  for (const entry of fallbackEntries) {
+    const bindingIdentifier = register(entry.paramName);
+    bindings.push(
+      factory.createBindingElement(
</file context>

[cubic-dev-ai]

sanitizeIdentifierCandidate can enter an infinite loop when the provided fallback starts with an invalid identifier character because the loop never changes the leading character.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/utils/identifiers.ts at line 43:

<comment>sanitizeIdentifierCandidate can enter an infinite loop when the provided fallback starts with an invalid identifier character because the loop never changes the leading character.</comment>

<file context>
@@ -0,0 +1,132 @@
+  raw: string,
+  options: SanitizeIdentifierOptions = {},
+): string {
+  const fallback = options.fallback ?? DEFAULT_FALLBACK;
+
+  let candidate = raw;
</file context>

[cubic-dev-ai]

Captures that use optional chaining are rebuilt as plain property accesses, so a capture like maybe?.value becomes maybe.value. If maybe is nullish this now throws instead of returning undefined, breaking runtime behavior.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/utils/capture-tree.ts at line 124:

<comment>Captures that use optional chaining are rebuilt as plain property accesses, so a capture like maybe?.value becomes maybe.value. If maybe is nullish this now throws instead of returning undefined, breaking runtime behavior.</comment>

<file context>
@@ -0,0 +1,147 @@
+  factory: ts.NodeFactory,
+): ts.Expression {
+  if (node.expression &amp;&amp; node.properties.size === 0) {
+    return createCaptureAccessExpression(rootName, node.path, factory);
+  }
+
</file context>

[cubic-dev-ai]

If the original callback captures an outer-scope identifier named element while omitting the element parameter, this transformation introduces a new parameter named element and silently rewires the body to read that instead of the captured variable, changing the result.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/closures/transformer.ts at line 915:

<comment>If the original callback captures an outer-scope identifier named `element` while omitting the element parameter, this transformation introduces a new parameter named `element` and silently rewires the body to read that instead of the captured variable, changing the result.</comment>

<file context>
@@ -1099,57 +889,81 @@ function createRecipeCallWithParams(
+  const elementBindingName = elemParam
+    ? normalizeBindingName(elemParam.name, factory, usedBindingNames)
+    : maybeReuseIdentifier(
+      factory.createIdentifier(&quot;element&quot;),
+      usedBindingNames,
+    );
</file context>