Skip to content

fix: resolve jsconfig.json path aliases, not just tsconfig.json - #693

Open
azlanali wants to merge 1 commit into
Egonex-AI:mainfrom
azlanali:fix/jsconfig-path-aliases
Open

azlanali wants to merge 1 commit into
Egonex-AI:mainfrom
azlanali:fix/jsconfig-path-aliases

Conversation

@azlanali

Copy link
Copy Markdown

The problem

loadTsConfigs() discovers alias configs by matching the literal filename:

if (base !== 'tsconfig.json') continue;

JavaScript-only projects declare the very same compilerOptions.paths in
jsconfig.json — it is what create-next-app writes when you decline
TypeScript. Those projects therefore get no aliases at all, so every
@/... import resolves to nothing and the knowledge graph loses its
app/ → lib/ edges.

The failure is quiet, which is the worst part: nothing errors, no warning is
emitted, and the dashboard renders a confident graph in which the application
layer and the library layer look almost disconnected. For a codebase whose
whole architecture is "pure logic in lib/, thin shell in app/", that is
the single most misleading thing the graph could say.

The alias machinery itself (matchTsAlias / applyTsAlias) was already
correct — only the discovery filter was wrong.

The fix

Accept both basenames. When one directory holds both, tsconfig.json wins: a
project carrying both is a TypeScript project whose jsconfig.json is
vestigial.

The reads go through readFilesParallel, so the winner is tracked explicitly
(wonBy) rather than left to whichever promise settles last — otherwise the
same repository could produce a different graph run to run.

Warning text and the resolver-config-parse failure message now name the file
actually read, instead of always saying "tsconfig.json". Existing tests assert
on failure path and stage rather than that message, so they are unaffected
(checked test_extract_import_map and test_prepare_incremental).

Impact

Measured on a real JS-only Next.js app (161 files, App Router + Supabase):

before after
resolved import edges 54 196
files with resolved imports 42 / 161 85 / 161

Corroborating detail: in that run the file-analyzer agents had independently
hand-rolled depends_on edges to paper over the missing imports. After the
fix, 74 of those 87 edges landed on exactly the pairs the resolver now
recovers deterministically.

Tests

Two added to tests/skill/understand/test_extract_import_map.test.mjs:

  • resolves jsconfig.json paths aliases in a JS-only project — verified to
    fail without this change and pass with it.
  • prefers tsconfig.json over jsconfig.json when a directory holds both —
    guards the precedence rule against parallel-read nondeterminism. This one
    passes either way by construction (with no jsconfig support the jsconfig-only
    alias is ignored anyway); it exists to protect the new wonBy logic.

test_extract_import_map + test_prepare_incremental: 135 passed.
pnpm lint clean.

🤖 Generated with Claude Code

loadTsConfigs() gated on the literal filename "tsconfig.json", so a
JavaScript-only project that declares its aliases in jsconfig.json got no
aliases at all. `create-next-app` writes jsconfig.json when you decline
TypeScript, with the identical compilerOptions.paths, so every `@/...`
import in such a project silently resolved to nothing and the graph lost
its app -> lib edges. The alias machinery (matchTsAlias/applyTsAlias) was
already correct; only the discovery filter was wrong.

Accept both basenames. When one directory holds both, tsconfig.json wins:
a project carrying both is a TypeScript project whose jsconfig is
vestigial. The reads run in parallel, so the winner is tracked explicitly
rather than left to whichever promise settles last -- otherwise the same
repo could produce different graphs run to run.

Warning text and the resolver-config-parse failure message now name the
file actually read instead of always saying "tsconfig.json". Existing
tests assert on failure `path` and `stage`, not that message, so they are
unaffected.

Measured on a real JS-only Next.js app (161 files): resolved import edges
54 -> 196, files with resolved imports 42 -> 85.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@turovskiy

Copy link
Copy Markdown

Independent confirmation from a second JS-only Next.js App Router project: @/* → ./src/* declared in jsconfig.json, no tsconfig.json, 479 files in scope after .understandignore.

main (6df3065) this PR (c38968d)
resolved import edges 166 787
files with resolved imports 86 / 479 271 / 479

Spot checks against git grep:

  • a shared data-fetching module with 10 direct importers: 1 found on main, 10 with this PR
  • a request module imported by 21 files: 0 on main, 21 with this PR

I also applied the one-line filename change on its own before finding this PR. It produced a byte-identical import map on this project, which is expected since there is no tsconfig.json for the precedence logic to act on.

Tests on this branch: test_extract_import_map and test_prepare_incremental, 135 in total, all passing. One prepare_incremental case failed once inside setupRepository while both files ran in parallel, then passed on a rerun (66/66). That looks like load-related flakiness, not something this change touches.

+1 for merging.

This branch has not been deployed

No deployments
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.

2 participants