diff --git a/packages/ts-transformers/src/closures/transformer.ts b/packages/ts-transformers/src/closures/transformer.ts index e28eaf89d..b3888e52b 100644 --- a/packages/ts-transformers/src/closures/transformer.ts +++ b/packages/ts-transformers/src/closures/transformer.ts @@ -1015,6 +1015,9 @@ function createRecipeCallWithParams( transformedBody, ); + // Mark this as a map callback for later transformers (e.g., OpaqueRefJSXTransformer) + context.markAsMapCallback(newCallback); + // Build a TypeNode for the callback parameter to pass as a type argument to recipe() const callbackParamTypeNode = buildCallbackParamTypeNode( mapCall, diff --git a/packages/ts-transformers/src/core/context.ts b/packages/ts-transformers/src/core/context.ts index 6497485b1..e5c114645 100644 --- a/packages/ts-transformers/src/core/context.ts +++ b/packages/ts-transformers/src/core/context.ts @@ -68,4 +68,21 @@ export class TransformationContext { column: location.character + 1, }); } + + /** + * Mark an arrow function as a map callback created by ClosureTransformer. + * This allows later transformers to identify synthetic map callback scopes. + */ + markAsMapCallback(node: ts.Node): void { + if (this.options.mapCallbackRegistry) { + this.options.mapCallbackRegistry.add(node); + } + } + + /** + * Check if a node is a map callback created by ClosureTransformer. + */ + isMapCallback(node: ts.Node): boolean { + return this.options.mapCallbackRegistry?.has(node) ?? false; + } } diff --git a/packages/ts-transformers/src/core/transformers.ts b/packages/ts-transformers/src/core/transformers.ts index 6fd9169f4..8b1d523d7 100644 --- a/packages/ts-transformers/src/core/transformers.ts +++ b/packages/ts-transformers/src/core/transformers.ts @@ -8,6 +8,7 @@ export interface TransformationOptions { readonly debug?: boolean; readonly logger?: (message: string) => void; readonly typeRegistry?: TypeRegistry; + readonly mapCallbackRegistry?: WeakSet; } export interface TransformationDiagnostic { diff --git a/packages/ts-transformers/src/ct-pipeline.ts b/packages/ts-transformers/src/ct-pipeline.ts index 04dc29558..8238b1f27 100644 --- a/packages/ts-transformers/src/ct-pipeline.ts +++ b/packages/ts-transformers/src/ct-pipeline.ts @@ -10,6 +10,7 @@ export class CommonToolsTransformerPipeline extends Pipeline { constructor(options: TransformationOptions = {}) { const ops = { typeRegistry: new WeakMap(), + mapCallbackRegistry: new WeakSet(), ...options, }; super([ diff --git a/packages/ts-transformers/src/transformers/opaque-ref/helpers.ts b/packages/ts-transformers/src/transformers/opaque-ref/helpers.ts index 195787785..177c2503f 100644 --- a/packages/ts-transformers/src/transformers/opaque-ref/helpers.ts +++ b/packages/ts-transformers/src/transformers/opaque-ref/helpers.ts @@ -115,81 +115,100 @@ export function filterRelevantDataFlows( const syntheticDataFlows = dataFlows.filter((df) => hasSyntheticRoot(df.expression) ); - const nonSyntheticDataFlows = dataFlows.filter((df) => - !hasSyntheticRoot(df.expression) - ); - - // If we have both synthetic and non-synthetic dataflows, we need to determine - // if we're inside or outside the map callback - if (syntheticDataFlows.length > 0 && nonSyntheticDataFlows.length > 0) { - // Check if any dataflow is in a scope with parameters from a map callback - // If so, we're inside a callback being transformed and should keep all dataflows - const isInMapCallbackScope = dataFlows.some((df) => { - const scope = analysis.graph.scopes.find((s) => s.id === df.scopeId); - if (!scope) return false; - // Check if any parameter in this scope comes from a builder or array-map - return scope.parameters.some((param) => { - if (!param.declaration) return false; - const callKind = getOpaqueCallKindForParameter( - param.declaration, - context.checker, - ); - return callKind === "builder" || callKind === "array-map"; - }); + // If we have synthetic dataflows (e.g., element, index, array from map callbacks), + // these are identifiers without symbols that were created by ClosureTransformer. + // We need to determine if they're being used in the correct scope or if they leaked. + if (syntheticDataFlows.length > 0) { + // Check if the synthetic identifiers are standard map callback parameter names + const hasSyntheticMapParams = syntheticDataFlows.some((df) => { + let rootExpr: ts.Expression = df.expression; + while ( + ts.isPropertyAccessExpression(rootExpr) || + ts.isElementAccessExpression(rootExpr) + ) { + rootExpr = rootExpr.expression; + } + if (ts.isIdentifier(rootExpr)) { + const name = rootExpr.text; + // Standard map callback parameter names created by ClosureTransformer + return name === "element" || name === "index" || name === "array"; + } + return false; }); - // If we're inside a map callback scope, keep all dataflows - if (isInMapCallbackScope) { - // Keep all dataflows - we're inside a callback with both synthetic params and captures - return dataFlows.filter((dataFlow) => { - if ( - originatesFromIgnoredParameter( - dataFlow.expression, - dataFlow.scopeId, - analysis, - context.checker, - ) - ) { - return false; - } - return true; - }); - } + if (hasSyntheticMapParams) { + // We have synthetic map callback params. These could be: + // 1. Inside a map callback (keep them) + // 2. In outer scope where they leaked (filter them out) - // Heuristic: Check if the non-synthetic dataflows have symbols in the outer - // scope If they reference outer-scope variables (like cells), we're at the - // outer scope and should filter synthetic params. - const nonSyntheticHaveOuterScopeSymbols = nonSyntheticDataFlows.every( - (df) => { - let rootExpr: ts.Expression = df.expression; - while ( - ts.isPropertyAccessExpression(rootExpr) || - ts.isElementAccessExpression(rootExpr) - ) { - rootExpr = rootExpr.expression; - } - if (ts.isIdentifier(rootExpr)) { - const symbol = context.checker.getSymbolAtLocation(rootExpr); - // If it has a symbol, it's likely from outer scope - return !!symbol; + const nonSyntheticDataFlows = dataFlows.filter((df) => + !hasSyntheticRoot(df.expression) + ); + + // If we have ONLY synthetic dataflows, we're definitely inside a map callback + if (nonSyntheticDataFlows.length === 0) { + // Pure synthetic - we're inside a map callback, keep all + return dataFlows.filter((dataFlow) => { + if ( + originatesFromIgnoredParameter( + dataFlow.expression, + dataFlow.scopeId, + analysis, + context.checker, + ) + ) { + return false; + } + return true; + }); + } + + // We have both synthetic and non-synthetic. This could be: + // 1. Inside a map callback with captures (keep all) + // 2. Outer scope with leaked synthetic params (filter synthetics) + + // Try to find if any dataflow is from a scope with parameters that's a marked callback + const isInMarkedCallback = dataFlows.some((df) => { + const scope = analysis.graph.scopes.find((s) => s.id === df.scopeId); + if (!scope || scope.parameters.length === 0) return false; + + const firstParam = scope.parameters[0]; + if (!firstParam || !firstParam.declaration) return false; + + let node: ts.Node | undefined = firstParam.declaration.parent; + while (node) { + if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) { + return context.isMapCallback(node); + } + node = node.parent; } return false; - }, - ); + }); - // If all non-synthetic dataflows have outer-scope symbols AND we have 2 or - // more of them, we're likely analyzing an outer-scope expression that - // contains nested map callbacks - if ( - nonSyntheticHaveOuterScopeSymbols && nonSyntheticDataFlows.length >= 2 - ) { + if (isInMarkedCallback) { + // Inside a map callback - keep all except ignored params + return dataFlows.filter((dataFlow) => { + if ( + originatesFromIgnoredParameter( + dataFlow.expression, + dataFlow.scopeId, + analysis, + context.checker, + ) + ) { + return false; + } + return true; + }); + } + + // Synthetic map params in outer scope - filter them out return dataFlows.filter((df) => !hasSyntheticRoot(df.expression)); } - - // Otherwise, we're inside a map callback with captures - keep all dataflows } + // No synthetic dataflows, use standard filtering return dataFlows.filter((dataFlow) => { if ( originatesFromIgnoredParameter( diff --git a/packages/ts-transformers/test/fixtures/closures/map-single-capture.expected.tsx b/packages/ts-transformers/test/fixtures/closures/map-single-capture.expected.tsx index ca27b444a..1bdc9609d 100644 --- a/packages/ts-transformers/test/fixtures/closures/map-single-capture.expected.tsx +++ b/packages/ts-transformers/test/fixtures/closures/map-single-capture.expected.tsx @@ -61,4 +61,4 @@ export default recipe({ // @ts-ignore: Internals function h(...args: any[]) { return __ctHelpers.h.apply(null, args); } // @ts-ignore: Internals -h.fragment = __ctHelpers.h.fragment; \ No newline at end of file +h.fragment = __ctHelpers.h.fragment; diff --git a/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.expected.tsx b/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.expected.tsx new file mode 100644 index 000000000..652f429fb --- /dev/null +++ b/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.expected.tsx @@ -0,0 +1,46 @@ +import * as __ctHelpers from "commontools"; +import { cell, recipe, UI } from "commontools"; +export default recipe("MapSingleCapture", (_state) => { + const people = cell([ + { id: "1", name: "Alice" }, + { id: "2", name: "Bob" }, + ]); + return { + [UI]: (
+ {__ctHelpers.derive(people, people => people.length > 0 && (
    + {people.mapWithPattern(__ctHelpers.recipe({ + $schema: "https://json-schema.org/draft/2020-12/schema", + type: "object", + properties: { + element: { + $ref: "#/$defs/__object" + }, + params: { + type: "object", + properties: {} + } + }, + required: ["element", "params"], + $defs: { + __object: { + type: "object", + properties: { + id: { + type: "string" + }, + name: { + type: "string" + } + }, + required: ["id", "name"] + } + } + } as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (
  • {element.name}
    • )), {})} +
))} +
), + }; +}); +// @ts-ignore: Internals +function h(...args: any[]) { return __ctHelpers.h.apply(null, args); } +// @ts-ignore: Internals +h.fragment = __ctHelpers.h.fragment; diff --git a/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.input.tsx b/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.input.tsx new file mode 100644 index 000000000..3b378144d --- /dev/null +++ b/packages/ts-transformers/test/fixtures/jsx-expressions/map-single-capture.input.tsx @@ -0,0 +1,23 @@ +/// +import { cell, recipe, UI } from "commontools"; + +export default recipe("MapSingleCapture", (_state) => { + const people = cell([ + { id: "1", name: "Alice" }, + { id: "2", name: "Bob" }, + ]); + + return { + [UI]: ( +
+ {people.length > 0 && ( +
    + {people.map((person) => ( +
  • {person.name}
    • + ))} +
+ )} +
+ ), + }; +});