From 953c788a1295cb630155f649d2e320ba8c8c3875 Mon Sep 17 00:00:00 2001 From: Tony Espinoza <86493411+tonyespinoza1@users.noreply.github.com> Date: Wed, 3 Dec 2025 08:32:59 -0800 Subject: [PATCH 1/6] test: Add cross-space wish resolution test coverage (#2192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: Add cross-space wish resolution test coverage Add comprehensive test coverage for wish tag resolution across space boundaries to verify correct behavior when patterns access data from different spaces. Tests verify: - Home space tags (#favorites, #hashtag) correctly access user's home space even when pattern runs in a different space - Pattern space tags (/, #default, etc.) correctly access the pattern's space, not home space - Mixed access patterns work correctly in a single pattern - Proper space isolation between home and pattern spaces Related to CT-1090 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix: Format test file with deno fmt 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * test: Add cross-space charm auto-start test Adds test to verify that when a wish in one space references a charm in another space (via favorites/hashtags), the charm is properly accessed and its data is available. This addresses the CT-1090 comment about cross-space charms needing to be accessible when accessed via wish. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Tony Espinoza Co-authored-by: Claude --- packages/runner/test/wish.test.ts | 308 ++++++++++++++++++++++++++++++ 1 file changed, 308 insertions(+) diff --git a/packages/runner/test/wish.test.ts b/packages/runner/test/wish.test.ts index f09e8dccbd..26b2359942 100644 --- a/packages/runner/test/wish.test.ts +++ b/packages/runner/test/wish.test.ts @@ -837,4 +837,312 @@ describe("wish built-in", () => { expect(result.key("deepValue").get()?.result).toEqual("found it"); }); }); + + describe("cross-space wish resolution", () => { + let userIdentity: Identity; + let patternSpace: Identity; + let storageManager: ReturnType; + let runtime: Runtime; + let tx: ReturnType; + let wish: ReturnType["commontools"]["wish"]; + let recipe: ReturnType["commontools"]["recipe"]; + + beforeEach(async () => { + userIdentity = await Identity.fromPassphrase("user-home-space"); + patternSpace = await Identity.fromPassphrase("pattern-space-1"); + + // Key: storageManager.as determines home space (user identity) + storageManager = StorageManager.emulate({ as: userIdentity }); + runtime = new Runtime({ + apiUrl: new URL("https://example.com"), + storageManager, + }); + tx = runtime.edit(); + + const { commontools } = createBuilder(); + ({ wish, recipe } = commontools); + }); + + afterEach(async () => { + await tx.commit(); + await runtime.dispose(); + await storageManager.close(); + }); + + it("resolves #favorites from home space when pattern runs in different space", async () => { + // Setup: Add favorites to home space + const homeSpaceCell = runtime.getHomeSpaceCell(tx); + const favoritesCell = homeSpaceCell.key("favorites"); + const favoriteItem = runtime.getCell( + userIdentity.did(), + "favorite-item", + undefined, + tx, + ); + favoriteItem.set({ name: "My Favorite", value: 42 }); + favoritesCell.set([ + { cell: favoriteItem, tag: "test favorite" }, + ]); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Execute: Run pattern in different space (patternSpace) + const wishRecipe = recipe("wish favorites cross-space", () => { + return { favorites: wish({ query: "#favorites" }) }; + }); + + const resultCell = runtime.getCell<{ + favorites?: { result?: unknown[] }; + }>( + patternSpace.did(), // Pattern runs in different space + "wish-favorites-result", + undefined, + tx, + ); + const result = runtime.run(tx, wishRecipe, {}, resultCell); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Verify: Favorites resolved from home space, not pattern space + const favorites = result.key("favorites").get()?.result; + expect(favorites).toBeDefined(); + expect(Array.isArray(favorites)).toBe(true); + expect((favorites as any[])[0].tag).toEqual("test favorite"); + }); + + it("resolves #default from pattern space, not home space", async () => { + // Setup: Add different #default data to home space + const homeSpaceCell = runtime.getCell( + userIdentity.did(), + userIdentity.did(), + ).withTx(tx); + const homeDefaultCell = runtime.getCell( + userIdentity.did(), + "home-default", + ).withTx(tx); + homeDefaultCell.set({ title: "Home Default", value: "home" }); + homeSpaceCell.key("defaultPattern").set(homeDefaultCell); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Setup: Add different #default data to pattern space + const patternSpaceCell = runtime.getCell( + patternSpace.did(), + patternSpace.did(), + ).withTx(tx); + const patternDefaultCell = runtime.getCell( + patternSpace.did(), + "pattern-default", + ).withTx(tx); + patternDefaultCell.set({ title: "Pattern Default", value: "pattern" }); + patternSpaceCell.key("defaultPattern").set(patternDefaultCell); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Execute: Run pattern in pattern space that wishes for #default + const wishRecipe = recipe("wish default cross-space", () => { + return { defaultData: wish({ query: "#default" }) }; + }); + + const resultCell = runtime.getCell<{ + defaultData?: { result?: unknown }; + }>( + patternSpace.did(), + "wish-default-result", + undefined, + tx, + ); + const result = runtime.run(tx, wishRecipe, {}, resultCell); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Verify: Gets pattern space's #default, not home space's + const defaultData = result.key("defaultData").get()?.result; + expect(defaultData).toEqual({ + title: "Pattern Default", + value: "pattern", + }); + }); + + it("resolves mixed tags (#favorites from home, / from pattern) in single pattern", async () => { + // Setup: Favorites in home space + const homeSpaceCell = runtime.getHomeSpaceCell(tx); + const favoritesCell = homeSpaceCell.key("favorites"); + const favoriteItem = runtime.getCell( + userIdentity.did(), + "favorite-mixed", + undefined, + tx, + ); + favoriteItem.set({ type: "favorite" }); + favoritesCell.set([{ cell: favoriteItem, tag: "mixed test" }]); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Setup: /data in pattern space + const patternSpaceCell = runtime.getCell( + patternSpace.did(), + patternSpace.did(), + ).withTx(tx); + patternSpaceCell.set({ data: { type: "pattern" } }); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Execute: Single pattern wishes for both + const wishRecipe = recipe("wish mixed tags", () => { + return { + favorites: wish({ query: "#favorites" }), + patternData: wish({ query: "/", path: ["data"] }), + }; + }); + + const resultCell = runtime.getCell<{ + favorites?: { result?: unknown[] }; + patternData?: { result?: unknown }; + }>( + patternSpace.did(), + "wish-mixed-result", + undefined, + tx, + ); + const result = runtime.run(tx, wishRecipe, {}, resultCell); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Verify: Each resolves to correct space + const favorites = result.key("favorites").get()?.result; + const patternData = result.key("patternData").get()?.result; + + expect(Array.isArray(favorites)).toBe(true); + expect((favorites as any[])[0].tag).toEqual("mixed test"); + expect(patternData).toEqual({ type: "pattern" }); + }); + + it("resolves hashtag search in home space favorites from different pattern space", async () => { + // Setup: Favorites with tags in home space + const homeSpaceCell = runtime.getHomeSpaceCell(tx); + const favoritesCell = homeSpaceCell.key("favorites"); + const favoriteItem1 = runtime.getCell( + userIdentity.did(), + "hashtag-item-1", + undefined, + tx, + ); + favoriteItem1.set({ name: "Item with #myTag" }); + const favoriteItem2 = runtime.getCell( + userIdentity.did(), + "hashtag-item-2", + undefined, + tx, + ); + favoriteItem2.set({ name: "Different item" }); + + favoritesCell.set([ + { cell: favoriteItem1, tag: "#myTag #awesome" }, + { cell: favoriteItem2, tag: "no hashtag here" }, + ]); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Execute: Pattern in different space wishes for #myTag + const wishRecipe = recipe("wish hashtag search", () => { + return { taggedItem: wish({ query: "#myTag" }) }; + }); + + const resultCell = runtime.getCell<{ + taggedItem?: { result?: unknown }; + }>( + patternSpace.did(), + "wish-hashtag-result", + undefined, + tx, + ); + const result = runtime.run(tx, wishRecipe, {}, resultCell); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Verify: Searches home space favorites, finds correct cell + const taggedItem = result.key("taggedItem").get()?.result; + expect(taggedItem).toEqual({ name: "Item with #myTag" }); + }); + + it("starts charm automatically when accessed via cross-space wish", async () => { + // Setup 1: Create a simple counter recipe/charm + const counterRecipe = recipe<{ count: number }>("counter charm", () => { + const count = 0; + return { + count, + increment: () => { + return count + 1; + }, + }; + }); + + // Setup 2: Store the charm in home space + const charmCell = runtime.getCell( + userIdentity.did(), + "counter-charm", + undefined, + tx, + ); + // Setup the charm (but don't start it yet) + runtime.setup(tx, counterRecipe, {}, charmCell); + + // Setup 3: Add charm to favorites + const homeSpaceCell = runtime.getHomeSpaceCell(tx); + const favoritesCell = homeSpaceCell.key("favorites"); + favoritesCell.set([ + { cell: charmCell, tag: "#counterCharm test charm" }, + ]); + + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Execute: Pattern in different space wishes for the charm via hashtag + const wishingRecipe = recipe("wish for charm", () => { + return { charmData: wish({ query: "#counterCharm" }) }; + }); + + const resultCell = runtime.getCell<{ + charmData?: { result?: unknown }; + }>( + patternSpace.did(), + "wish-charm-result", + undefined, + tx, + ); + const result = runtime.run(tx, wishingRecipe, {}, resultCell); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + // Verify: Wish triggered charm to start and returns running charm data + const charmData = result.key("charmData").get()?.result; + expect(charmData).toBeDefined(); + expect(typeof charmData).toBe("object"); + + // The charm should be running and have its state accessible + // Note: This test may need adjustment based on actual charm startup behavior + if (typeof charmData === "object" && charmData !== null) { + expect("count" in charmData || "increment" in charmData).toBe(true); + } + }); + }); }); From 9099de27cbc2ba6b06c99967757e65ba51f72ae9 Mon Sep 17 00:00:00 2001 From: jakedahn Date: Wed, 3 Dec 2025 10:55:37 -0700 Subject: [PATCH 2/6] Remove Sentry error tracking usage --- .github/workflows/deno.yml | 21 --------------------- .github/workflows/deploy-production.yml | 21 --------------------- packages/shell/src/lib/runtime.ts | 1 - packages/toolshed/env.ts | 6 ------ packages/toolshed/index.ts | 8 -------- packages/toolshed/lib/create-app.ts | 3 --- 6 files changed, 60 deletions(-) diff --git a/.github/workflows/deno.yml b/.github/workflows/deno.yml index b9f9882844..2a4aa0bef2 100644 --- a/.github/workflows/deno.yml +++ b/.github/workflows/deno.yml @@ -391,27 +391,6 @@ jobs: with: cache: false - - name: 🔽 Pre-download Sentry CLI - run: | - echo "::group::Downloading Sentry CLI" - deno run --allow-all npm:@sentry/cli --version - echo "::endgroup::" - - - name: 📊 Create Toolshed server Sentry release - run: | - # Create a release with version based on commit SHA - deno run --allow-all npm:@sentry/cli releases new ${{ github.sha }} - - # Associate commits with the release - deno run --allow-all npm:@sentry/cli releases set-commits ${{ github.sha }} --auto - - # Finalize the release - deno run --allow-all npm:@sentry/cli releases finalize ${{ github.sha }} - env: - SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} - SENTRY_ORG: ${{ vars.SENTRY_ORG }} - SENTRY_PROJECT: ${{ vars.SENTRY_TOOLSHED_PROJECT }} - - name: 🚀 Deploy application to Toolshed (Staging) uses: appleboy/ssh-action@master with: diff --git a/.github/workflows/deploy-production.yml b/.github/workflows/deploy-production.yml index fda3c5f3ba..d0a9aa6221 100644 --- a/.github/workflows/deploy-production.yml +++ b/.github/workflows/deploy-production.yml @@ -51,27 +51,6 @@ jobs: with: cache: false - - name: 🔽 Pre-download Sentry CLI - run: | - echo "::group::Downloading Sentry CLI" - deno run --allow-all npm:@sentry/cli --version - echo "::endgroup::" - - - name: 📊 Create Toolshed server Sentry release - run: | - # Create a release with version based on commit SHA - deno run --allow-all npm:@sentry/cli releases new ${{ github.event.inputs.commit_sha }} - - # Associate commits with the release - deno run --allow-all npm:@sentry/cli releases set-commits ${{ github.event.inputs.commit_sha }} --auto - - # Finalize the release - deno run --allow-all npm:@sentry/cli releases finalize ${{ github.event.inputs.commit_sha }} - env: - SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} - SENTRY_ORG: ${{ vars.SENTRY_ORG }} - SENTRY_PROJECT: ${{ vars.SENTRY_TOOLSHED_PROJECT }} - - name: 🚀 Deploy application to Estuary (Production) id: deployment uses: appleboy/ssh-action@master diff --git a/packages/shell/src/lib/runtime.ts b/packages/shell/src/lib/runtime.ts index e0bfefdf20..0a9b8a89c1 100644 --- a/packages/shell/src/lib/runtime.ts +++ b/packages/shell/src/lib/runtime.ts @@ -146,7 +146,6 @@ export class RuntimeInternals extends EventTarget { }), errorHandlers: [(error) => { console.error(error); - //Sentry.captureException(error); }], telemetry, consoleHandler: (metadata, method, args) => { diff --git a/packages/toolshed/env.ts b/packages/toolshed/env.ts index 7040d2113e..66b9797d76 100644 --- a/packages/toolshed/env.ts +++ b/packages/toolshed/env.ts @@ -88,12 +88,6 @@ const EnvSchema = z.object({ { message: "DB_PATH must be an absolute path" }, ).optional(), MEMORY_URL: z.string().default("http://localhost:8000"), - // =========================================================================== - // Sentry DSN global middleware - // * /lib/create-app.ts - // =========================================================================== - SENTRY_DSN: z.string().default(""), - // =========================================================================== GOOGLE_CLIENT_ID: z.string().default(""), GOOGLE_CLIENT_SECRET: z.string().default(""), diff --git a/packages/toolshed/index.ts b/packages/toolshed/index.ts index 897a0c3025..a5a9994dd1 100644 --- a/packages/toolshed/index.ts +++ b/packages/toolshed/index.ts @@ -1,6 +1,5 @@ import app from "@/app.ts"; import env from "@/env.ts"; -import * as Sentry from "@sentry/deno"; import { identity } from "@/lib/identity.ts"; import { Runtime } from "@commontools/runner"; import { StorageManager } from "@commontools/runner/storage/cache.deno"; @@ -86,19 +85,12 @@ function startServer() { console.log(`Server is starting on port http://${env.HOST}:${env.PORT}`); initializeRuntime(); - Sentry.init({ - dsn: env.SENTRY_DSN, - tracesSampleRate: 1.0, - environment: env.ENV || "development", - }); - const serverOptions = { hostname: env.HOST, port: env.PORT, signal: ac.signal, onError: (error: unknown) => { console.error("Server error:", error); - Sentry.captureException(error); return new Response("Internal Server Error", { status: 500 }); }, onListen: ({ port, hostname }: { port: number; hostname: string }) => { diff --git a/packages/toolshed/lib/create-app.ts b/packages/toolshed/lib/create-app.ts index 059ef09490..92ae8bc41c 100644 --- a/packages/toolshed/lib/create-app.ts +++ b/packages/toolshed/lib/create-app.ts @@ -3,7 +3,6 @@ import { notFound, serveEmojiFavicon } from "stoker/middlewares"; import { defaultHook } from "stoker/openapi"; import { pinoLogger } from "@/middlewares/pino-logger.ts"; import { otelTracing } from "@/middlewares/opentelemetry.ts"; -import { sentry } from "@hono/sentry"; import env from "@/env.ts"; import type { AppBindings, AppOpenAPI } from "@/lib/types.ts"; import { initOpenTelemetry } from "@/lib/otel.ts"; @@ -21,8 +20,6 @@ export default function createApp() { const app = createRouter(); - app.use("*", sentry({ dsn: env.SENTRY_DSN })); - // Add OpenTelemetry tracing if enabled if (env.OTEL_ENABLED) { app.use( From 463978d85b212d363b49559657aac9bdbf509f0b Mon Sep 17 00:00:00 2001 From: jakedahn Date: Wed, 3 Dec 2025 11:45:56 -0700 Subject: [PATCH 3/6] whitespace --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d449f04183..d47b3dbd3c 100644 --- a/README.md +++ b/README.md @@ -124,3 +124,4 @@ backend running in the cloud, by running the following command: ```shell API_URL=https://toolshed.saga-castor.ts.net/ deno task dev ``` + From 81d97528fcda5e77820463117b7e222daa4ef81f Mon Sep 17 00:00:00 2001 From: jakedahn Date: Wed, 3 Dec 2025 11:48:06 -0700 Subject: [PATCH 4/6] Revert "whitespace" This reverts commit 463978d85b212d363b49559657aac9bdbf509f0b. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index d47b3dbd3c..d449f04183 100644 --- a/README.md +++ b/README.md @@ -124,4 +124,3 @@ backend running in the cloud, by running the following command: ```shell API_URL=https://toolshed.saga-castor.ts.net/ deno task dev ``` - From 3e1d2ea91c0b49e459397374660ba96b065cd221 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Wed, 3 Dec 2025 12:09:24 -0800 Subject: [PATCH 5/6] feat: Run tests in parallel when executing via tasks, speeding up total execution (#2194) --- packages/utils/src/encoding.ts | 3 +- tasks/check.sh | 1 + tasks/test.ts | 57 ++++++++++++++++++++-------------- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/packages/utils/src/encoding.ts b/packages/utils/src/encoding.ts index 37930e5b6a..bd31aef16a 100644 --- a/packages/utils/src/encoding.ts +++ b/packages/utils/src/encoding.ts @@ -6,7 +6,8 @@ const decoder = new TextDecoder(); * @param input - The string to encode * @returns The encoded Uint8Array */ -export const encode = (input: string): Uint8Array => encoder.encode(input); +export const encode = (input: string): Uint8Array => + encoder.encode(input); /** * Decodes a Uint8Array into a string using UTF-8 decoding. diff --git a/tasks/check.sh b/tasks/check.sh index f0cfbf7a6e..31e0da5ca0 100755 --- a/tasks/check.sh +++ b/tasks/check.sh @@ -9,6 +9,7 @@ if [[ ! " ${DENO_VERSIONS_ALLOWED[@]} " =~ " ${DENO_VERSION} " ]]; then exit 1 fi +deno check tasks/*.ts deno check recipes/[!_]*.ts* deno check \ packages/api \ diff --git a/tasks/test.ts b/tasks/test.ts index e583e5ab36..b2cdf040e5 100755 --- a/tasks/test.ts +++ b/tasks/test.ts @@ -1,29 +1,36 @@ #!/usr/bin/env -S deno run --allow-read --allow-run import * as path from "@std/path"; - -const decoder = new TextDecoder(); +import { decode, encode } from "@commontools/utils/encoding"; export const ALL_DISABLED = [ "background-charm-service", // no tests yet "vendor-astral", // no tests yet ]; -export async function testPackage(packagePath: string): Promise { - const result = await new Deno.Command(Deno.execPath(), { - args: ["task", "test"], - cwd: packagePath, - stdout: "piped", - }).output(); - - const stdout = decoder.decode(result.stdout); - if (stdout) { - console.log(stdout); - } - const stderr = decoder.decode(result.stderr); - if (stderr) { - console.error(stderr); +export async function testPackage( + packagePath: string, +): Promise<{ packagePath: string; result: Deno.CommandOutput }> { + try { + return { + packagePath, + result: await new Deno.Command(Deno.execPath(), { + args: ["task", "test"], + cwd: packagePath, + stdout: "piped", + }).output(), + }; + } catch (e) { + return { + packagePath, + result: { + success: false, + stdout: new Uint8Array(), + stderr: encode(`${e}`), + code: 1, + signal: null, + }, + }; } - return result.success; } export async function runTests(disabledPackages: string[]): Promise { @@ -31,8 +38,7 @@ export async function runTests(disabledPackages: string[]): Promise { const manifest = JSON.parse(await Deno.readTextFile("./deno.json")); const members: string[] = manifest.workspace; - const failedPackages: string[] = []; - + const tests = []; for (const memberPath of members) { // Convert "./packages/memory" to "memory" const packageName = memberPath.substring(2).split("/")[1]; @@ -42,18 +48,21 @@ export async function runTests(disabledPackages: string[]): Promise { } console.log(`Testing ${packageName}...`); const packagePath = path.join(workspaceCwd, "packages", packageName); - if (!await testPackage(packagePath)) { - failedPackages.push(packageName); - } + tests.push(testPackage(packagePath)); } + const results = await Promise.all(tests); + const failedPackages = results.filter((result) => !result.result.success); + if (failedPackages.length === 0) { console.log("All tests passing!"); } else { console.error("One or more tests failed."); console.error("Failed packages:"); - for (const pkg of failedPackages) { - console.error(`- ${pkg}`); + for (const result of failedPackages) { + console.error(`- ${result.packagePath}`); + console.log(decode(result.result.stdout)); + console.error(decode(result.result.stderr)); } Deno.exit(1); } From 819fa2844b55bb0f26ead50f2d293caef04cd48c Mon Sep 17 00:00:00 2001 From: gideon Date: Thu, 4 Dec 2025 05:03:23 +0800 Subject: [PATCH 6/6] refactor(ts-transformers): Simplify dataflow analyzer internal types (#2191) * refactor(ts-transformers): Simplify dataflow analyzer internal types Two key simplifications to the data flow analysis code: 1. Replace localNodes with expressionToNodeId Map - Eliminates InternalAnalysis.localNodes field entirely - Parent-child relationships now tracked via O(1) Map lookup - Reduces merge complexity (no more localNodes concatenation) - ~30 lines removed across return sites 2. Eliminate DataFlowScopeInternal type - Merge into DataFlowScope by computing enriched parameters eagerly - Remove pre-computed `aggregated` Set in favor of on-demand computation - getAggregatedSymbols() walks parent chain when needed - Remove toDataFlowScope() conversion function * refactor(ts-transformers): Unify synthetic/non-synthetic analysis paths Remove the separate ~300 line synthetic node handling block by integrating fallback logic into the unified expression handlers. This refactoring: - Adds tryGetSymbol/tryGetType helpers that gracefully handle synthetic nodes - Updates each expression handler to try checker methods first, then fall back to heuristics when dealing with synthetic nodes - Removes duplicate logic that was maintained in two places - Adds proper handlers for JsxSelfClosingElement and JsxFragment Net result: 214 lines removed (432 deletions, 218 insertions) * refactor(ts-transformers): Complete JSX handling in dataflow analyzer The dataflow analyzer now provides complete, self-contained analysis for JSX elements including both attributes and children. Previously, attribute analysis used `ts.isExpression(attr)` which always returned false for JsxAttribute nodes (dead code). The system worked because the OpaqueRefJSXTransformer handled JSX at the visitor level, but this created unclear contracts and hidden coupling. Now the analyzer properly handles: - JsxAttribute with expression initializers: `value={expr}` - JsxSpreadAttribute: `{...expr}` - JsxElement children (JsxExpression, nested elements) - JsxSelfClosingElement - JsxFragment This makes the analyzer's contract clear: callers get correct results regardless of how they traverse the AST. --- packages/ts-transformers/src/ast/dataflow.ts | 711 +++++++------------ 1 file changed, 260 insertions(+), 451 deletions(-) diff --git a/packages/ts-transformers/src/ast/dataflow.ts b/packages/ts-transformers/src/ast/dataflow.ts index ff28d37534..c03dfb859a 100644 --- a/packages/ts-transformers/src/ast/dataflow.ts +++ b/packages/ts-transformers/src/ast/dataflow.ts @@ -52,25 +52,18 @@ export interface DataFlowAnalysis { rewriteHint?: RewriteHint; } -interface DataFlowScopeInternal { - readonly id: number; - readonly parentId: number | null; - readonly parameterSymbols: ts.Symbol[]; - readonly aggregated: Set; -} - interface AnalyzerContext { nextNodeId: number; nextScopeId: number; readonly collectedNodes: DataFlowNode[]; // All nodes collected during analysis - readonly scopes: Map; + readonly scopes: Map; + readonly expressionToNodeId: Map; // For O(1) parent lookups } interface InternalAnalysis { containsOpaqueRef: boolean; requiresRewrite: boolean; dataFlows: ts.Expression[]; - localNodes: DataFlowNode[]; // Nodes from this expression subtree only rewriteHint?: RewriteHint; } @@ -78,7 +71,6 @@ const emptyAnalysis = (): InternalAnalysis => ({ containsOpaqueRef: false, requiresRewrite: false, dataFlows: [], - localNodes: [], rewriteHint: undefined, }); @@ -86,19 +78,16 @@ const mergeAnalyses = (...analyses: InternalAnalysis[]): InternalAnalysis => { let contains = false; let requires = false; const dataFlows: ts.Expression[] = []; - const localNodes: DataFlowNode[] = []; for (const analysis of analyses) { if (!analysis) continue; contains ||= analysis.containsOpaqueRef; requires ||= analysis.requiresRewrite; dataFlows.push(...analysis.dataFlows); - localNodes.push(...analysis.localNodes); } return { containsOpaqueRef: contains, requiresRewrite: requires, dataFlows, - localNodes, rewriteHint: undefined, }; }; @@ -106,57 +95,83 @@ const mergeAnalyses = (...analyses: InternalAnalysis[]): InternalAnalysis => { export function createDataFlowAnalyzer( checker: ts.TypeChecker, ): (expression: ts.Expression) => DataFlowAnalysis { + // === Synthetic node helpers === + // These enable unified handling of both synthetic (transformer-created) and + // non-synthetic (original source) nodes by gracefully handling cases where + // the TypeChecker can't resolve symbols or types. + + const isSynthetic = (node: ts.Node): boolean => !node.getSourceFile(); + + const tryGetSymbol = (node: ts.Node): ts.Symbol | undefined => { + try { + return checker.getSymbolAtLocation(node) ?? undefined; + } catch { + return undefined; + } + }; + + const tryGetType = (node: ts.Node): ts.Type | undefined => { + try { + return checker.getTypeAtLocation(node); + } catch { + return undefined; + } + }; + + // Convert symbols to enriched parameters with name and declaration info + const toScopeParameters = ( + symbols: ts.Symbol[], + ): DataFlowScopeParameter[] => + symbols.map((symbol) => { + const declarations = symbol.getDeclarations(); + const parameterDecl = declarations?.find(( + decl, + ): decl is ts.ParameterDeclaration => ts.isParameter(decl)); + return parameterDecl + ? { name: symbol.getName(), symbol, declaration: parameterDecl } + : { name: symbol.getName(), symbol }; + }); + const createScope = ( context: AnalyzerContext, - parent: DataFlowScopeInternal | null, + parent: DataFlowScope | null, parameterSymbols: ts.Symbol[], - ): DataFlowScopeInternal => { - const aggregated = parent - ? new Set(parent.aggregated) - : new Set(); - for (const symbol of parameterSymbols) aggregated.add(symbol); - const scope: DataFlowScopeInternal = { + ): DataFlowScope => { + const scope: DataFlowScope = { id: context.nextScopeId++, parentId: parent ? parent.id : null, - parameterSymbols, - aggregated, + parameters: toScopeParameters(parameterSymbols), }; context.scopes.set(scope.id, scope); return scope; }; + // Compute all parameters accessible from a scope (own + ancestors) on-demand + const getAggregatedSymbols = ( + scope: DataFlowScope, + scopes: Map, + ): Set => { + const result = new Set(); + let current: DataFlowScope | undefined = scope; + while (current) { + for (const param of current.parameters) { + result.add(param.symbol); + } + current = current.parentId !== null + ? scopes.get(current.parentId) + : undefined; + } + return result; + }; + const createCanonicalKey = ( expression: ts.Expression, - scope: DataFlowScopeInternal, + scope: DataFlowScope, ): string => { const text = getExpressionText(expression); return `${scope.id}:${text}`; }; - const toDataFlowScope = ( - scope: DataFlowScopeInternal, - ): DataFlowScope => ({ - id: scope.id, - parentId: scope.parentId, - parameters: scope.parameterSymbols.map((symbol) => { - const declarations = symbol.getDeclarations(); - const parameterDecl = declarations?.find(( - decl, - ): decl is ts.ParameterDeclaration => ts.isParameter(decl)); - if (parameterDecl) { - return { - name: symbol.getName(), - symbol, - declaration: parameterDecl, - } satisfies DataFlowScopeParameter; - } - return { - name: symbol.getName(), - symbol, - } satisfies DataFlowScopeParameter; - }), - }); - // Determine how CallExpressions should be handled based on their call kind. // Returns the appropriate InternalAnalysis with correct requiresRewrite logic. const handleCallExpression = ( @@ -205,344 +220,19 @@ export function createDataFlowAnalyzer( const analyzeExpression = ( expression: ts.Expression, - scope: DataFlowScopeInternal, + scope: DataFlowScope, context: AnalyzerContext, ): InternalAnalysis => { - // Handle synthetic nodes (created by previous transformers) - // We can't analyze them directly, but we need to visit children - if (!expression.getSourceFile()) { - // Set parent pointers for the entire synthetic subtree to enable - // parent-based logic (method call detection, etc.) to work + // Set parent pointers for synthetic nodes (needed for parent-based logic like method call detection) + if (isSynthetic(expression)) { setParentPointers(expression); - // Synthetic nodes don't have positions, so getText() will crash - // We don't currently use the printed text, but keep it for debugging - try { - const printer = ts.createPrinter(); - const _exprText = printer.printNode( - ts.EmitHint.Unspecified, - expression, - expression.getSourceFile() || - ts.createSourceFile("", "", ts.ScriptTarget.Latest), - ); - // _exprText could be logged for debugging if needed - void _exprText; - } catch { - // Ignore errors - } - - // Special handling for synthetic identifiers (must come BEFORE child traversal) - // These are likely parameters from map closure transformation (like `discount`) - // We can't resolve symbols for synthetic nodes, but we should treat them as opaque - if (ts.isIdentifier(expression)) { - // Skip property names in property access expressions - they're not data flows. - // For example, `toSchema` in `__ctHelpers.toSchema` is just a property name. - if ( - expression.parent && - ts.isPropertyAccessExpression(expression.parent) && - expression.parent.name === expression - ) { - // This is a property name, not a value reference - don't capture it - return emptyAnalysis(); - } - - // If it's a synthetic identifier, treat it as opaque - // This handles cases like `discount` where the whole identifier is synthetic - // We need to record it in the graph so normalizeDataFlows can find it - const node: DataFlowNode = { - id: context.nextNodeId++, - expression, - canonicalKey: `${scope.id}:${getExpressionText(expression)}`, - parentId: null, - scopeId: scope.id, - isExplicit: true, // Explicit: synthetic opaque parameter - }; - context.collectedNodes.push(node); - return { - containsOpaqueRef: true, - requiresRewrite: false, - dataFlows: [expression], - localNodes: [node], - }; - } - - // Collect analyses from all children - const childAnalyses: InternalAnalysis[] = []; - - // Helper to analyze a child expression - const analyzeChild = (child: ts.Node) => { - if (ts.isExpression(child)) { - const childAnalysis = analyzeExpression(child, scope, context); - childAnalyses.push(childAnalysis); - } - }; - - // Special handling for JSX elements - ts.forEachChild doesn't traverse JSX expression children - if (ts.isJsxElement(expression)) { - // Traverse opening element attributes - if (expression.openingElement.attributes) { - expression.openingElement.attributes.properties.forEach(analyzeChild); - } - // Traverse JSX children (this is what forEachChild misses!) - expression.children.forEach((child) => { - if (ts.isJsxExpression(child)) { - if (child.expression) { - analyzeChild(child.expression); - } - } else { - analyzeChild(child); - } - }); - } else if (ts.isJsxSelfClosingElement(expression)) { - // Traverse self-closing element attributes - if (expression.attributes) { - expression.attributes.properties.forEach(analyzeChild); - } - } else if (ts.isJsxFragment(expression)) { - // Traverse fragment children - expression.children.forEach((child) => { - if (ts.isJsxExpression(child) && child.expression) { - analyzeChild(child.expression); - } else { - analyzeChild(child); - } - }); - } else { - // For non-JSX nodes, use the default traversal - ts.forEachChild(expression, analyzeChild); - } - - // Inherit properties from children - if (childAnalyses.length > 0) { - const merged = mergeAnalyses(...childAnalyses); - - // For synthetic CallExpressions, detect call kind and set rewriteHint - if (ts.isCallExpression(expression)) { - const callKind = detectCallKind(expression, checker); - const rewriteHint: RewriteHint | undefined = (() => { - if (callKind?.kind === "builder") { - return { kind: "skip-call-rewrite", reason: "builder" }; - } - if (callKind?.kind === "array-map") { - return { kind: "skip-call-rewrite", reason: "array-map" }; - } - if ( - callKind?.kind === "ifElse" && expression.arguments.length > 0 - ) { - const predicate = expression.arguments[0]; - if (predicate) { - return { kind: "call-if-else", predicate }; - } - } - return undefined; - })(); - - // For synthetic CallExpressions, we don't have a separate callee analysis - // Approximate by using merged for both parameters - return handleCallExpression(merged, callKind, merged, rewriteHint); - } - - // Special handling for synthetic property access expressions - // For synthetic nodes, we can't use checker.getSymbolAtLocation or isOpaqueRefType reliably - // But we can detect if this looks like a property access that should be a dataflow - if (ts.isPropertyAccessExpression(expression)) { - // Find the root identifier by walking up the property chain - let current: ts.Expression = expression; - while (ts.isPropertyAccessExpression(current)) { - current = current.expression; - } - - if (ts.isIdentifier(current)) { - const symbol = checker.getSymbolAtLocation(current); - if (symbol) { - // Check if this is a parameter in an opaque call (builder or array-map) - const declarations = symbol.getDeclarations(); - if (declarations) { - for (const decl of declarations) { - if (ts.isParameter(decl)) { - // Walk up to find if this parameter belongs to a builder or array-map call - let func: ts.Node | undefined = decl.parent; - while (func && !ts.isFunctionLike(func)) func = func.parent; - if (func) { - let callNode: ts.Node | undefined = func.parent; - while (callNode && !ts.isCallExpression(callNode)) { - callNode = callNode.parent; - } - if (callNode) { - const callKind = detectCallKind( - callNode as ts.CallExpression, - checker, - ); - if ( - callKind?.kind === "array-map" || - callKind?.kind === "builder" - ) { - // This is element.price or state.foo - return full property access as dataflow - // Add to graph so normalizeDataFlows can find it - const node: DataFlowNode = { - id: context.nextNodeId++, - expression, - canonicalKey: `${scope.id}:${ - getExpressionText(expression) - }`, - parentId: null, - scopeId: scope.id, - isExplicit: true, // Explicit: synthetic opaque property access - }; - context.collectedNodes.push(node); - return { - containsOpaqueRef: true, - requiresRewrite: true, - dataFlows: [expression], - localNodes: [node], - }; - } - } - } - } - } - } - } else { - // Symbol is undefined for the root - this is likely a synthetic parameter - // from a transformer (like `element` from map closure transformer). - // We can't resolve symbols for synthetic nodes, but if this looks like - // a property access on a simple identifier (not a complex expression), - // treat it as an opaque property access that needs derive wrapping. - // This handles cases like `element.price` where `element` is synthetic. - - // Skip __ctHelpers.* property accesses - these are helper functions, not opaque refs. - // For example: __ctHelpers.toSchema, __ctHelpers.recipe, __ctHelpers.derive - if ( - ts.isIdentifier(current) && - current.text === "__ctHelpers" - ) { - // This is a helper function access, not an opaque ref - don't capture it - return merged; - } - - // Don't capture property accesses that are method calls. - // For example, `element.trim` in `element.trim()` should not be captured. - if (isMethodCall(expression)) { - // This is a method call like element.trim() - don't capture it - return merged; - } - - // Add to graph so normalizeDataFlows can find it - const node: DataFlowNode = { - id: context.nextNodeId++, - expression, - canonicalKey: `${scope.id}:${getExpressionText(expression)}`, - parentId: null, - scopeId: scope.id, - isExplicit: true, // Explicit: synthetic opaque property access - }; - context.collectedNodes.push(node); - return { - containsOpaqueRef: true, - requiresRewrite: true, - dataFlows: [expression], - localNodes: [node], - }; - } - } - // Otherwise preserve merged analysis from children - // NOTE: We should rarely hit this - it means the root wasn't an identifier - return merged; - } - - // For binary expressions with OpaqueRef, set requiresRewrite based on containsOpaqueRef - // This matches the logic in the non-synthetic code path (line 380-388) - if (ts.isBinaryExpression(expression)) { - return { - ...merged, - requiresRewrite: merged.containsOpaqueRef, - }; - } - - // For conditional expressions, set requiresRewrite to true if they contain opaque refs - // This matches the non-synthetic code path for conditional expressions (line 720) - if (ts.isConditionalExpression(expression)) { - return { - ...merged, - requiresRewrite: true, - }; - } - - // Element access expressions: static indices don't need derive wrapping, - // but dynamic indices with opaque refs do (e.g., tagCounts[element]) - if (ts.isElementAccessExpression(expression)) { - const isStaticIndex = isStaticElementAccess(expression); - - if (isStaticIndex) { - // Static index like element[0] - preserve merged analysis - return merged; - } else if (merged.containsOpaqueRef) { - // Dynamic index with opaque refs - requires derive wrapper - return { - ...merged, - requiresRewrite: true, - }; - } - return merged; - } - - // For JSX elements, arrow functions, and other expression containers, preserve requiresRewrite from children - // This matches the non-synthetic code paths for these node types - if ( - ts.isJsxElement(expression) || - ts.isJsxFragment(expression) || - ts.isJsxSelfClosingElement(expression) || - ts.isParenthesizedExpression(expression) || - ts.isArrowFunction(expression) || - ts.isFunctionExpression(expression) - ) { - return merged; - } - - // Other synthetic nodes don't require rewrite - return { - ...merged, - requiresRewrite: false, - }; - } - - // No children with analysis - return { - containsOpaqueRef: false, - requiresRewrite: false, - dataFlows: [], - localNodes: [], - rewriteHint: undefined, - }; } - const isSymbolIgnored = (symbol: ts.Symbol | undefined): boolean => { - if (!symbol) return false; - if (scope.aggregated.has(symbol) && isRootOpaqueParameter(symbol)) { - return false; - } - return scope.aggregated.has(symbol); - }; - - const originatesFromIgnored = (expr: ts.Expression): boolean => { - if (ts.isIdentifier(expr)) { - const symbol = checker.getSymbolAtLocation(expr); - return isSymbolIgnored(symbol); - } - if ( - ts.isPropertyAccessExpression(expr) || - ts.isElementAccessExpression(expr) - ) { - return originatesFromIgnored(expr.expression); - } - if (ts.isCallExpression(expr)) { - return originatesFromIgnored(expr.expression); - } - return false; - }; + // === Helper functions (available for both synthetic and non-synthetic paths) === const recordDataFlow = ( expr: ts.Expression, - ownerScope: DataFlowScopeInternal, + ownerScope: DataFlowScope, parentId: number | null = null, isExplicit: boolean = false, ): DataFlowNode => { @@ -555,6 +245,7 @@ export function createDataFlowAnalyzer( isExplicit, }; context.collectedNodes.push(node); + context.expressionToNodeId.set(expr, node.id); return node; }; @@ -624,43 +315,91 @@ export function createDataFlowAnalyzer( ): boolean => { const root = findRootIdentifier(expr); if (!root) return false; - const symbol = checker.getSymbolAtLocation(root); + const symbol = tryGetSymbol(root); return isRootOpaqueParameter(symbol); }; + const isSymbolIgnored = (symbol: ts.Symbol | undefined): boolean => { + if (!symbol) return false; + const aggregated = getAggregatedSymbols(scope, context.scopes); + if (aggregated.has(symbol) && isRootOpaqueParameter(symbol)) { + return false; + } + return aggregated.has(symbol); + }; + + const originatesFromIgnored = (expr: ts.Expression): boolean => { + if (ts.isIdentifier(expr)) { + const symbol = tryGetSymbol(expr); + return isSymbolIgnored(symbol); + } + if ( + ts.isPropertyAccessExpression(expr) || + ts.isElementAccessExpression(expr) + ) { + return originatesFromIgnored(expr.expression); + } + if (ts.isCallExpression(expr)) { + return originatesFromIgnored(expr.expression); + } + return false; + }; + + // === Expression type handlers === + if (ts.isIdentifier(expression)) { - const symbol = checker.getSymbolAtLocation(expression); + // Skip property names in property access expressions - they're not data flows. + // For example, `toSchema` in `__ctHelpers.toSchema` is just a property name. + if ( + expression.parent && + ts.isPropertyAccessExpression(expression.parent) && + expression.parent.name === expression + ) { + return emptyAnalysis(); + } + + const symbol = tryGetSymbol(expression); + + // Can't resolve symbol - if synthetic, treat as opaque parameter + // This handles cases like `discount` where the whole identifier is synthetic + if (!symbol && isSynthetic(expression)) { + recordDataFlow(expression, scope, null, true); // Explicit: synthetic opaque parameter + return { + containsOpaqueRef: true, + requiresRewrite: false, + dataFlows: [expression], + }; + } + if (isSymbolIgnored(symbol)) { return emptyAnalysis(); } - const type = checker.getTypeAtLocation(expression); - if (isOpaqueRefType(type, checker)) { - const node = recordDataFlow(expression, scope, null, true); // Explicit: direct OpaqueRef + + const type = tryGetType(expression); + if (type && isOpaqueRefType(type, checker)) { + recordDataFlow(expression, scope, null, true); // Explicit: direct OpaqueRef return { containsOpaqueRef: true, requiresRewrite: false, dataFlows: [expression], - localNodes: [node], }; } if (symbolDeclaresCommonToolsDefault(symbol, checker)) { - const node = recordDataFlow(expression, scope, null, true); // Explicit: CommonTools default + recordDataFlow(expression, scope, null, true); // Explicit: CommonTools default return { containsOpaqueRef: true, requiresRewrite: false, dataFlows: [expression], - localNodes: [node], }; } // Check if this identifier is a parameter to a builder or array-map call (like recipe) // These parameters become implicitly opaque even though their type isn't OpaqueRef if (isRootOpaqueParameter(symbol)) { - const node = recordDataFlow(expression, scope, null, true); // Explicit: opaque parameter + recordDataFlow(expression, scope, null, true); // Explicit: opaque parameter return { containsOpaqueRef: true, requiresRewrite: false, dataFlows: [expression], - localNodes: [node], }; } return emptyAnalysis(); @@ -668,16 +407,15 @@ export function createDataFlowAnalyzer( if (ts.isPropertyAccessExpression(expression)) { const target = analyzeExpression(expression.expression, scope, context); - const propertyType = checker.getTypeAtLocation(expression); + const propertyType = tryGetType(expression); - if (isOpaqueRefType(propertyType, checker)) { + if (propertyType && isOpaqueRefType(propertyType, checker)) { if (originatesFromIgnored(expression.expression)) { return emptyAnalysis(); } const parentId = - findParentNodeId(target.localNodes, expression.expression) ?? - null; - const node = recordDataFlow(expression, scope, parentId, true); // Explicit: OpaqueRef property + context.expressionToNodeId.get(expression.expression) ?? null; + recordDataFlow(expression, scope, parentId, true); // Explicit: OpaqueRef property // If the target is a complex expression requiring rewrite (like ElementAccess), // propagate its dataFlows. Otherwise, add this property access as a dataFlow. @@ -686,14 +424,12 @@ export function createDataFlowAnalyzer( containsOpaqueRef: true, requiresRewrite: target.requiresRewrite, dataFlows: target.dataFlows, - localNodes: [node], }; } else { return { containsOpaqueRef: true, requiresRewrite: target.requiresRewrite, dataFlows: [expression], - localNodes: [node], }; } } @@ -703,13 +439,12 @@ export function createDataFlowAnalyzer( return emptyAnalysis(); } const parentId = - findParentNodeId(target.localNodes, expression.expression) ?? null; - const node = recordDataFlow(expression, scope, parentId, true); // Explicit: CommonTools property + context.expressionToNodeId.get(expression.expression) ?? null; + recordDataFlow(expression, scope, parentId, true); // Explicit: CommonTools property return { containsOpaqueRef: true, requiresRewrite: true, dataFlows: [expression], - localNodes: [node], }; } if (isImplicitOpaqueRefExpression(expression)) { @@ -721,7 +456,7 @@ export function createDataFlowAnalyzer( const isPropertyOnCall = ts.isCallExpression(expression.expression); const parentId = - findParentNodeId(target.localNodes, expression.expression) ?? null; + context.expressionToNodeId.get(expression.expression) ?? null; // If the target is a complex expression requiring rewrite (ElementAccess or Call), // propagate its dataFlows. Otherwise add this property access as a dataFlow. @@ -730,30 +465,80 @@ export function createDataFlowAnalyzer( (target.requiresRewrite && target.dataFlows.length > 0) ) { // This is a computed expression - use the dependencies from the target - const node = recordDataFlow(expression, scope, parentId, false); + recordDataFlow(expression, scope, parentId, false); return { containsOpaqueRef: true, requiresRewrite: true, dataFlows: target.dataFlows, - localNodes: [node], }; } // This is a direct property access on an OpaqueRef (like state.charms.length) // It should be its own explicit dependency - const node = recordDataFlow(expression, scope, parentId, true); + recordDataFlow(expression, scope, parentId, true); return { containsOpaqueRef: true, requiresRewrite: true, dataFlows: [expression], - localNodes: [node], }; } + + // For synthetic nodes where type/symbol resolution failed, check the root identifier + if (isSynthetic(expression)) { + const root = findRootIdentifier(expression); + if (root && ts.isIdentifier(root)) { + const rootSymbol = tryGetSymbol(root); + if (rootSymbol) { + // Root symbol found - check if it's from builder/array-map + const callKind = getOpaqueParameterCallKind(rootSymbol); + if (callKind) { + // This is element.price or similar - treat as opaque property access + const parentId = + context.expressionToNodeId.get(expression.expression) ?? null; + recordDataFlow(expression, scope, parentId, true); + return { + containsOpaqueRef: true, + requiresRewrite: true, + dataFlows: [expression], + }; + } + } else { + // Root symbol undefined - fully synthetic parameter (like `element`) + // Skip __ctHelpers.* property accesses - these are helper functions + if (root.text === "__ctHelpers") { + return { + containsOpaqueRef: target.containsOpaqueRef, + requiresRewrite: target.requiresRewrite || + target.containsOpaqueRef, + dataFlows: target.dataFlows, + }; + } + // Skip method calls like element.trim() + if (isMethodCall(expression)) { + return { + containsOpaqueRef: target.containsOpaqueRef, + requiresRewrite: target.requiresRewrite || + target.containsOpaqueRef, + dataFlows: target.dataFlows, + }; + } + // Treat as opaque property access + const parentId = + context.expressionToNodeId.get(expression.expression) ?? null; + recordDataFlow(expression, scope, parentId, true); + return { + containsOpaqueRef: true, + requiresRewrite: true, + dataFlows: [expression], + }; + } + } + } + return { containsOpaqueRef: target.containsOpaqueRef, requiresRewrite: target.requiresRewrite || target.containsOpaqueRef, dataFlows: target.dataFlows, - localNodes: target.localNodes, }; } @@ -780,14 +565,13 @@ export function createDataFlowAnalyzer( return emptyAnalysis(); } const parentId = - findParentNodeId(target.localNodes, expression.expression) ?? null; + context.expressionToNodeId.get(expression.expression) ?? null; // Element access on implicit opaque ref - this is likely an explicit dependency - const node = recordDataFlow(expression, scope, parentId, true); + recordDataFlow(expression, scope, parentId, true); return { containsOpaqueRef: true, requiresRewrite: true, dataFlows: [expression], - localNodes: [node], }; } return { @@ -795,7 +579,6 @@ export function createDataFlowAnalyzer( argument.containsOpaqueRef, requiresRewrite: true, dataFlows: [...target.dataFlows, ...argument.dataFlows], - localNodes: [...target.localNodes, ...argument.localNodes], }; } @@ -829,11 +612,6 @@ export function createDataFlowAnalyzer( ...whenTrue.dataFlows, ...whenFalse.dataFlows, ], - localNodes: [ - ...condition.localNodes, - ...whenTrue.localNodes, - ...whenFalse.localNodes, - ], }; } @@ -856,7 +634,6 @@ export function createDataFlowAnalyzer( containsOpaqueRef: operand.containsOpaqueRef, requiresRewrite: operand.containsOpaqueRef, dataFlows: operand.dataFlows, - localNodes: operand.localNodes, }; } @@ -885,7 +662,7 @@ export function createDataFlowAnalyzer( if (isFunctionLikeExpression(arg)) { const parameterSymbols: ts.Symbol[] = []; for (const parameter of arg.parameters) { - const symbol = checker.getSymbolAtLocation(parameter.name); + const symbol = tryGetSymbol(parameter.name); if (symbol) { parameterSymbols.push(symbol); } @@ -933,7 +710,7 @@ export function createDataFlowAnalyzer( if (isFunctionLikeExpression(expression)) { const parameterSymbols: ts.Symbol[] = []; for (const parameter of expression.parameters) { - const symbol = checker.getSymbolAtLocation(parameter.name); + const symbol = tryGetSymbol(parameter.name); if (symbol) parameterSymbols.push(symbol); } const childScope = createScope(context, scope, parameterSymbols); @@ -976,29 +753,74 @@ export function createDataFlowAnalyzer( return mergeAnalyses(...analyses); } - // Handle JSX elements in non-synthetic path too - if (ts.isJsxElement(expression)) { - const analyses: InternalAnalysis[] = []; - // Analyze opening element attributes - if (expression.openingElement.attributes) { - expression.openingElement.attributes.properties.forEach((attr) => { - if (ts.isExpression(attr)) { - analyses.push(analyzeExpression(attr, scope, context)); + // === JSX Expression Handling === + // The analyzer provides complete data flow analysis for JSX elements, + // including both attributes (like `value={expr}`) and children. + // This makes the analyzer self-contained - callers get correct results + // regardless of how they traverse the AST. + + // Helper: analyze JSX attributes (JsxAttribute and JsxSpreadAttribute) + const analyzeJsxAttributes = ( + attributes: ts.JsxAttributes, + ): InternalAnalysis[] => { + const results: InternalAnalysis[] = []; + for (const attr of attributes.properties) { + if (ts.isJsxAttribute(attr)) { + // - analyze the expression inside {expr} + if ( + attr.initializer && + ts.isJsxExpression(attr.initializer) && + attr.initializer.expression + ) { + results.push( + analyzeExpression(attr.initializer.expression, scope, context), + ); } - }); + // String literal initializers (value="string") have no dependencies + } else if (ts.isJsxSpreadAttribute(attr)) { + // - analyze the spread expression + results.push(analyzeExpression(attr.expression, scope, context)); + } } - // Analyze JSX children - must handle JsxExpression specially - expression.children.forEach((child) => { - if (ts.isJsxExpression(child)) { - if (child.expression) { - analyses.push(analyzeExpression(child.expression, scope, context)); - } - } else if (ts.isJsxElement(child)) { - analyses.push(analyzeExpression(child, scope, context)); + return results; + }; + + // Helper: analyze JSX children + const analyzeJsxChildren = ( + children: ts.NodeArray, + ): InternalAnalysis[] => { + const results: InternalAnalysis[] = []; + for (const child of children) { + if (ts.isJsxExpression(child) && child.expression) { + // {expr} - analyze the inner expression + results.push(analyzeExpression(child.expression, scope, context)); + } else if ( + ts.isJsxElement(child) || ts.isJsxSelfClosingElement(child) + ) { + // Nested JSX elements - recurse + results.push(analyzeExpression(child, scope, context)); } - // Ignore JsxText and other non-expression children - }); - return mergeAnalyses(...analyses); + // JsxText nodes have no dependencies + } + return results; + }; + + if (ts.isJsxElement(expression)) { + const attrAnalyses = analyzeJsxAttributes( + expression.openingElement.attributes, + ); + const childAnalyses = analyzeJsxChildren(expression.children); + return mergeAnalyses(...attrAnalyses, ...childAnalyses); + } + + if (ts.isJsxSelfClosingElement(expression)) { + const attrAnalyses = analyzeJsxAttributes(expression.attributes); + return mergeAnalyses(...attrAnalyses); + } + + if (ts.isJsxFragment(expression)) { + const childAnalyses = analyzeJsxChildren(expression.children); + return mergeAnalyses(...childAnalyses); } const analyses: InternalAnalysis[] = []; @@ -1019,16 +841,15 @@ export function createDataFlowAnalyzer( nextScopeId: 0, collectedNodes: [], scopes: new Map(), + expressionToNodeId: new Map(), }; const rootScope = createScope(context, null, []); const result = analyzeExpression(expression, rootScope, context); - const scopes = Array.from(context.scopes.values()).map(toDataFlowScope); - const { localNodes: _, ...resultWithoutNodes } = result; return { - ...resultWithoutNodes, + ...result, graph: { nodes: context.collectedNodes, - scopes, + scopes: Array.from(context.scopes.values()), rootScopeId: rootScope.id, }, }; @@ -1090,15 +911,3 @@ export function collectOpaqueRefs( visit(node); return refs; } -const findParentNodeId = ( - nodes: DataFlowNode[], - target: ts.Expression, -): number | null => { - for (let index = nodes.length - 1; index >= 0; index--) { - const node = nodes[index]; - if (node && node.expression === target) { - return node.id; - } - } - return null; -};