From 8bd55fa75ce6e3118bc902c440257b32d4c2f1aa Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Thu, 29 Oct 2026 19:30:39 +0000 Subject: [PATCH] fix(frameworks): a component's same-folder preference compares whole folders (#3020) The React, Svelte, Astro and Vue component resolvers each picked among a name's definitions by preferring one whose path started with the referencing file's folder, compared as text with no separator: `src/ui/Card.tsx`'s `Widget` took `src/ui-kit/Widget.tsx`, since `src/ui-kit/...` starts with `src/ui`. A file at the project root has the folder `''`, which every path starts with, so its pick was whichever definition came first, skipping the rule that a name with several definitions and nothing to choose by links nothing (#764). One helper, inReferencingFolder (frameworks/component-folder.ts), now serves all four copies: a candidate counts when its path starts with the folder plus `/`, so a folder below the referencing file's still counts, and a root file has no folder to prefer. Validation: new component-same-folder tests (7) fail 6 of 7 on main, one per resolver plus the root and subfolder cases; the helper's unit test runs on the new module in both. A/B against main: outline, mastodon (app/javascript), excalidraw, vben, shadcn-svelte (docs/src) and gin are byte-identical, since real code imports its components and the framework pick runs only for a name the file doesn't import. Reimplements PR #683 by @Mubashirrrr. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/component-same-folder.test.ts | 133 ++++++++++++++++++ src/resolution/frameworks/astro.ts | 6 +- src/resolution/frameworks/component-folder.ts | 26 ++++ src/resolution/frameworks/react.ts | 6 +- src/resolution/frameworks/svelte.ts | 6 +- src/resolution/frameworks/vue.ts | 6 +- 7 files changed, 172 insertions(+), 12 deletions(-) create mode 100644 __tests__/component-same-folder.test.ts create mode 100644 src/resolution/frameworks/component-folder.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e300b8cd..955f1ae9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -294,6 +294,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - React Router routes are now read from every table a route-building function returns, an early `if (!user) return [];` and each branch alike, and never from a list that a function written inside it returns, like a `.forEach` callback's `return []` or a helper's menu. A function that returns another's table, as in `return buildRoutes()`, is followed too, and so is a table behind `useMemo`, like `useMemo(() => getRoutes(user), [user])` or `useMemo(getRoutes, [])`. Before, the first `return [` anywhere in the function was taken for the table, so an empty guard or a callback's list could hide an app's routes entirely, or a menu could be read in their place. Re-index React projects after upgrading. - In JavaScript and TypeScript, an import written `import { default as AppRoot } from './routes/app/root'`, as in bulletproof-react's router, now counts as the module's default import, the same as `import AppRoot from './routes/app/root'`. Before, it was read as an import of an export named `default`, which no module has, so a call, a route or a JSX attribute that used `AppRoot` was matched by its name alone: it could link to another file's `AppRoot`, or to nothing when the default export has a name of its own. Svelte, Vue and Astro script blocks are read the same way. - In React, a component's JSX no longer links it to a class or component that only shares a name with one of its type arguments or with a variable of its own. A type in angle brackets, like `Document` in ` items={…} />`, `User` in `useState()` or `Entry` in a generic `(…) =>` component, is no longer read as a tag. A tag naming a variable or parameter the component sets itself, like `` after `const Content = isDropdown ? DropdownMenu.SubContent : ContextMenu.SubContent` or `` in `widgets.map((Widget) => )`, now links to nothing, unless the component declares a component of that name inside itself. Before, these linked to an unrelated class or component elsewhere in the repository: outline's document lists showed up among the callers of its `Document` model class, and its menus among the callers of the command bar's `Content`, so `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading. +- In React, Svelte, Astro and Vue, a component name used without an import, with several definitions in the project, now prefers one in the using file's own folder or a folder below it, compared folder by folder. Before, a folder whose name only starts the same way counted too, so `Widget` in `src/ui/Card.tsx` could link to `src/ui-kit/Widget.tsx` instead of `src/ui/Widget.tsx`, and a file at the project root, like `App.tsx`, linked to whichever definition came first instead of to none. Re-index affected projects after upgrading. Thanks @Mubashirrrr. - In React, a tag for a component a function destructures from its props or from an object, like `({ Header, Footer, children }) =>` in refine's layouts or `const { Sidebar, Footer } = excalidrawLib` in excalidraw's example app, now links to nothing, as a tag naming a variable the function sets itself already did, unless the function declares a component of that name inside itself. A function nested in the one that sets or destructures the name, like a `renderSidebar` helper inside the component, reads the tag the same way. A name destructured from `require(…)` or `import(…)` still links to that module's export. Before, these tags were matched by their name alone, so refine's layouts showed up among the callers of other examples' `Header` and `Footer` components, and `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading. - In React, a tag for a component a file destructures at its top, like `const { Title, Text } = Typography;` with antd's `Typography`, as in refine's pages, now links to nothing in every component of that file, unless the component declares one of that name inside itself. A name destructured from `require(…)` there still links to that module's export. Before, these tags were matched by their name alone, so refine's pages showed up among the callers of other examples' `Title` and `Text` components. Re-index React projects after upgrading. - In React, a tag a component only mentions in a comment or a string no longer links it to that component: a JSDoc example, markup commented out like `{/* */}`, or an error message like react-router's "You cannot render a inside another " or shadcn's "useCarousel must be used within a ". Nothing in a TypeScript `.ts` file, which can't hold JSX, is read as a tag either, like the XML in outline's OpenSearch template. Before, these counted as renders, so react-router's hooks showed up among the callers of `Router`, `Routes` and `Route`, and `codegraph_explore`, callers and impact followed renders that never happen. A tag after an apostrophe, a URL or a backtick in a component's text, like `

Don't have an account? `, still links as before. Re-index React projects after upgrading. diff --git a/__tests__/component-same-folder.test.ts b/__tests__/component-same-folder.test.ts new file mode 100644 index 000000000..c5413c886 --- /dev/null +++ b/__tests__/component-same-folder.test.ts @@ -0,0 +1,133 @@ +/** + * The folder preference of the React, Svelte, Astro and Vue component + * resolvers, which pick among several definitions of a component name. + * + * They preferred a definition whose path started with the referencing file's + * folder, compared as text: `src/ui` also starts `src/ui-kit/Widget.tsx`, so + * `src/ui/Card.tsx`'s `Widget` could land in the sibling folder. A file at the + * project root has the folder `''`, which every path starts with, so its pick + * was whatever definition came first, skipping the rule that a name with + * several definitions and nothing to tell them apart links nothing (#764). + * + * The folder is now compared whole, a folder below it still counts, and a root + * file has no folder to prefer. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { inReferencingFolder } from '../src/resolution/frameworks/component-folder'; + +let dirs: string[] = []; +let graphs: CodeGraph[] = []; +afterEach(() => { + for (const graph of graphs) graph.close(); + for (const dir of dirs) fs.rmSync(dir, { recursive: true, force: true }); + graphs = []; + dirs = []; +}); + +async function index(files: Record): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-component-folder-')); + dirs.push(dir); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), content); + } + const cg = await CodeGraph.init(dir, { index: true }); + graphs.push(cg); + return cg; +} + +/** Every edge the framework resolvers made into a node named `name`, as `file` of the target. */ +function frameworkPicks(cg: CodeGraph, name: string): string[] { + return cg + .getNodesByName(name) + .flatMap((target) => + cg + .getIncomingEdges(target.id) + .filter((e) => (e.metadata as { resolvedBy?: string } | undefined)?.resolvedBy === 'framework') + .map(() => target.filePath) + ) + .sort(); +} + +const REACT = JSON.stringify({ name: 'app', dependencies: { react: '^18.0.0' } }); +const SVELTE = JSON.stringify({ name: 'app', devDependencies: { svelte: '^4.0.0' } }); +const ASTRO = JSON.stringify({ name: 'app', dependencies: { astro: '^4.0.0' } }); +const VUE = JSON.stringify({ name: 'app', dependencies: { vue: '^3.0.0' } }); +const reactWidget = 'export function Widget(props: {}) { return null; }\n'; + +describe('the folder a component resolver prefers', () => { + it('compares folders whole, keeps folders below, and gives a root file none', () => { + const files = ['src/ui-kit/Widget.tsx', 'src/ui/forms/Widget.tsx', 'src/ui/Widget.tsx', 'Widget.tsx']; + expect(inReferencingFolder('src/ui/Card.tsx', files, (f) => f)).toEqual(['src/ui/forms/Widget.tsx', 'src/ui/Widget.tsx']); + expect(inReferencingFolder('App.tsx', files, (f) => f)).toEqual([]); + }); + + it('React: a sibling folder that only starts with the same name is not the folder', async () => { + const cg = await index({ + 'package.json': REACT, + 'src/ui/Card.tsx': 'export function Card() {\n return Widget({});\n}\n', + 'src/ui-kit/Widget.tsx': reactWidget, + 'lib/Widget.tsx': reactWidget, + }); + // Two definitions, neither in `src/ui/`: the framework has nothing to choose by. + expect(frameworkPicks(cg, 'Widget')).toEqual([]); + }); + + it('React: the folder itself, and a folder below it, still come first', async () => { + const cg = await index({ + 'package.json': REACT, + 'src/ui/Card.tsx': 'export function Card() {\n return Widget({});\n}\n', + 'src/ui/Banner.tsx': 'export function Banner() {\n return Field({});\n}\n', + 'src/ui-kit/Widget.tsx': reactWidget, + 'src/ui/Widget.tsx': reactWidget, + 'src/ui/forms/Field.tsx': 'export function Field(props: {}) { return null; }\n', + 'lib/Field.tsx': 'export function Field(props: {}) { return null; }\n', + }); + expect(frameworkPicks(cg, 'Widget')).toEqual(['src/ui/Widget.tsx']); + expect(frameworkPicks(cg, 'Field')).toEqual(['src/ui/forms/Field.tsx']); + }); + + it('React: a file at the project root does not take whichever definition comes first', async () => { + const cg = await index({ + 'package.json': REACT, + 'App.tsx': 'export function App() {\n return Panel({});\n}\n', + 'src/a/Panel.tsx': 'export function Panel(props: {}) { return null; }\n', + 'src/b/Panel.tsx': 'export function Panel(props: {}) { return null; }\n', + }); + expect(frameworkPicks(cg, 'Panel')).toEqual([]); + }); + + it('Svelte: a sibling folder that only starts with the same name is not the folder', async () => { + const cg = await index({ + 'package.json': SVELTE, + 'src/ui/mount.ts': 'export function mount() {\n return Widget({});\n}\n', + 'src/ui-kit/Widget.svelte': 'kit\n', + 'lib/Widget.svelte': 'lib\n', + }); + expect(frameworkPicks(cg, 'Widget')).toEqual([]); + }); + + it('Astro: a sibling folder that only starts with the same name is not the folder', async () => { + const cg = await index({ + 'package.json': ASTRO, + 'src/ui/Card.astro': '---\n---\n

\n \n
\n', + 'src/ui-kit/Widget.astro': '---\n---\nkit\n', + 'lib/Widget.astro': '---\n---\nlib\n', + }); + expect(frameworkPicks(cg, 'Widget')).toEqual([]); + }); + + it('Vue: a sibling folder that only starts with the same name is not the folder', async () => { + const cg = await index({ + 'package.json': VUE, + 'src/ui/mount.ts': 'export function mount() {\n return Widget({});\n}\n', + 'src/ui-kit/Widget.vue': '\n', + 'lib/Widget.vue': '\n', + }); + expect(frameworkPicks(cg, 'Widget')).toEqual([]); + }); +}); diff --git a/src/resolution/frameworks/astro.ts b/src/resolution/frameworks/astro.ts index 635d19e97..fc4f99a09 100644 --- a/src/resolution/frameworks/astro.ts +++ b/src/resolution/frameworks/astro.ts @@ -8,6 +8,7 @@ import { Node } from '../../types'; import { FrameworkResolver, UnresolvedRef, ResolvedRef, ResolutionContext } from '../types'; import { pageComponentRef, resolvePageComponent } from './page-component'; +import { inReferencingFolder } from './component-folder'; /** * Astro virtual module prefixes — framework-provided, not user code @@ -182,9 +183,8 @@ function resolveComponent( if (components.length === 0) return null; - // Prefer same directory - const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/')); - const sameDir = components.filter((n) => n.filePath.startsWith(fromDir)); + // Prefer the referencing file's folder, or one below it + const sameDir = inReferencingFolder(fromFile, components, (n) => n.filePath); if (sameDir.length > 0) return sameDir[0]!.id; // No positional signal: only an UNAMBIGUOUS name may resolve — picking diff --git a/src/resolution/frameworks/component-folder.ts b/src/resolution/frameworks/component-folder.ts new file mode 100644 index 000000000..ff9031683 --- /dev/null +++ b/src/resolution/frameworks/component-folder.ts @@ -0,0 +1,26 @@ +/** + * The folder preference the component resolvers share (React, Svelte, Astro, + * Vue): when a component name has several definitions, the ones in the folder + * of the file that uses the name, or in a folder below it, come first. + * + * The folder is compared whole. A bare `startsWith('src/ui')` also held + * `src/ui-kit/Widget.tsx`, so `src/ui/Card.tsx`'s `Widget` could land in the + * sibling folder; and a file at the project root, whose folder is `''`, held + * every definition in the project, so its pick skipped the "only one + * definition may resolve" rule that keeps an ambiguous name unlinked (#764). + */ + +/** + * The candidates in `fromFile`'s folder or a folder below it, in their given + * order. A file at the project root has no folder to prefer, so it gets none. + */ +export function inReferencingFolder( + fromFile: string, + candidates: readonly T[], + pathOf: (candidate: T) => string +): T[] { + const cut = fromFile.lastIndexOf('/'); + if (cut < 0) return []; + const folder = fromFile.slice(0, cut + 1); + return candidates.filter((candidate) => pathOf(candidate).startsWith(folder)); +} diff --git a/src/resolution/frameworks/react.ts b/src/resolution/frameworks/react.ts index b46e051f8..780fbcb2e 100644 --- a/src/resolution/frameworks/react.ts +++ b/src/resolution/frameworks/react.ts @@ -15,6 +15,7 @@ import { escapeRegExp } from '../../utils'; import { FN_KINDS, makeLineAt } from '../synth-utils'; import { isTestPath } from '../../search/query-utils'; import { matchBracket, skipString } from './object-literal'; +import { inReferencingFolder } from './component-folder'; /** The languages React components, hooks and contexts are written and used in. */ const REACT_SCRIPT_LANGUAGES: ReadonlySet = new Set(['typescript', 'javascript', 'tsx', 'jsx']); @@ -2425,9 +2426,8 @@ function resolveComponent( const components = candidates.filter((n) => COMPONENT_KINDS.has(n.kind)); if (components.length === 0) return null; - // Prefer same directory - const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/')); - const sameDir = components.filter((n) => n.filePath.startsWith(fromDir)); + // Prefer the referencing file's folder, or one below it + const sameDir = inReferencingFolder(fromFile, components, (n) => n.filePath); if (sameDir.length > 0) return sameDir[0]!.id; // Prefer component directories diff --git a/src/resolution/frameworks/svelte.ts b/src/resolution/frameworks/svelte.ts index 228c2248a..3675f2f9c 100644 --- a/src/resolution/frameworks/svelte.ts +++ b/src/resolution/frameworks/svelte.ts @@ -7,6 +7,7 @@ import { Node } from '../../types'; import { FrameworkResolver, UnresolvedRef, ResolvedRef, ResolutionContext } from '../types'; +import { inReferencingFolder } from './component-folder'; /** * Svelte 5 runes — compiler-provided, not user code @@ -225,9 +226,8 @@ function resolveComponent( if (components.length === 0) return null; - // Prefer same directory - const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/')); - const sameDir = components.filter((n) => n.filePath.startsWith(fromDir)); + // Prefer the referencing file's folder, or one below it + const sameDir = inReferencingFolder(fromFile, components, (n) => n.filePath); if (sameDir.length > 0) return sameDir[0]!.id; // No positional signal: only an UNAMBIGUOUS name may resolve — picking diff --git a/src/resolution/frameworks/vue.ts b/src/resolution/frameworks/vue.ts index 257c536fd..79e130285 100644 --- a/src/resolution/frameworks/vue.ts +++ b/src/resolution/frameworks/vue.ts @@ -10,6 +10,7 @@ import { FrameworkResolver, UnresolvedRef, ResolvedRef, ResolutionContext } from import { isJsPathImportRef, resolveImportPath } from '../import-resolver'; import { dependsOn } from './package-deps'; import { pageComponentRef, resolvePageComponent } from './page-component'; +import { inReferencingFolder } from './component-folder'; /** The languages a Vue app's scripts are written in. */ const VUE_SCRIPT_LANGUAGES: ReadonlySet = new Set(['vue', 'javascript', 'typescript', 'tsx', 'jsx']); @@ -362,9 +363,8 @@ function resolveComponent( return component ? component.id : null; }; - // Same directory first for specificity - const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/')); - const sameDir = matches.filter((f) => f.startsWith(fromDir)); + // The referencing file's folder, or one below it, first for specificity + const sameDir = inReferencingFolder(fromFile, matches, (f) => f); if (sameDir.length > 0) return componentIn(sameDir[0]!); // No positional signal: only an UNAMBIGUOUS basename may resolve;