mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-24 11:24:49 +08:00
fix(react): a type annotation in a .tsx file names a type, never a same-named component (#2737)
React's Pattern 1 ran its component lookup on every PascalCase name in a
.tsx or .jsx file, type annotations included, and its candidate could
outrank name matching, so an annotation linked to a component: outline's
InputSelect.tsx declares `type Option`, and its annotations went to the
`Option` component of Export/ExportDialog.tsx; mastodon's `<Item>` type
parameters went to the `Item` class of media_gallery.jsx, and bulletproof's
`TableProps<Entry>` to another app's `function Entry`.
A type annotation is now name matching's, which scopes a type as TypeScript
does since PR #2736: the file's own, an import's or a global one. Pattern 1
decides only a type alias or interface the file declares beside a value or
a component of the same name, both of them the file's own: the annotation is
the type. That covers bigbluebutton's `type ModalRegistration` beside its
render-prop component, mastodon's `type ShortNumberRenderer` and `interface
AccountList` beside their functions, and a type beside a value that PR #2420
reads through its wrapper or loader. Every other reference keeps PR #2420's
order: a value's loaded or wrapped component, then a function, class or
component the file declares, then name matching for any other declaration
of the file's, and the component lookup only for a name the file doesn't
declare.
Validated on 13 React repos, main 458ad519 against this fix, every changed
site checked against the TypeScript checker and the source:
- 11 annotations move from a component to the type their file declares
(bbb 5, outline 3, mastodon 3);
- 15 type parameters and qualified types lose a wrong component and link
nothing (mastodon 8, bulletproof 3, bbb 3, outline 1);
- 220 last names of qualified types, like `React.ReactNode`, `Y.Doc` and
`SectionKind.Legend`, now come from name matching, wrong before and after:
219 keep their target and 1 moves to another wrong one (outline 205,
excalidraw 9, signoz 3, mastodon 3); PR #2736 left that name to name
matching on purpose;
- next-saas, proshop, t3-turbo, minimal-kit, matx, berry and mantis are
byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
458ad51921
commit
7b9c356dd7
@@ -264,6 +264,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In TypeScript and JavaScript, a call to a function's own parameter, like `isAvailable(f)` on a parameter typed `(f: string) => boolean` or `compare(a, b)` on a comparator parameter, no longer links to a function of the same name in another file, or to the file's own import of that name. Before, a parameter whose type or default value contained parentheses, as a function type does, wasn't recognized as one. A parameter or local stands in for an import of its name only where it is in scope, so a Promise executor's `resolve(…)` no longer links to an imported `path.resolve`, while a `resolve(file)` after the executor still does. Re-index TypeScript and JavaScript projects after upgrading.
|
||||
- In TypeScript and JavaScript, a name ending in `$`, like `syncState$`, is no longer read as a declaration of `syncState`, so a call to an imported `syncState(…)` in the same function keeps its link, and a file's own `function $()` or `const $ = …` now counts as declaring `$`.
|
||||
- In TypeScript, a type a file uses without importing it, like `Omit<T, K>`, `Record<K, V>`, the DOM's `Event`, or a generic's own `T`, no longer links to a type of that name another module of your project declares, like a local `export type Omit` or an enum member `T`: it links to the file's own type or a global one, declared in a script `.d.ts` or a `declare global` block. A type you import, write qualified (`vscode.TextDocument`), bring in with `import X = ns.X`, or name inside a `declare module` block links as before. Re-index TypeScript projects after upgrading.
|
||||
- In TypeScript React files, a type annotation no longer links to a component of the same name. When a file declares a type next to a component or value of the same name, like `type ModalRegistration` beside a `ModalRegistration` component, its annotations now link to the type while its calls still link to the component, and a type the file declares on its own, like `type Option`, or a generic's type parameter, like `<Item>`, no longer links to an `Option` or `Item` component elsewhere in the project. Before, `codegraph callers` and impact listed those annotations as uses of the component. Re-index TypeScript React projects after upgrading.
|
||||
|
||||
## [1.6.2] - 2026-10-03
|
||||
|
||||
|
||||
@@ -0,0 +1,286 @@
|
||||
/**
|
||||
* A type annotation in a .tsx file names a type, never a component elsewhere,
|
||||
* and a type alias or interface the file declares beside a component or value
|
||||
* of the same name is what its annotations name. The React resolver's
|
||||
* component lookup ran before name matching, so annotations went to
|
||||
* components: outline's InputSelect.tsx declares `type Option` and its
|
||||
* annotations linked to the `Option` component of Export/ExportDialog.tsx;
|
||||
* bigbluebutton's modalController.tsx declares `type ModalRegistration` beside
|
||||
* a `ModalRegistration` component and its annotations linked to the component;
|
||||
* mastodon's `<Item>` type parameters linked to an `Item` class.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import type { Node } from '../src/types';
|
||||
|
||||
const roots: string[] = [];
|
||||
const graphs: CodeGraph[] = [];
|
||||
|
||||
async function index(files: Record<string, string>): Promise<CodeGraph> {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-react-own-'));
|
||||
roots.push(root);
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
const cg = await CodeGraph.init(root, { index: true });
|
||||
graphs.push(cg);
|
||||
return cg;
|
||||
}
|
||||
|
||||
const PACKAGE = JSON.stringify({ name: 'app', private: true, dependencies: { react: '^18.0.0', 'styled-components': '^6.0.0' } });
|
||||
|
||||
/** The components folder's namesakes of the names the files below declare themselves. */
|
||||
const NAMESAKES: Record<string, string> = {
|
||||
'app/components/Emoji.tsx': `import styled from "styled-components";
|
||||
|
||||
type Props = { size?: number };
|
||||
|
||||
export const Emoji = styled.span<Props>\`
|
||||
font-size: \${({ size }) => size ?? 16}px;
|
||||
\`;
|
||||
`,
|
||||
'app/components/Export/ExportDialog.tsx': `export function Option({ label }: { label: string }) {
|
||||
return <li>{label}</li>;
|
||||
}
|
||||
`,
|
||||
'app/features/explore/components/Story.tsx': `export function Story({ title }: { title: string }) {
|
||||
return <article>{title}</article>;
|
||||
}
|
||||
`,
|
||||
'app/components/Mode.tsx': `export function Mode({ label }: { label: string }) {
|
||||
return <span>{label}</span>;
|
||||
}
|
||||
`,
|
||||
'app/pages/profile.tsx': `export function Entry({ label }: { label: string }) {
|
||||
return <dt>{label}</dt>;
|
||||
}
|
||||
`,
|
||||
};
|
||||
|
||||
const FILES: Record<string, string> = {
|
||||
'package.json': PACKAGE,
|
||||
'app/editor/components/SuggestionsMenu.tsx': `export type Props<T> = {
|
||||
items: T[];
|
||||
renderMenuItem: (item: T) => unknown;
|
||||
search?: string;
|
||||
};
|
||||
|
||||
export default function SuggestionsMenu<T>(props: Props<T>) {
|
||||
return <ul>{props.items.length}</ul>;
|
||||
}
|
||||
`,
|
||||
'app/editor/components/EmojiMenu.tsx': `import type { Props as SuggestionsMenuProps } from "./SuggestionsMenu";
|
||||
|
||||
type Emoji = {
|
||||
name: string;
|
||||
emoji: string;
|
||||
};
|
||||
|
||||
type Props = Omit<
|
||||
SuggestionsMenuProps<Emoji>,
|
||||
"renderMenuItem" | "items"
|
||||
>;
|
||||
|
||||
export default function EmojiMenu(props: Props) {
|
||||
return <div>{props.search}</div>;
|
||||
}
|
||||
`,
|
||||
'app/components/primitives/SplitButton.tsx': `interface Option {
|
||||
label: string;
|
||||
onClick: () => void;
|
||||
}
|
||||
|
||||
type Props = {
|
||||
options: Option[];
|
||||
};
|
||||
|
||||
export function SplitButton({ options }: Props) {
|
||||
return <div>{options.length}</div>;
|
||||
}
|
||||
`,
|
||||
'app/components/button/button.stories.tsx': `import type { Meta, StoryObj } from "@storybook/react";
|
||||
|
||||
const meta = {
|
||||
title: "Button",
|
||||
} satisfies Meta;
|
||||
|
||||
export default meta;
|
||||
|
||||
type Story = StoryObj<typeof meta>;
|
||||
|
||||
export const Primary: Story = {
|
||||
args: { label: "Save" },
|
||||
};
|
||||
`,
|
||||
// A type and a component of the same name: bigbluebutton's modalController.tsx.
|
||||
'app/core/singletons/modalController.tsx': `type ModalRegistration = {
|
||||
id: string;
|
||||
priority: number;
|
||||
};
|
||||
|
||||
type ModalState = {
|
||||
byKey: Record<string, ModalRegistration>;
|
||||
};
|
||||
|
||||
export const state: ModalState = { byKey: {} };
|
||||
|
||||
export function register(reg: ModalRegistration) {
|
||||
state.byKey[reg.id] = reg;
|
||||
}
|
||||
|
||||
export const ModalRegistration = ({ id }: { id: string }) => {
|
||||
register({ id, priority: 0 });
|
||||
return null;
|
||||
};
|
||||
|
||||
export function Host() {
|
||||
return ModalRegistration({ id: "host" });
|
||||
}
|
||||
`,
|
||||
// A type and a value of the same name, the value wrapping a component.
|
||||
'app/scenes/Toolbar.tsx': `import { observer } from "mobx-react";
|
||||
|
||||
type Toolbar = {
|
||||
items: string[];
|
||||
};
|
||||
|
||||
type Props = {
|
||||
toolbar: Toolbar;
|
||||
};
|
||||
|
||||
function ToolbarView({ toolbar }: Props) {
|
||||
return <div>{toolbar.items.length}</div>;
|
||||
}
|
||||
|
||||
const Toolbar = observer(ToolbarView);
|
||||
|
||||
export default Toolbar;
|
||||
`,
|
||||
// A component's own type parameter, named like a component elsewhere.
|
||||
'app/components/ui/table/table.tsx': `type TableProps<Entry> = {
|
||||
data: Entry[];
|
||||
};
|
||||
|
||||
export const Table = <Entry extends { id: string }>({ data }: TableProps<Entry>) => {
|
||||
return <table>{data.length}</table>;
|
||||
};
|
||||
`,
|
||||
'app/editor/components/ModeToggle.tsx': `enum Mode {
|
||||
Edit = "edit",
|
||||
Read = "read",
|
||||
}
|
||||
|
||||
type Props = {
|
||||
mode: Mode;
|
||||
};
|
||||
|
||||
export default function ModeToggle({ mode }: Props) {
|
||||
return <button>{mode === Mode.Edit ? "Read" : "Edit"}</button>;
|
||||
}
|
||||
`,
|
||||
...NAMESAKES,
|
||||
};
|
||||
|
||||
let cg: CodeGraph;
|
||||
let bare: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
cg = await index(FILES);
|
||||
// The same project without the components folder's namesakes.
|
||||
bare = await index(Object.fromEntries(Object.entries(FILES).filter(([rel]) => !(rel in NAMESAKES))));
|
||||
}, 120_000);
|
||||
|
||||
afterAll(() => {
|
||||
for (const g of graphs) g.close();
|
||||
for (const root of roots) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function node(g: CodeGraph, file: string, name: string, kind?: string): Node {
|
||||
const found = g.getNodesInFile(file).filter((n) => n.name === name && (!kind || n.kind === kind));
|
||||
expect(found, `${kind ?? 'node'} ${name} in ${file}`).toHaveLength(1);
|
||||
return found[0]!;
|
||||
}
|
||||
|
||||
/** Where a node's references named `name` go, `kind:name@file`, and which strategy resolved each. */
|
||||
function referencesFrom(g: CodeGraph, from: Node, name: string, edgeKind = 'references') {
|
||||
return g.getOutgoingEdges(from.id)
|
||||
.filter((e) => e.kind === edgeKind && e.metadata?.refName === name)
|
||||
.map((e) => {
|
||||
const target = g.getNode(e.target)!;
|
||||
return { to: `${target.kind}:${target.name}@${target.filePath}`, resolvedBy: e.metadata?.resolvedBy };
|
||||
});
|
||||
}
|
||||
|
||||
describe('a name a .tsx file declares itself', () => {
|
||||
it('has the namesake components the cases below need', () => {
|
||||
expect(node(cg, 'app/components/Emoji.tsx', 'Emoji').kind).toBe('component');
|
||||
expect(node(cg, 'app/components/Export/ExportDialog.tsx', 'Option').kind).toBe('function');
|
||||
expect(node(cg, 'app/features/explore/components/Story.tsx', 'Story').kind).toBe('function');
|
||||
expect(node(cg, 'app/components/Mode.tsx', 'Mode').kind).toBe('function');
|
||||
expect(node(cg, 'app/pages/profile.tsx', 'Entry').kind).toBe('function');
|
||||
});
|
||||
|
||||
it('is the type alias a type annotation names, not a styled component of that name', () => {
|
||||
const props = node(cg, 'app/editor/components/EmojiMenu.tsx', 'Props', 'type_alias');
|
||||
expect(referencesFrom(cg, props, 'Emoji').map((r) => r.to)).toEqual(['type_alias:Emoji@app/editor/components/EmojiMenu.tsx']);
|
||||
});
|
||||
|
||||
it('is the interface a type annotation names', () => {
|
||||
const props = node(cg, 'app/components/primitives/SplitButton.tsx', 'Props', 'type_alias');
|
||||
expect(referencesFrom(cg, props, 'Option').map((r) => r.to)).toEqual(['interface:Option@app/components/primitives/SplitButton.tsx']);
|
||||
});
|
||||
|
||||
it("is a story file's own Story type", () => {
|
||||
const primary = node(cg, 'app/components/button/button.stories.tsx', 'Primary');
|
||||
expect(referencesFrom(cg, primary, 'Story').map((r) => r.to)).toEqual(['type_alias:Story@app/components/button/button.stories.tsx']);
|
||||
});
|
||||
|
||||
it('is the enum a type annotation names', () => {
|
||||
const props = node(cg, 'app/editor/components/ModeToggle.tsx', 'Props', 'type_alias');
|
||||
expect(referencesFrom(cg, props, 'Mode').map((r) => r.to)).toEqual(['enum:Mode@app/editor/components/ModeToggle.tsx']);
|
||||
});
|
||||
|
||||
it("is a type parameter, never another file's component", () => {
|
||||
const file = 'app/components/ui/table/table.tsx';
|
||||
expect(referencesFrom(cg, node(cg, file, 'Table'), 'Entry')).toEqual([]);
|
||||
expect(referencesFrom(cg, node(cg, file, 'TableProps'), 'Entry')).toEqual([]);
|
||||
});
|
||||
|
||||
// (Name matching's confidence still counts the namesake as a candidate.)
|
||||
it('resolves as it does with no namesake component in the project', () => {
|
||||
const cases: Array<[string, string, string]> = [
|
||||
['app/editor/components/EmojiMenu.tsx', 'Props', 'Emoji'],
|
||||
['app/components/primitives/SplitButton.tsx', 'Props', 'Option'],
|
||||
['app/components/button/button.stories.tsx', 'Primary', 'Story'],
|
||||
['app/editor/components/ModeToggle.tsx', 'Props', 'Mode'],
|
||||
['app/components/ui/table/table.tsx', 'Table', 'Entry'],
|
||||
];
|
||||
for (const [file, from, name] of cases) {
|
||||
expect(referencesFrom(cg, node(cg, file, from), name), `${from} -> ${name}`)
|
||||
.toEqual(referencesFrom(bare, node(bare, file, from), name));
|
||||
}
|
||||
});
|
||||
|
||||
it('is the type, in a type annotation, when the file also declares a component of that name', () => {
|
||||
const file = 'app/core/singletons/modalController.tsx';
|
||||
const type = 'type_alias:ModalRegistration@app/core/singletons/modalController.tsx';
|
||||
expect(referencesFrom(cg, node(cg, file, 'ModalState'), 'ModalRegistration').map((r) => r.to)).toEqual([type]);
|
||||
expect(referencesFrom(cg, node(cg, file, 'register'), 'ModalRegistration').map((r) => r.to)).toEqual([type]);
|
||||
});
|
||||
|
||||
it('is the type, in a type annotation, when the file also declares a value of that name', () => {
|
||||
const file = 'app/scenes/Toolbar.tsx';
|
||||
expect(referencesFrom(cg, node(cg, file, 'Props', 'type_alias'), 'Toolbar').map((r) => r.to))
|
||||
.toEqual(['type_alias:Toolbar@app/scenes/Toolbar.tsx']);
|
||||
});
|
||||
|
||||
it('is the component, where code calls it', () => {
|
||||
const file = 'app/core/singletons/modalController.tsx';
|
||||
expect(referencesFrom(cg, node(cg, file, 'Host'), 'ModalRegistration', 'calls').map((r) => r.to))
|
||||
.toEqual(['function:ModalRegistration@app/core/singletons/modalController.tsx']);
|
||||
});
|
||||
});
|
||||
@@ -84,16 +84,39 @@ export const reactResolver: FrameworkResolver = {
|
||||
isPascalCase(ref.referenceName) &&
|
||||
!isBuiltInType(ref.referenceName)
|
||||
) {
|
||||
// A name the file declares itself is that declaration, whatever refers
|
||||
// to it — a route's `element`, a page's default export, a type: the
|
||||
// module a `const RegisterPage = Loadable(lazy(() =>
|
||||
// import('pages/auth/Register')))` loads, the function a value wraps, or
|
||||
// else the declaration, which name matching binds — never a same-named
|
||||
// component in another app of the repository.
|
||||
const own = declaredComponent(ref.referenceName, ref.filePath, context);
|
||||
if (own) {
|
||||
return own.component ? { original: ref, targetNodeId: own.component.id, confidence: 0.9, resolvedBy: 'framework' } : null;
|
||||
// A type annotation is name matching's, which scopes a type as
|
||||
// TypeScript does — the file's own, an import's or a global one — and
|
||||
// never a component elsewhere: component lookup handed the `Option`
|
||||
// annotations of outline's InputSelect.tsx, which declares `type
|
||||
// Option`, to the `Option` component of Export/ExportDialog.tsx, and
|
||||
// mastodon's `<Item>` type parameters to the `Item` class of
|
||||
// media_gallery.jsx. Only a type alias or interface the file declares
|
||||
// beside a value or a component of its name is decided here, where both
|
||||
// are the file's own: the annotation is the type, as bigbluebutton's
|
||||
// `type ModalRegistration` is beside its render-prop component `const
|
||||
// ModalRegistration = (…) =>`. (A class is a type and a value at once.)
|
||||
const own = ownDeclarations(ref.referenceName, ref.filePath, context);
|
||||
if (namesType(ref)) {
|
||||
const type = own.find((n) => n.kind === 'type_alias' || n.kind === 'interface');
|
||||
return type && own.some((n) => n.kind !== 'type_alias' && n.kind !== 'interface' && n.kind !== 'class')
|
||||
? { original: ref, targetNodeId: type.id, confidence: 0.8, resolvedBy: 'framework' } : null;
|
||||
}
|
||||
// Anything else the file declares itself is that declaration, whatever
|
||||
// refers to it — a route's `element`, a page's default export — never a
|
||||
// same-named component in another app of the repository: the module a
|
||||
// `const RegisterPage = Loadable(lazy(() =>
|
||||
// import('pages/auth/Register')))` loads, the function a value wraps, a
|
||||
// function, class or component of that name, or else the declaration,
|
||||
// which name matching binds.
|
||||
const value = declaredComponent(ref.referenceName, ref.filePath, context);
|
||||
if (value) {
|
||||
return value.component ? { original: ref, targetNodeId: value.component.id, confidence: 0.9, resolvedBy: 'framework' } : null;
|
||||
}
|
||||
const component = own.find((n) => COMPONENT_KINDS.has(n.kind));
|
||||
if (component) {
|
||||
return { original: ref, targetNodeId: component.id, confidence: 0.8, resolvedBy: 'framework' };
|
||||
}
|
||||
if (own.length > 0) return null;
|
||||
const result = resolveComponent(ref.referenceName, ref.filePath, context);
|
||||
if (result) {
|
||||
return {
|
||||
@@ -2344,6 +2367,28 @@ const BUILT_IN_TYPES = new Set([
|
||||
|
||||
const COMPONENT_KINDS = new Set(['component', 'function', 'class']);
|
||||
|
||||
/**
|
||||
* What `filePath` declares at its top level under `name`: a type alias, an
|
||||
* interface, a class, an enum, a function, a component or a value. A
|
||||
* declaration nested in another (`EmojiMenu::renderMenuItem`) carries its
|
||||
* owner in its qualified name; an import is no declaration.
|
||||
*/
|
||||
function ownDeclarations(name: string, filePath: string, context: ResolutionContext): Node[] {
|
||||
return (context.getNodesInFileNamed?.(filePath, name) ?? context.getNodesInFile(filePath).filter((n) => n.name === name))
|
||||
.filter((n) => n.qualifiedName === name && n.kind !== 'import' && n.kind !== 'export' && n.kind !== 'file');
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a reference is written where TypeScript reads a type: what a class
|
||||
* implements, or a type annotation's name — the extractor writes a
|
||||
* `references` ref only for a name in a type, and a route's reference to what
|
||||
* it renders comes from the router's own extraction.
|
||||
*/
|
||||
function namesType(ref: UnresolvedRef): boolean {
|
||||
return ref.referenceKind === 'implements' ||
|
||||
(ref.referenceKind === 'references' && !ref.fromNodeId.startsWith('route:'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a component reference using name-based lookup
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user