mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
abcbce991e
## Problem The AST component index (`buildAstIndexWithDiagnostics`) is keyed by component **name**. When two files define a component with the same name — most commonly platform variants, e.g. `List` in `List.tsx` and `List` in `List.web.tsx` — the index kept whichever file the directory walk happened to reach **first** (`if (!index.has(name))`) and threw the rest away. So a lookup for `List` could return the wrong platform variant's source, with no signal that other matches even existed. And because it depended on filesystem walk order, *which* `List` survived was effectively arbitrary. Raised in review on #287. ## Fix - **Collect every match per name** instead of keeping the first walked. - **Choose a deterministic primary**: base files are preferred over `.web` / `.ios` / `.android` / `.native` variants (those only override the base when bundling for that platform), then ties break on path → line → column. The result no longer depends on directory-walk order. - **Surface the rest** as `otherMatches` on the index entry. `react-profiler-component-source` now returns `otherMatches: [{ file, line, col }]` alongside the primary, so a caller asking for `List` can tell the locations apart instead of trusting a single arbitrary file. `react-profiler-analyze` needs no change — it enriches findings with the (now deterministic) primary location, which is strictly better than before; surfacing per-finding alternates there is out of scope. ## Tests `packages/tool-server/test/react-profiler/ast-index.test.ts` (index layer) and a new `component-source-duplicates.test.ts` (end-to-end through the tool's `execute`): - same name in `List.tsx` + `List.web.tsx` → primary is the base file, its source is returned, `List.web.tsx` is under `otherMatches`; - only platform variants (`.android` + `.ios`, no base) → deterministic primary by path; - uniquely-named component → no `otherMatches`. The two collision assertions fail on pre-fix code (no `otherMatches`, arbitrary primary) and pass with the fix. Verified locally with Node 20: `vitest run test/react-profiler/` — 80 passed; `tsc --build` clean; prettier + eslint clean.
150 lines
6.9 KiB
TypeScript
150 lines
6.9 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
|
import { mkdtempSync, mkdirSync, writeFileSync } from "fs";
|
|
import { tmpdir } from "os";
|
|
import { join } from "path";
|
|
import { buildAstIndexWithDiagnostics } from "../../src/utils/react-profiler/pipeline/06-resolve/ast-index";
|
|
|
|
/**
|
|
* react-profiler-component-source returned found:false for every component in a
|
|
* standard TS/TSX Expo project because @swmansion/argent never declared (or
|
|
* shipped) tree-sitter / tree-sitter-typescript, so the parser require() threw,
|
|
* was swallowed, and the index was always empty. These tests guard the matcher
|
|
* for the idiomatic declaration forms (plain/default/arrow AND memo/forwardRef
|
|
* wrappers) and, via treeSitterAvailable, fail loudly if the parser dependency
|
|
* goes missing again.
|
|
*/
|
|
describe("buildAstIndexWithDiagnostics", () => {
|
|
it("indexes export-default-function, plain-function, and arrow TSX components", async () => {
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-"));
|
|
mkdirSync(join(dir, "components"), { recursive: true });
|
|
writeFileSync(
|
|
join(dir, "components", "foo.tsx"),
|
|
[
|
|
`import React from "react";`,
|
|
`type P = { x: number };`,
|
|
`export default function Foo({ x }: P) { return <View>{x}</View>; }`,
|
|
`function Bar({ y }: { y: number }) { return <View>{y}</View>; }`,
|
|
`const Baz = ({ z }: { z: number }) => <View>{z}</View>;`,
|
|
``,
|
|
].join("\n")
|
|
);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
|
|
expect(res.treeSitterAvailable).toBe(true);
|
|
expect(res.index.get("Foo")?.line).toBe(3);
|
|
expect(res.index.get("Bar")?.line).toBe(4);
|
|
expect(res.index.get("Baz")?.line).toBe(5);
|
|
});
|
|
|
|
it("indexes memo()/forwardRef()/React.memo()/nested-wrapped components", async () => {
|
|
// Regression: wrapped components are declared `const X = memo(...)`, whose
|
|
// value node is a call_expression (not arrow/function), so they were missed
|
|
// entirely and returned found:false — and profiler findings are
|
|
// disproportionately memo-wrapped. memo => isMemoized true; forwardRef alone
|
|
// => false.
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-wrap-"));
|
|
mkdirSync(join(dir, "components"), { recursive: true });
|
|
writeFileSync(
|
|
join(dir, "components", "wrapped.tsx"),
|
|
[
|
|
`import React, { memo, forwardRef } from "react";`,
|
|
`export const MemoFn = memo(function MemoFn() { return <View />; });`,
|
|
`export const MemoArrow = memo(({ a }: { a: number }) => <View>{a}</View>);`,
|
|
`export const Fwd = forwardRef(function FwdImpl(p, ref) { return <View />; });`,
|
|
`export const Dotted = React.memo(function Dotted() { return <View />; });`,
|
|
`export const Nested = memo(forwardRef(function NestedImpl(p, ref) { return <View />; }));`,
|
|
``,
|
|
].join("\n")
|
|
);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
|
|
expect(res.treeSitterAvailable).toBe(true);
|
|
expect(res.index.get("MemoFn")).toMatchObject({ line: 2, isMemoized: true });
|
|
expect(res.index.get("MemoArrow")).toMatchObject({ line: 3, isMemoized: true });
|
|
expect(res.index.get("Fwd")).toMatchObject({ line: 4, isMemoized: false });
|
|
expect(res.index.get("Dotted")).toMatchObject({ line: 5, isMemoized: true });
|
|
expect(res.index.get("Nested")).toMatchObject({ line: 6, isMemoized: true });
|
|
});
|
|
|
|
it("keeps every same-named component and prefers the base file over platform variants", async () => {
|
|
// Regression: when a name exists in several files (e.g. List.tsx and
|
|
// List.web.tsx) only the first one the walk reached survived and the rest
|
|
// were thrown away, so a lookup could hand back the wrong platform variant's
|
|
// source. Now the base file is the deterministic primary and the variants
|
|
// are surfaced under otherMatches.
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-dup-"));
|
|
mkdirSync(join(dir, "components"), { recursive: true });
|
|
const base = join(dir, "components", "List.tsx");
|
|
const web = join(dir, "components", "List.web.tsx");
|
|
writeFileSync(base, `export function List() { return <View>native</View>; }\n`);
|
|
writeFileSync(
|
|
web,
|
|
[`// platform override`, `export function List() { return <div>web</div>; }`, ``].join("\n")
|
|
);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
const entry = res.index.get("List");
|
|
|
|
expect(entry?.file).toBe(base);
|
|
expect(entry?.line).toBe(1);
|
|
expect(entry?.otherMatches).toEqual([{ file: web, line: 2, col: 16 }]);
|
|
});
|
|
|
|
it("falls back to a deterministic primary when no base file exists", async () => {
|
|
// Only platform variants — there is no base file to prefer, so the tie
|
|
// breaks on path (walk-order independent) rather than whatever the directory
|
|
// happened to yield first.
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-dup2-"));
|
|
mkdirSync(join(dir, "components"), { recursive: true });
|
|
const android = join(dir, "components", "List.android.tsx");
|
|
const ios = join(dir, "components", "List.ios.tsx");
|
|
writeFileSync(ios, `export function List() { return <View>ios</View>; }\n`);
|
|
writeFileSync(android, `export function List() { return <View>android</View>; }\n`);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
const entry = res.index.get("List");
|
|
|
|
// ".android." sorts before ".ios." lexicographically.
|
|
expect(entry?.file).toBe(android);
|
|
expect(entry?.otherMatches).toEqual([{ file: ios, line: 1, col: 16 }]);
|
|
});
|
|
|
|
it("omits otherMatches when a component name is unique", async () => {
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-uniq-"));
|
|
writeFileSync(join(dir, "Solo.tsx"), `export function Solo() { return <View>x</View>; }\n`);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
|
|
expect(res.index.get("Solo")).toBeDefined();
|
|
expect(res.index.get("Solo")?.otherMatches).toBeUndefined();
|
|
});
|
|
|
|
it("detects cross-referenced memo() via AST and ignores memo() in comments/strings", async () => {
|
|
// `function Card(){}; export default memo(Card)` is the cross-reference form
|
|
// (not an inline `const X = memo(...)`). The decoy `memo(Ghost)` lives only
|
|
// in a comment and a string literal — tree-sitter never emits a call node
|
|
// there, so Ghost stays unmemoized. The old raw-source regex flagged it.
|
|
const dir = mkdtempSync(join(tmpdir(), "ast-index-memo-ref-"));
|
|
mkdirSync(join(dir, "components"), { recursive: true });
|
|
writeFileSync(
|
|
join(dir, "components", "ref.tsx"),
|
|
[
|
|
`import React, { memo } from "react";`,
|
|
`function Card() { return <View />; }`,
|
|
`export default memo(Card);`,
|
|
`function Ghost() { return <View />; }`,
|
|
`// not memoized: memo(Ghost) appears only in this comment`,
|
|
`const note = "memo(Ghost)";`,
|
|
``,
|
|
].join("\n")
|
|
);
|
|
|
|
const res = await buildAstIndexWithDiagnostics(dir);
|
|
|
|
expect(res.index.get("Card")).toMatchObject({ isMemoized: true });
|
|
expect(res.index.get("Ghost")).toMatchObject({ isMemoized: false });
|
|
});
|
|
});
|