actually fix name collisions in derive closures (commontoolsinc#2044)

* actually fix name collisions in derive closures

* fix shorthand property assignments; also remove $schema everywhere

* fix: update test expectations after rebase

- Remove $schema from all test fixtures (main already had this change)
- Update asOpaque to asCell for cell types (API change in main)
- Fix derive-nested-callback to show proper array schema (typeRegistry fix working correctly)

Author: mathpirate

packages/schema-generator/src/schema-generator.ts

@@ -400,7 +400,6 @@ export class SchemaGenerator implements ISchemaGenerator {
     // final output
     const filtered = this.collectReferencedDefinitions(base, definitions);
     const out: Record<string, unknown> = {
-      $schema: "https://json-schema.org/draft/2020-12/schema",
       ...(base as Record<string, unknown>),
     };
     if (Object.keys(filtered).length > 0) out.$defs = filtered;
@@ -701,18 +700,14 @@ export class SchemaGenerator implements ISchemaGenerator {
       return rootSchema;
     }
 
-    // If no definitions were created or used, return simple schema with $schema
+    // If no definitions were created or used, return simple schema
     if (Object.keys(definitions).length === 0 || emittedRefs.size === 0) {
-      return {
-        $schema: "https://json-schema.org/draft/2020-12/schema",
-        ...(rootSchema as Record<string, unknown>),
-      } as SchemaDefinition;
+      return rootSchema;
     }
 
     // Object schema: attach only the definitions actually referenced
     const filtered = this.collectReferencedDefinitions(rootSchema, definitions);
     const out: Record<string, unknown> = {
-      $schema: "https://json-schema.org/draft/2020-12/schema",
       ...(rootSchema as Record<string, unknown>),
     };
     if (Object.keys(filtered).length > 0) out.$defs = filtered;

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

@@ -1441,6 +1441,39 @@ function extractDeriveCallback(
   return undefined;
 }
 
+/**
+ * Resolve capture name collisions with the original input parameter name.
+ * If a capture has the same name as originalInputParamName, rename it (e.g., multiplier -> multiplier_1).
+ * Returns a mapping from original capture names to their potentially renamed versions.
+ */
+function resolveDeriveCaptureNameCollisions(
+  originalInputParamName: string,
+  captureTree: Map<string, CaptureTreeNode>,
+): Map<string, string> {
+  const captureNameMap = new Map<string, string>();
+  const usedNames = new Set<string>([originalInputParamName]);
+
+  for (const [captureName] of captureTree) {
+    if (captureName === originalInputParamName) {
+      // Collision detected - rename the capture
+      let renamed = `${captureName}_1`;
+      let suffix = 1;
+      while (usedNames.has(renamed) || captureTree.has(renamed)) {
+        suffix++;
+        renamed = `${captureName}_${suffix}`;
+      }
+      captureNameMap.set(captureName, renamed);
+      usedNames.add(renamed);
+    } else {
+      // No collision - use original name
+      captureNameMap.set(captureName, captureName);
+      usedNames.add(captureName);
+    }
+  }
+
+  return captureNameMap;
+}
+
 /**
  * Build the merged input object containing both the original input and captures.
  * Example: {value, multiplier} where value is the original input and multiplier is a capture.
@@ -1452,6 +1485,7 @@ function buildDeriveInputObject(
   originalInput: ts.Expression,
   originalInputParamName: string,
   captureTree: Map<string, CaptureTreeNode>,
+  captureNameMap: Map<string, string>,
   factory: ts.NodeFactory,
   hadZeroParameters: boolean,
 ): ts.ObjectLiteralExpression {
@@ -1478,15 +1512,90 @@ function buildDeriveInputObject(
     }
   }
 
-  // Add captures
-  properties.push(...buildCapturePropertyAssignments(captureTree, factory));
+  // Add captures with potentially renamed property names
+  for (const [originalName, node] of captureTree) {
+    const propertyName = captureNameMap.get(originalName) ?? originalName;
+    properties.push(
+      factory.createPropertyAssignment(
+        createPropertyName(propertyName, factory),
+        buildHierarchicalParamsValue(node, originalName, factory),
+      ),
+    );
+  }
 
   return factory.createObjectLiteralExpression(
     properties,
     properties.length > 1,
   );
 }
 
+/**
+ * Rewrite the callback body to use renamed capture identifiers.
+ * For example, if `multiplier` was renamed to `multiplier_1`, replace all
+ * references to the captured `multiplier` with `multiplier_1`.
+ */
+function rewriteCaptureReferences(
+  body: ts.ConciseBody,
+  captureNameMap: Map<string, string>,
+  factory: ts.NodeFactory,
+): ts.ConciseBody {
+  // Build a reverse map: original capture name -> list of renamed names that should be substituted
+  const substitutions = new Map<string, string>();
+  for (const [originalName, renamedName] of captureNameMap) {
+    if (originalName !== renamedName) {
+      substitutions.set(originalName, renamedName);
+    }
+  }
+
+  if (substitutions.size === 0) {
+    return body; // No substitutions needed
+  }
+
+  const visitor = (node: ts.Node, parent?: ts.Node): ts.Node => {
+    // Handle shorthand property assignments specially
+    // { multiplier } needs to become { multiplier: multiplier_1 } if multiplier is renamed
+    if (ts.isShorthandPropertyAssignment(node)) {
+      const substituteName = substitutions.get(node.name.text);
+      if (substituteName) {
+        // Expand shorthand into full property assignment
+        return factory.createPropertyAssignment(
+          node.name, // Property name stays the same
+          factory.createIdentifier(substituteName), // Value uses renamed identifier
+        );
+      }
+      // No substitution needed, keep as shorthand
+      return node;
+    }
+
+    // Don't substitute identifiers that are property names
+    if (ts.isIdentifier(node)) {
+      // Skip if this identifier is the property name in a property access (e.g., '.get' in 'obj.get')
+      if (
+        parent && ts.isPropertyAccessExpression(parent) && parent.name === node
+      ) {
+        return node;
+      }
+
+      // Skip if this identifier is a property name in an object literal (e.g., 'foo' in '{ foo: value }')
+      if (parent && ts.isPropertyAssignment(parent) && parent.name === node) {
+        return node;
+      }
+
+      const substituteName = substitutions.get(node.text);
+      if (substituteName) {
+        return factory.createIdentifier(substituteName);
+      }
+    }
+
+    return ts.visitEachChild(node, (child) => visitor(child, node), undefined);
+  };
+
+  return ts.visitNode(
+    body,
+    (node) => visitor(node, undefined),
+  ) as ts.ConciseBody;
+}
+
 /**
  * Create the derive callback with parameter aliasing to preserve user's parameter name.
  * Example: ({value: v, multiplier}) => v * multiplier
@@ -1499,6 +1608,7 @@ function createDeriveCallback(
   transformedBody: ts.ConciseBody,
   originalInputParamName: string,
   captureTree: Map<string, CaptureTreeNode>,
+  captureNameMap: Map<string, string>,
   context: TransformationContext,
   hadZeroParameters: boolean,
 ): ts.ArrowFunction | ts.FunctionExpression {
@@ -1590,14 +1700,19 @@ function createDeriveCallback(
     ),
   );
 
-  // Add bindings for captures
+  // Add bindings for captures using the potentially renamed property names
   const createBindingIdentifier = (name: string): ts.Identifier => {
     return reserveIdentifier(name, usedBindingNames, factory);
   };
 
+  // Create binding elements using the renamed capture names
+  const renamedCaptureNames = Array.from(captureTree.keys()).map(
+    (originalName) => captureNameMap.get(originalName) ?? originalName,
+  );
+
   bindingElements.push(
     ...createBindingElementsFromNames(
-      captureTree.keys(),
+      renamedCaptureNames,
       factory,
       createBindingIdentifier,
     ),
@@ -1613,6 +1728,13 @@ function createDeriveCallback(
     undefined,
   );
 
+  // Rewrite the body to use renamed capture identifiers
+  const rewrittenBody = rewriteCaptureReferences(
+    transformedBody,
+    captureNameMap,
+    factory,
+  );
+
   // Create the new callback
   if (ts.isArrowFunction(callback)) {
     return factory.createArrowFunction(
@@ -1621,7 +1743,7 @@ function createDeriveCallback(
       [parameter],
       undefined, // No return type - rely on inference
       callback.equalsGreaterThanToken,
-      transformedBody,
+      rewrittenBody,
     );
   } else {
     return factory.createFunctionExpression(
@@ -1631,7 +1753,7 @@ function createDeriveCallback(
       callback.typeParameters,
       [parameter],
       undefined, // No return type - rely on inference
-      transformedBody as ts.Block,
+      rewrittenBody as ts.Block,
     );
   }
 }
@@ -1646,6 +1768,7 @@ function buildDeriveInputSchema(
   originalInputParamName: string,
   originalInput: ts.Expression,
   captureTree: Map<string, CaptureTreeNode>,
+  captureNameMap: Map<string, string>,
   context: TransformationContext,
   hadZeroParameters: boolean,
 ): ts.TypeNode {
@@ -1677,11 +1800,40 @@ function buildDeriveInputSchema(
     );
   }
 
-  // Add type elements for captures
-  typeElements.push(
-    ...buildTypeElementsFromCaptureTree(captureTree, context),
+  // Add type elements for captures using the existing helper
+  const captureTypeElements = buildTypeElementsFromCaptureTree(
+    captureTree,
+    context,
   );
 
+  // Rename the property signatures if there are collisions
+  for (const typeElement of captureTypeElements) {
+    if (
+      ts.isPropertySignature(typeElement) && ts.isIdentifier(typeElement.name)
+    ) {
+      const originalName = typeElement.name.text;
+      const renamedName = captureNameMap.get(originalName) ?? originalName;
+
+      if (renamedName !== originalName) {
+        // Create a new property signature with the renamed identifier
+        typeElements.push(
+          factory.createPropertySignature(
+            typeElement.modifiers,
+            factory.createIdentifier(renamedName),
+            typeElement.questionToken,
+            typeElement.type,
+          ),
+        );
+      } else {
+        // No renaming needed
+        typeElements.push(typeElement);
+      }
+    } else {
+      // Not a simple property signature, keep as-is
+      typeElements.push(typeElement);
+    }
+  }
+
   // Create object type literal
   return factory.createTypeLiteralNode(typeElements);
 }
@@ -1757,11 +1909,18 @@ function transformDeriveCall(
   // In this case, we don't need to preserve the input - just use captures
   const hadZeroParameters = callback.parameters.length === 0;
 
+  // Resolve capture name collisions with the original input parameter name
+  const captureNameMap = resolveDeriveCaptureNameCollisions(
+    originalInputParamName,
+    captureTree,
+  );
+
   // Build merged input object
   const mergedInput = buildDeriveInputObject(
     originalInput,
     originalInputParamName,
     captureTree,
+    captureNameMap,
     factory,
     hadZeroParameters,
   );
@@ -1772,6 +1931,7 @@ function transformDeriveCall(
     transformedBody,
     originalInputParamName,
     captureTree,
+    captureNameMap,
     context,
     hadZeroParameters,
   );
@@ -1782,6 +1942,7 @@ function transformDeriveCall(
     originalInputParamName,
     originalInput,
     captureTree,
+    captureNameMap,
     context,
     hadZeroParameters,
   );
@@ -1790,19 +1951,26 @@ function transformDeriveCall(
   // SchemaInjectionTransformer will use this to generate the result schema
   const signature = context.checker.getSignatureFromDeclaration(callback);
   let resultTypeNode: ts.TypeNode | undefined;
+  let resultType: ts.Type | undefined;
 
   if (callback.type) {
-    // Explicit return type annotation - use it
+    // Explicit return type annotation - use it directly (no need to register in typeRegistry)
     resultTypeNode = callback.type;
   } else if (signature) {
     // Infer from callback signature
-    const returnType = signature.getReturnType();
+    resultType = signature.getReturnType();
     resultTypeNode = context.checker.typeToTypeNode(
-      returnType,
+      resultType,
       context.sourceFile,
       ts.NodeBuilderFlags.NoTruncation |
         ts.NodeBuilderFlags.UseStructuralFallback,
     );
+
+    // Register the result Type in typeRegistry for the synthetic TypeNode
+    // This fixes schema generation for shorthand properties referencing captured variables
+    if (resultTypeNode && context.options.typeRegistry) {
+      context.options.typeRegistry.set(resultTypeNode, resultType);
+    }
   }
 
   // Build the derive call expression

packages/ts-transformers/src/transformers/schema-injection.ts

@@ -345,11 +345,25 @@ export class SchemaInjectionTransformer extends Transformer {
             return ts.visitEachChild(node, visit, transformation);
           }
 
+          // Check if ClosureTransformer registered Types for these TypeNodes
+          // This preserves type information for shorthand properties with captured variables
+          let argumentTypeValue: ts.Type | undefined;
+          let resultTypeValue: ts.Type | undefined;
+
+          if (typeRegistry) {
+            if (typeRegistry.has(argumentType)) {
+              argumentTypeValue = typeRegistry.get(argumentType);
+            }
+            if (typeRegistry.has(resultType)) {
+              resultTypeValue = typeRegistry.get(resultType);
+            }
+          }
+
           return updateWithSchemas(
             argumentType,
-            undefined,
+            argumentTypeValue,
             resultType,
-            undefined,
+            resultTypeValue,
           );
         }
 

packages/ts-transformers/test/fixtures/ast-transform/recipe-array-map.expected.tsx

@@ -43,7 +43,6 @@ export default recipe({
           <button type="button" onClick={adder({ values })}>Add Value</button>
           <div>
             {values.mapWithPattern(__ctHelpers.recipe({
-                $schema: "https://json-schema.org/draft/2020-12/schema",
                 type: "object",
                 properties: {
                     element: {

packages/ts-transformers/test/fixtures/ast-transform/schema-generation-builders.expected.tsx

@@ -47,7 +47,6 @@ export default recipe({
         </button>
         <ul>
           {state.items.mapWithPattern(__ctHelpers.recipe({
-            $schema: "https://json-schema.org/draft/2020-12/schema",
             type: "object",
             properties: {
                 element: {