Skip to content

fix(resolution): same-dir component preference must compare paths at a directory boundary#683

Open
Mubashirrrr wants to merge 1 commit into
colbymchenry:mainfrom
Mubashirrrr:fix/component-samedir-prefix-boundary
Open

fix(resolution): same-dir component preference must compare paths at a directory boundary#683
Mubashirrrr wants to merge 1 commit into
colbymchenry:mainfrom
Mubashirrrr:fix/component-samedir-prefix-boundary

Conversation

@Mubashirrrr
Copy link
Copy Markdown

The bug

The React and Svelte framework resolvers disambiguate same-named components by preferring the one in the referencing file's directory:

// src/resolution/frameworks/react.ts  (and svelte.ts)
const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/'));
const sameDir = components.filter((n) => n.filePath.startsWith(fromDir));
if (sameDir.length > 0) return sameDir[0]!.id;

startsWith(fromDir) matches on a raw string prefix with no path-separator boundary. So a component in a sibling directory whose name merely shares a prefix is wrongly treated as "same directory":

  • referencing file: src/ui/Card.tsxfromDir = "src/ui"
  • candidates named Widget: src/ui/Widget.tsx (the true same-dir one) and src/ui-kit/Widget.tsx (a prefix sibling)
  • "src/ui-kit/Widget.tsx".startsWith("src/ui") is true, so src/ui-kit/Widget.tsx lands in sameDir and — depending on candidate order — can be returned instead of the real src/ui/Widget.tsx.

The result is a wrong references edge to a different component in a prefix-sibling directory. This is the same data-integrity hazard the workspace-package matcher already guards against with importPath === name || importPath.startsWith(name + '/').

There's also a secondary edge: for a root-level file like App.tsx (no /), lastIndexOf('/') is -1 and substring(0, -1) returns '', so startsWith('') matches every file — silently defeating the "same directory" preference entirely.

Repro (fail-before / pass-after)

Added __tests__/framework-samedir-boundary.test.ts. It drives the public reactResolver.resolve() / svelteResolver.resolve() with two same-named components — the true same-dir one and a prefix-sibling decoy listed first — and asserts the resolver picks the same-directory component.

Before this change:

× does not treat a prefix-sibling directory as the same directory
  → expected 'id-uikit' to be 'id-ui'
× Svelte: prefix-sibling directory is not the same directory
  → expected 'sv-uikit' to be 'sv-ui'

After: all 3 pass. A sanity case (a genuine same-directory component listed after an unrelated one) confirms the same-dir preference itself still works.

The fix

Replace the boundary-unsafe startsWith with an exact directory comparison via a small dirOf helper (which also returns '' cleanly for root-level files):

function dirOf(filePath: string): string {
  const slash = filePath.lastIndexOf('/');
  return slash >= 0 ? filePath.slice(0, slash) : '';
}
// ...
const fromDir = dirOf(fromFile);
const sameDir = components.filter((n) => dirOf(n.filePath) === fromDir);

Surgical: +34/-6 across the two affected resolvers, no behavior change beyond fixing the wrong-directory match.

Scope notes

  • src/resolution/frameworks/vue.ts has the same startsWith(fromDir) idiom, but its loop additionally gates on componentName === name (filename basename equals the reference name), so the directory filter there is effectively redundant for the common case and I couldn't construct a realistic wrong-result repro without a contrived filename/component-name mismatch — left untouched rather than ship a speculative change. Happy to follow up if you'd prefer it normalized for consistency.
  • Verified no regressions: frameworks, frameworks-integration, resolution, and react-native-bridge suites (184 tests) all pass, and tsc --noEmit is clean. (The MCP-daemon / status-json CLI-spawn suites fail identically on main in my environment — pre-existing, unrelated.)

The React and Svelte framework resolvers disambiguate same-named
components by preferring one in the referencing file's directory:

  const fromDir = fromFile.substring(0, fromFile.lastIndexOf('/'));
  const sameDir = components.filter((n) => n.filePath.startsWith(fromDir));

`startsWith(fromDir)` has no trailing-separator boundary, so a component
in a SIBLING directory whose name merely shares a prefix (`src/ui-kit/`
vs `src/ui/`) is treated as same-directory and can win the tie — the
reference resolves to the wrong component (a wrong graph edge).

Compare the directory portions exactly instead. A small `dirOf` helper
returns the directory (or '' for a root-level file, fixing the
`lastIndexOf('/') === -1` edge where `substring(0, -1)` yielded '' and
made `startsWith('')` match every file).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant