Fix map capture collisions (#1975)

* Fix map closure destructured alias handling and add regression fixtures

* Fix map closure capture name collisions and add regression fixture

* Avoid recomputing computed destructured aliases and add fixture

Author: mathpirate

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

@@ -897,17 +897,114 @@ function transformDestructuredProperties(
 ): ts.ConciseBody {
   const elemName = elemParam?.name;
 
-  // Collect destructured property names if the param is an object destructuring pattern
-  const destructuredProps = new Set<string>();
+  let transformedBody: ts.ConciseBody = body;
+
+  const prependStatements = (
+    statements: readonly ts.Statement[],
+    currentBody: ts.ConciseBody,
+  ): ts.ConciseBody => {
+    if (statements.length === 0) return currentBody;
+
+    if (ts.isBlock(currentBody)) {
+      return factory.updateBlock(
+        currentBody,
+        factory.createNodeArray([
+          ...statements,
+          ...currentBody.statements,
+        ]),
+      );
+    }
+
+    return factory.createBlock(
+      [
+        ...statements,
+        factory.createReturnStatement(currentBody as ts.Expression),
+      ],
+      true,
+    );
+  };
+
+  const destructuredProps = new Map<string, () => ts.Expression>();
+  const computedInitializers: ts.VariableStatement[] = [];
+  const usedTempNames = new Set<string>();
+
+  const registerTempName = (base: string): string => {
+    let candidate = `__ct_${base || "prop"}_key`;
+    let counter = 1;
+    while (usedTempNames.has(candidate)) {
+      candidate = `__ct_${base || "prop"}_key_${counter++}`;
+    }
+    usedTempNames.add(candidate);
+    return candidate;
+  };
+
   if (elemName && ts.isObjectBindingPattern(elemName)) {
     for (const element of elemName.elements) {
       if (ts.isBindingElement(element) && ts.isIdentifier(element.name)) {
-        destructuredProps.add(element.name.text);
+        const alias = element.name.text;
+        const propertyName = element.propertyName;
+        usedTempNames.add(alias);
+
+        destructuredProps.set(alias, () => {
+          const target = factory.createIdentifier("element");
+
+          if (!propertyName) {
+            return factory.createPropertyAccessExpression(
+              target,
+              factory.createIdentifier(alias),
+            );
+          }
+
+          if (ts.isIdentifier(propertyName)) {
+            return factory.createPropertyAccessExpression(
+              target,
+              factory.createIdentifier(propertyName.text),
+            );
+          }
+
+          if (
+            ts.isStringLiteral(propertyName) ||
+            ts.isNumericLiteral(propertyName)
+          ) {
+            return factory.createElementAccessExpression(target, propertyName);
+          }
+
+          if (ts.isComputedPropertyName(propertyName)) {
+            const tempName = registerTempName(alias);
+            const tempIdentifier = factory.createIdentifier(tempName);
+
+            computedInitializers.push(
+              factory.createVariableStatement(
+                undefined,
+                factory.createVariableDeclarationList(
+                  [
+                    factory.createVariableDeclaration(
+                      tempIdentifier,
+                      undefined,
+                      undefined,
+                      propertyName.expression,
+                    ),
+                  ],
+                  ts.NodeFlags.Const,
+                ),
+              ),
+            );
+
+            return factory.createElementAccessExpression(
+              target,
+              tempIdentifier,
+            );
+          }
+
+          return factory.createPropertyAccessExpression(
+            target,
+            factory.createIdentifier(alias),
+          );
+        });
       }
     }
   }
 
-  // Collect array destructured identifiers: [date, pizza] -> {date: 0, pizza: 1}
   const arrayDestructuredVars = new Map<string, number>();
   if (elemName && ts.isArrayBindingPattern(elemName)) {
     let index = 0;
@@ -919,35 +1016,28 @@ function transformDestructuredProperties(
     }
   }
 
-  // If param was object-destructured, replace property references with element.prop
   if (destructuredProps.size > 0) {
     const visitor: ts.Visitor = (node) => {
       if (ts.isIdentifier(node) && destructuredProps.has(node.text)) {
-        // Check if this identifier is not part of a property access already
-        // (e.g., don't transform the 'x' in 'something.x')
         if (
           !node.parent ||
           !(ts.isPropertyAccessExpression(node.parent) &&
             node.parent.name === node)
         ) {
-          return factory.createPropertyAccessExpression(
-            factory.createIdentifier("element"),
-            factory.createIdentifier(node.text),
-          );
+          const accessFactory = destructuredProps.get(node.text)!;
+          return accessFactory();
         }
       }
       return visitEachChildWithJsx(node, visitor, undefined);
     };
-    return ts.visitNode(body, visitor) as ts.ConciseBody;
+    transformedBody = ts.visitNode(transformedBody, visitor) as ts.ConciseBody;
   }
 
-  // If param was array-destructured, replace variable references with element[index]
   if (arrayDestructuredVars.size > 0) {
     const visitor: ts.Visitor = (node) => {
       if (ts.isIdentifier(node)) {
         const index = arrayDestructuredVars.get(node.text);
         if (index !== undefined) {
-          // Check if this identifier is not part of a property access already
           if (
             !node.parent ||
             !(ts.isPropertyAccessExpression(node.parent) &&
@@ -962,10 +1052,14 @@ function transformDestructuredProperties(
       }
       return visitEachChildWithJsx(node, visitor, undefined);
     };
-    return ts.visitNode(body, visitor) as ts.ConciseBody;
+    transformedBody = ts.visitNode(transformedBody, visitor) as ts.ConciseBody;
   }
 
-  return body;
+  if (computedInitializers.length > 0) {
+    transformedBody = prependStatements(computedInitializers, transformedBody);
+  }
+
+  return transformedBody;
 }
 
 /**
@@ -1155,12 +1249,32 @@ function transformMapCallback(
   const captureExpressions = collectCaptures(callback, checker);
 
   // Build map of capture name -> expression
-  const captures = new Map<string, ts.Expression>();
+  const captureEntries: Array<{ name: string; expr: ts.Expression }> = [];
+  const usedNames = new Set<string>(["element", "index", "array", "params"]);
+
   for (const expr of captureExpressions) {
-    const name = getCaptureName(expr);
-    if (name && !captures.has(name)) {
-      captures.set(name, expr);
+    const baseName = getCaptureName(expr);
+    if (!baseName) continue;
+
+    // Skip if an existing entry captures an equivalent expression
+    const existing = captureEntries.find((entry) =>
+      expressionsMatch(expr, entry.expr)
+    );
+    if (existing) continue;
+
+    let candidate = baseName;
+    let counter = 2;
+    while (usedNames.has(candidate)) {
+      candidate = `${baseName}_${counter++}`;
     }
+
+    usedNames.add(candidate);
+    captureEntries.push({ name: candidate, expr });
+  }
+
+  const captures = new Map<string, ts.Expression>();
+  for (const entry of captureEntries) {
+    captures.set(entry.name, entry.expr);
   }
 
   // Build set of captured variable names

packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.expected.tsx

@@ -0,0 +1,55 @@
+import * as __ctHelpers from "commontools";
+import { recipe, UI } from "commontools";
+let keyCounter = 0;
+function nextKey() {
+    return `value-${keyCounter++}`;
+}
+interface State {
+    items: Array<Record<string, number>>;
+}
+export default recipe({
+    type: "object",
+    properties: {
+        items: {
+            type: "array",
+            items: {
+                type: "object",
+                properties: {},
+                additionalProperties: {
+                    type: "number"
+                }
+            }
+        }
+    },
+    required: ["items"]
+} as const satisfies __ctHelpers.JSONSchema, (state) => {
+    return {
+        [UI]: (<div>
+        {state.items.mapWithPattern(__ctHelpers.recipe({
+                $schema: "https://json-schema.org/draft/2020-12/schema",
+                type: "object",
+                properties: {
+                    element: {
+                        type: "object",
+                        properties: {},
+                        additionalProperties: {
+                            type: "number"
+                        }
+                    },
+                    params: {
+                        type: "object",
+                        properties: {}
+                    }
+                },
+                required: ["element", "params"]
+            } as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => {
+                const __ct_amount_key = nextKey();
+                return (<span>{__ctHelpers.derive({ element, __ct_amount_key }, ({ element: element, __ct_amount_key: __ct_amount_key }) => element[__ct_amount_key])}</span>);
+            }), {})}
+      </div>),
+    };
+});
+// @ts-ignore: Internals
+function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
+// @ts-ignore: Internals
+h.fragment = __ctHelpers.h.fragment;

packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.input.tsx

@@ -0,0 +1,23 @@
+/// <cts-enable />
+import { recipe, UI } from "commontools";
+
+let keyCounter = 0;
+function nextKey() {
+  return `value-${keyCounter++}`;
+}
+
+interface State {
+  items: Array<Record<string, number>>;
+}
+
+export default recipe<State>("ComputedAliasSideEffect", (state) => {
+  return {
+    [UI]: (
+      <div>
+        {state.items.map(({ [nextKey()]: amount }) => (
+          <span>{amount}</span>
+        ))}
+      </div>
+    ),
+  };
+});

packages/ts-transformers/test/fixtures/closures/map-destructured-alias.expected.tsx

@@ -0,0 +1,64 @@
+import * as __ctHelpers from "commontools";
+import { recipe, UI } from "commontools";
+interface State {
+    items: Array<{
+        price: number;
+    }>;
+    discount: number;
+}
+export default recipe({
+    type: "object",
+    properties: {
+        items: {
+            type: "array",
+            items: {
+                type: "object",
+                properties: {
+                    price: {
+                        type: "number"
+                    }
+                },
+                required: ["price"]
+            }
+        },
+        discount: {
+            type: "number"
+        }
+    },
+    required: ["items", "discount"]
+} as const satisfies __ctHelpers.JSONSchema, (state) => {
+    return {
+        [UI]: (<div>
+        {state.items.mapWithPattern(__ctHelpers.recipe({
+                $schema: "https://json-schema.org/draft/2020-12/schema",
+                type: "object",
+                properties: {
+                    element: {
+                        type: "object",
+                        properties: {
+                            price: {
+                                type: "number"
+                            }
+                        },
+                        required: ["price"]
+                    },
+                    params: {
+                        type: "object",
+                        properties: {
+                            discount: {
+                                type: "number",
+                                asOpaque: true
+                            }
+                        },
+                        required: ["discount"]
+                    }
+                },
+                required: ["element", "params"]
+            } as const satisfies __ctHelpers.JSONSchema, ({ element, params: { discount } }) => (<span>{__ctHelpers.derive({ element_price: element.price, discount }, ({ element_price: _v1, discount: discount }) => _v1 * discount)}</span>)), { discount: state.discount })}
+      </div>),
+    };
+});
+// @ts-ignore: Internals
+function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
+// @ts-ignore: Internals
+h.fragment = __ctHelpers.h.fragment;

packages/ts-transformers/test/fixtures/closures/map-destructured-alias.input.tsx

@@ -0,0 +1,19 @@
+/// <cts-enable />
+import { recipe, UI } from "commontools";
+
+interface State {
+  items: Array<{ price: number }>;
+  discount: number;
+}
+
+export default recipe<State>("MapDestructuredAlias", (state) => {
+  return {
+    [UI]: (
+      <div>
+        {state.items.map(({ price: cost }) => (
+          <span>{cost * state.discount}</span>
+        ))}
+      </div>
+    ),
+  };
+});