Skip to content

fix: apply ESM .js extension fallback to tsconfig path alias resolution#1530

Merged
magyargergo merged 22 commits into
abhigyanpatwari:mainfrom
chouzz:fix/esm-path-alias-js-extension
May 14, 2026
Merged

fix: apply ESM .js extension fallback to tsconfig path alias resolution#1530
magyargergo merged 22 commits into
abhigyanpatwari:mainfrom
chouzz:fix/esm-path-alias-js-extension

Conversation

@chouzz

@chouzz chouzz commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1528 — Path alias imports with .js extensions now correctly resolve to .ts source files.

Problem

import { foo } from '@/utils.js' (via tsconfig path aliases) failed to resolve because the alias branch in resolveImportPath called tryResolveWithExtensions without the stripJsExtension retry logic that was already applied to relative imports in #1525.

Fix

Apply the same stripJsExtension + tryResolveWithExtensions fallback in the tsconfig alias branch after the initial direct resolution fails. Also removed the now-outdated NOTE comment about this being a known limitation.

Testing

  • 5 new unit tests covering: alias + .js.ts, alias + .jsx.tsx, real .js wins over fallback, alias with baseUrl prefix, and null for missing files.
  • All existing tests continue to pass (25 unit + 2 integration).
  • tsc --noEmit passes.

Path alias imports (e.g. `@/utils.js` via tsconfig paths) now correctly
strip JS-family extensions and retry with TS equivalents when the literal
.js file does not exist. This applies the same stripJsExtension fallback
already used for relative imports to the alias resolution branch.

Fixes abhigyanpatwari#1528
@vercel

vercel Bot commented May 12, 2026

Copy link
Copy Markdown

@chouzz is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

✨ PR Autofix

Found fixable formatting / unused-import issues across 19 changed lines. Comment /autofix on this PR to apply them, or run npm run lint:fix && npm run format locally.

{"schema":"gitnexus.pr-autofix/v2","state":"fixes-available","pr_number":1530,"changed_lines":19,"head_sha":"13049de0e77d1b24640acf604adfef51808c4d55","run_id":"25747404292","apply_command":"/autofix"}

@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
8993 8992 0 1 434s

✅ All 8992 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 78.18% 28412/36340 N/A% 🟢 ███████████████░░░░░
Branches 66.54% 18013/27067 N/A% 🟢 █████████████░░░░░░░
Functions 83.02% 2856/3440 N/A% 🟢 ████████████████░░░░
Lines 81.47% 25660/31495 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@magyargergo

Copy link
Copy Markdown
Collaborator

/autofix

@github-actions

Copy link
Copy Markdown
Contributor

✅ Applied autofix and pushed a commit. (apply run)

@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 4m 53s —— View job


PR #1530 Production-Readiness Review

Tasks

  • Post initial comment
  • Read repo guidance docs (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, ARCHITECTURE.md, DoD.md, TESTING.md)
  • Inspect changed files (standard.ts, esm-extension-resolution.test.ts)
  • Inspect related files (utils.ts, types.ts, language-config.ts)
  • Run targeted searches (resolveImportPath, tryResolveWithExtensions, stripJsExtension, TsconfigPaths, suffixResolve, etc.)
  • Check CI status and branch diff
  • Run hidden Unicode hygiene checks
  • Write comprehensive review

Review bar for this PR

This PR touches resolveImportPath — the central dispatch function for all import edge generation across all supported languages. Import resolution correctness directly determines graph edge accuracy, which determines impact analysis, process tracing, dependency edges, and downstream agent context. A false positive creates a spurious IMPORTS edge to the wrong file; a false negative silently drops an edge that should exist. Both are production-impacting graph corruptions. The DoD (§2.7) requires that tests cover the real changed path and would fail if the behavior or wiring broke. CI must be green on all required platforms before merge (DoD §4.2). The scope of the change must be limited to the stated intent — the merge-heavy commit history requires explicit verification.


Current PR state

Item Status
Branch chouzz:fix/esm-path-alias-js-extension
Head (local checkout) 8b0c410 (merge commit: Merge branch 'main' into fix/esm-path-alias-js-extension)
PR-only commits f335069 (original fix), b6a1fe1 (autofix)
Changed files (net) 2 filesstandard.ts (+10/-3), esm-extension-resolution.test.ts (+58/-0 after autofix)
CI — Dependency Review ✅ success
CI — Gitleaks ✅ success
CI — CodeQL ✅ success
CI — PR Autofix ✅ success (clean after /autofix)
CI — Main CI (tests/typecheck/lint) queued — not yet complete for head 8b0c410
CI — Docker Build & Push ⏳ in_progress
GitHub UI completeness Incomplete — CI not yet finished for final head

The previous CI run (referenced in the CI report comment, run_id 25783775950) showed "Some checks failed" — attributable to the formatting issues that /autofix subsequently fixed. That run is stale. The current run triggered at 2026-05-13T10:53:29Z by the latest merge-from-main commit is still queued and has not produced a result for the required test and quality checks.


Branch hygiene assessment

Verdict: merge-from-main commit present but harmless and merge-safe.

The PR-only net diff is exactly 2 files. Confirmed by inspecting git show f335069 --stat (the original fix commit) and git show b6a1fe1 --stat (autofix). All other changed files in the branch history (wiki/generator.ts, mermaid-sanitizer.ts, hooks/, package-lock.json, etc.) arrive exclusively via Merge branch 'main' into fix/esm-path-alias-js-extension commits — they are not net additions to the PR diff. The merge commits merely keep the branch current with main and do not pollute the intended change. A squash merge or merge commit would produce a clean 2-file PR diff.


Understanding of the change

Before this PR: resolveImportPath had two separate ESM-fallback code paths:

  1. Relative imports (importPath.startsWith('.')) — after direct resolution failed, the function called stripJsExtension(basePath) and retried tryResolveWithExtensions. Added in fix: resolve TypeScript ESM .js extension imports to .ts source files #1525.
  2. tsconfig alias branch — after direct resolution failed, the function fell straight through to suffixResolve. No strip-and-retry. The stale NOTE comment in the code documented this as a known limitation.

After this PR: The alias branch (standard.ts:75-80) now applies the same 7-line pattern: call stripJsExtension(rewritten), and if non-null, call tryResolveWithExtensions(strippedAlias, allFiles) before falling through to suffix matching. The stale NOTE comment is removed.

Why it matters for graph correctness: An import @/utils.js in a TypeScript ESM project rewrites to e.g. src/utils.js. The file on disk is src/utils.ts. Before this fix, tryResolveWithExtensions('src/utils.js', allFiles) returns null (no literal .js file), and suffix matching on parts ['src', 'utils.js'] also returns null (because utils.js.ts is not in the suffix index). The IMPORTS edge is silently dropped. After this fix, the strip-and-retry correctly resolves src/utilssrc/utils.ts via the EXTENSIONS probe order, and the IMPORTS edge is correctly emitted.


Findings

Finding 1 — CI incomplete, cannot confirm required checks pass

  • Severity: Blocking
  • Location: GitHub Actions run 25794587955
  • Problem: The main CI workflow (tests on ubuntu/macOS/Windows, typecheck, lint, format) is queued and has not completed for head 8b0c410. The previous run showed a "checks failed" state prior to the autofix commit. There is no completed green run for the final head.
  • Risk: The DoD (§4.2) and review instructions require green required checks before merge. Any test or typecheck regression introduced by the latest merge-from-main would be invisible.
  • Recommended fix: Wait for the queued CI run to complete and confirm all required checks pass (quality/format, quality/lint, quality/typecheck, tests/ubuntu, tests/windows-latest, tests/macos-latest).
  • Blocks merge: YES

Finding 2 — No alias-specific tests for .mjs/.cjs extensions

  • Severity: Minor
  • File: gitnexus/test/unit/esm-extension-resolution.test.ts
  • Problem: The describe('ESM extension resolution — path aliases with .js extensions') block covers .js and .jsx aliases but does not include explicit tests for @/config.mjs → src/config.mts or @/legacy.cjs → src/legacy.cts via alias. These are handled by the shared stripJsExtension regex (/\.(js|jsx|mjs|cjs)$/) which is covered by both the stripJsExtension unit tests and the relative-import tests — but the alias branch is not explicitly exercised for .mjs/.cjs.
  • Risk: Low. The shared stripJsExtension function is thoroughly tested and the alias branch calls the same function identically to the relative branch. A regression in .mjs/.cjs alias resolution would only be caught by the relative-import tests, not an alias-specific test.
  • Recommended fix: Add two alias tests: @/config.mjs → src/config.mts and @/legacy.cjs → src/legacy.cts, mirroring the existing relative import tests for these extensions. Fix this →
  • Blocks merge: maybe — acceptable if maintainer judges the shared function coverage sufficient; recommend fixing regardless.

Finding 3 — suffixResolve uses rewritten (with .js extension), not stripped path — verified safe, no regression

  • Severity: Informational
  • File: standard.ts:82-85
  • Problem: (This is a verification, not a new issue.) After both direct resolution and ESM fallback fail, suffixResolve is called on parts = rewritten.split('/').filter(Boolean) where rewritten still carries the .js extension. Suffix matching tries ['src', 'utils.js'] → suffixes utils.js, utils.js.tsx, utils.js.ts — none of which match src/utils.ts. This confirms the suffix fallback cannot accidentally resolve a .ts file through the .js-suffixed path.
  • Risk: None. The test 'returns null when alias .js import has no matching source' validates this.
  • Blocks merge: NO

PR-specific assessment sections

Resolver correctness

Pass. The 7-line addition (standard.ts:75-80) is correctly placed: after tryResolveWithExtensions(rewritten, allFiles) fails and before the suffixResolve fallback. Real .js precedence is preserved — tryResolveWithExtensions tries the literal path first (extension list begins with empty string '', then .tsx, .ts... so src/utils.js exact match wins if the file exists). Confirmed by the test 'prefers actual .js file over TS fallback in alias resolution'. stripJsExtension correctly returns null for .ts extensions and no-extension paths (the stripJsExtension unit tests cover this). Alias fallback is gated on language === TypeScript || JavaScript and tsconfigPaths && !importPath.startsWith('.'), so it cannot run on other languages or relative imports.

Alias/baseUrl semantics

Pass. rewritten is constructed identically to pre-PR code (targetPrefix + remainder for baseUrl='.', baseUrl + '/' + targetPrefix + remainder otherwise). The TsconfigPaths.aliases is a Map<string, string> loaded via loadTsconfigPaths in language-config.ts:71-89, which converts glob patterns correctly (@/*@/, src/*src/). The for...of loop iterates the Map in insertion order. The test for baseUrl='app' (~/helpers/token.js → app/src/helpers/token.ts) confirms non-dot baseUrl works. Edge cases with trailing slashes work because loadTsconfigPaths normalizes them via the /*-stripping. Overlapping aliases (e.g., @/ and @shared/) are safe because @shared/foo.js does not start with @/ (the character at index 1 is s, not /).

Suffix fallback interaction

Pass. Suffix matching is only reached if both direct resolution and ESM stripped resolution fail. The suffix call uses rewritten (with .js extension still present), so the suffix index entries for .ts files cannot be reached via a .js-extension suffix (suffix index maps exact file names). No regression on Issue B.

Extension matrix

Partial pass. Direct alias tests cover .js.ts/.tsx and .jsx.tsx. The EXTENSIONS probe order (['.tsx', '.ts', '.mts', '.cts', '.jsx', '.js', '/index.tsx', '/index.ts', ...] in utils.ts:7-53) determines which source extension wins — well-documented in the test comments. Directory index resolution via strip('dir.js') → 'dir'/index.ts probe is tested. .mjs/.cjs are covered by stripJsExtension unit tests and relative-import tests, but not by explicit alias tests (see Finding 2).

Cache behavior

Pass. Cache key ${currentFile}::${importPath} is unchanged (uses raw importPath, not rewritten). The alias fallback result is stored under the original import path, which is correct — two calls from the same file with the same import path should return the same result. The resolveCache is typed as Map<string, string | null> in ImportResolutionContext (types.ts:43) — its lifecycle is per analysis run per repo, not shared across tsconfig contexts. This is a pre-existing design; the PR does not worsen it. null caching is preserved — after the alias loop exits without a match, the function falls through to the relative/suffix path and ultimately returns cache(null) if nothing matches.

Test adequacy

Mostly pass. The resolveWithAlias helper in the test correctly passes a tsconfigPaths object (with aliases and baseUrl) to exercise the real alias branch. The makeCtx helper builds a real SuffixIndex via buildSuffixIndex — no mocks hiding normalization differences. The 5 new alias tests cover the key scenarios. Existing tests (25 unit + 2 integration per PR description) continue to pass. Missing: explicit .mjs/.cjs alias tests (Finding 2). A note: the resolveWithAlias helper passes aliases as [string, string][] (array of tuples) where TsconfigPaths.aliases is typed Map<string, string>. At runtime both are iterable with for...of, and TypeScript's --noEmit is reported green, so this is acceptable — the PR description confirms typecheck passes.

CI/checks

⚠️ Blocked pending. As of this review:

  • ✅ CodeQL, Gitleaks, Dependency Review, PR Autofix: all success
  • ⏳ Main CI (tests + quality): queued — run 25794587955 started at 2026-05-13T10:53:29Z, no conclusion yet
  • ⏳ Docker Build & Push: in_progress

The previous visible CI report (from run 25783775950) showed a "checks failed" state attributed to formatting issues before the /autofix commit was applied. That run is stale and should not be used to gate this review. CI must complete and pass on all required platforms before merge. This is the primary blocker.

Hidden Unicode / hygiene

Clean. Both standard.ts and esm-extension-resolution.test.ts were scanned for bidirectional controls (\x{202A}-\x{202E}, \x{2066}-\x{2069}), zero-width characters (\x{200B}, \x{200C}, \x{200D}, \x{FEFF}), and non-ASCII characters. Zero matches in both files. No hidden or bidi controls present.


Final verdict

Not production-ready

The implementation is correct and well-scoped — the 7-line alias-branch fallback is placed exactly right, real .js precedence is preserved, suffix matching is not regressed, and the relative-import path is unchanged. Branch hygiene is acceptable: the net diff is exactly the 2 intended files and the merge-from-main commits are harmless. Unicode hygiene is clean.

However, the primary blocker is CI incomplete: the main CI workflow (tests on all 3 platforms + quality checks) is queued and has not produced a result for the final head 8b0c410. Per the repo DoD (§4.2) and the PR-specific DoD, I cannot approve until all required checks are green. Additionally, two alias-specific tests for .mjs and .cjs are missing from the alias test block (Finding 2, non-blocking but recommended). Once CI completes green and the .mjs/.cjs alias tests are added (or a maintainer explicitly waives them), this PR is production-ready with minor follow-ups.


· Current CI run

@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 4m 56s —— View job


PR #1530 Adversarial Production-Readiness Review

Tasks

  • Post initial comment
  • Verify head SHA and net diff against origin/main
  • Read repo guidance docs (tsconfig.json, tsconfig.test.json, ci-quality.yml, language-config.ts, types.ts)
  • Inspect changed files (standard.ts, esm-extension-resolution.test.ts)
  • Inspect related files (utils.ts, types.ts, language-config.ts)
  • Run hidden Unicode / bidi scans on changed files
  • Verify CI status on latest head SHA
  • Write comprehensive adversarial review

Review bar for this PR

resolveImportPath is the central dispatch for all import-edge generation. A false-positive IMPORTS edge corrupts the graph; a false-negative silently drops an edge. The DoD requires tests that exercise the real changed path and fail if the fix is removed. CI must be green on the final head before merge. Branch hygiene must ensure the net diff is limited to the intended files.


Current PR state

Item Status
Head SHA dec773e1c1c6daf2acc4bf116380cdd8c1010290
Head commit test(esm): cover .mjs/.cjs path-alias extension resolution
PR-only commits f335069 (original fix), b6a1fe1 (autofix), dec773e (mjs/cjs alias tests)
Changed files (net, PR-only) 2 filesstandard.ts (+7/-3), esm-extension-resolution.test.ts (+70/-0)
CI — Docker Build & Push ✅ success (run 25859337786)
CI — Gitleaks ✅ success (run 25859337765)
CI — PR Autofix ✅ success (run 25859337770)
CI — Dependency Review ✅ success (run 25859337782)
CI — CodeQL ✅ success (run 25859337771)
CI — Main CI (tests/typecheck/lint) success (run 25859337799)

All 6 required CI runs are completed/success on the final head dec773e1.


Branch hygiene assessment

Verdict: merge-from-main commit present but harmless and merge-safe.

The PR has 20 commits, the majority of which are Merge branch 'main' into fix/esm-path-alias-js-extension. Inspecting each PR-specific commit via git show:

  • f335069 --stat — only touches standard.ts and esm-extension-resolution.test.ts (plus files that arrived via the initial fork commit from an earlier state of main, which are already present on main)
  • b6a1fe1 --stat — only reformatting of the same test file
  • dec773e --stat — adds 12 lines to the test file only

A squash or regular merge produces a 2-file net PR diff. No unrelated file churn from the PR's own commits.


Understanding of the change

Pre-fix flow for @/utils.js (TypeScript, alias @/ → src/, baseUrl='.'):

  1. rewritten = 'src/utils.js'
  2. tryResolveWithExtensions('src/utils.js', allFiles) — returns null (no literal .js file)
  3. ESM strip-and-retrymissing
  4. suffixResolve(['src', 'utils.js'], ...) — tries utils.js, utils.js.tsx, etc. — no match → null (edge dropped)

Post-fix flow:

  1. rewritten = 'src/utils.js'
  2. tryResolveWithExtensions('src/utils.js', allFiles) — null
  3. stripJsExtension('src/utils.js')'src/utils'
  4. tryResolveWithExtensions('src/utils', allFiles) — probes src/utils.tsx, src/utils.ts, ... → src/utils.ts
  5. (never reached) suffixResolve

The 7-line block (standard.ts:75-80) mirrors exactly the relative-import fallback already present at standard.ts:142-147.


Findings

Finding 1 — Test helper uses [string, string][] where production uses Map<string, string>

  • Severity: Minor
  • File: gitnexus/test/unit/esm-extension-resolution.test.ts:160-173
  • Problem: The resolveWithAlias helper declares aliases: [string, string][] and passes { aliases, baseUrl } as TsconfigPaths. Production TsconfigPaths.aliases is typed Map<string, string> (language-config.ts:15). The object literal is structurally incompatible with Map<string, string> (maps require get, has, set, delete, clear, size that arrays do not expose). This type mismatch is not caught by CI typecheck because npx tsc --noEmit uses tsconfig.json which includes only src/**/*, not test/**/*. tsconfig.test.json is never invoked in CI.
  • Production risk: None — the production code path always receives a real Map from loadTsconfigPaths. Runtime behavior of the test is correct because for...of on both Map and [K,V][] yields [K, V] tuples identically. However, if the production code were ever changed to call Map-specific methods (e.g., aliases.get(key)), the tests would silently pass while being type-unsafe.
  • Recommended fix: Change aliases: [string, string][] to aliases: Map<string, string> in the test helper and pass new Map([['@/', 'src/']]) in each test call, mirroring the exact type from loadTsconfigPaths. Additionally, add tsc -p tsconfig.test.json --noEmit as a separate CI step to enforce type-checking of test files. Fix this →
  • Blocks merge: No — runtime-safe, and production code is unaffected.

Finding 2 — Test tsconfig not covered by CI typecheck (pre-existing gap, not introduced by PR)

  • Severity: Informational
  • File: .github/workflows/ci-quality.yml:42npx tsc --noEmit uses gitnexus/tsconfig.json which has "include": ["src/**/*"]. tsconfig.test.json (which extends tsconfig.json and adds test/**/*) is never invoked in CI.
  • Problem: Type errors in test files are never caught by CI. Finding 1 is an example of this gap.
  • Production risk: None — tests are not shipped. But test type errors could mask API drift between test helpers and production types.
  • Recommended fix: Add npx tsc -p tsconfig.test.json --noEmit as a CI step in ci-quality.yml.
  • Blocks merge: No — pre-existing gap, not introduced by this PR.

PR-specific assessment sections

Resolver correctness

Pass. The ESM fallback block (standard.ts:75-80) is placed correctly: after tryResolveWithExtensions(rewritten, allFiles) at line 72 (direct resolution) and before suffixResolve at line 83 (suffix fallback). It only runs inside the tsconfigPaths && !importPath.startsWith('.') guard (line 57-61), so it cannot fire for relative imports or non-TS/JS languages. The relative import fallback at lines 142-147 is unchanged.

Alias/baseUrl semantics

Pass. rewritten is built identically to pre-PR code: targetPrefix + remainder for baseUrl='.', baseUrl + '/' + targetPrefix + remainder otherwise. loadTsconfigPaths normalizes @/*@/ and src/*src/. The test at line 206-216 confirms baseUrl='app' + alias ~/ → src/ produces app/src/helpers/token.ts. Trailing slash normalization and insertion order are pre-existing behaviors unmodified by this PR.

Extension matrix

Pass (all 4 JS-family extensions covered for aliases, including .mjs/.cjs added in dec773e). Extension probe order: '', .tsx, .ts, .mts, .cts, .jsx, .js, .mjs, .cjs, /index.tsx, /index.ts... When stripping .mjs from @/config.mjssrc/config, the probe tries src/config.ts before src/config.mts. If both exist, .ts wins. This behavior is explicitly documented and tested at lines 112-120 as intentional for source-analysis (not runtime resolution).

Literal JS precedence

Pass. tryResolveWithExtensions('src/utils.js', allFiles) at line 72 runs first. Since EXTENSIONS[0] = '', the exact path src/utils.js is tried first. If src/utils.js exists, it returns before the ESM strip-and-retry is reached. Test at line 200-204 ('prefers actual .js file over TS fallback in alias resolution') verifies this.

Suffix fallback interaction

Pass. suffixResolve at line 83 receives rewritten.split('/') — i.e., ['src', 'utils.js'] — which still carries the .js extension. Suffix probing tries utils.js + EXTENSIONS entries (e.g., utils.js.ts, utils.js.tsx), none of which would match a real .ts file. This means suffix matching cannot accidentally resolve a .ts target through a .js-suffixed alias path. Test at line 218-222 ('returns null when alias .js import has no matching source') verifies this.

Cache behavior

Pass. Cache key ${currentFile}::${importPath} uses raw importPath (e.g., @/utils.js), not rewritten. This is consistent with pre-PR behavior: same file + same import string always returns the same cached result. Cache key does not include tsconfig context, but this is a pre-existing design. The PR does not introduce any new cache key or eviction change.

Test adequacy

Pass. All 4 JS-family extensions are covered for both relative imports and alias imports. The resolveWithAlias helper calls the real resolveImportPath (no mocks), builds a real SuffixIndex via buildSuffixIndex, and uses a real in-memory Set and Map. Tests would fail if the 7-line fallback block were removed. The one caveat (type mismatch in the helper, Finding 1) is runtime-safe and non-blocking.

CI/checks

Pass. All 6 required CI runs are completed/success on dec773e1:

Check Conclusion
Docker Build & Push ✅ success
Gitleaks ✅ success
PR Autofix ✅ success
Dependency Review ✅ success
CodeQL ✅ success
CI (tests/typecheck/lint/e2e) ✅ success

The prior review's CI-blocking concern is resolved. This is not a stale result — runs were created at 2026-05-14T12:12:43Z against the final head.

Hidden Unicode / hygiene

Clean. Grep for bidi controls (U+202A–U+202E, U+2066–U+2069) and zero-width characters (U+200B, U+200C, U+200D, U+FEFF) found zero matches in both changed files. No non-ASCII characters found in executable paths, regexes, or string literals.


Final verdict

production-ready with minor follow-ups

The implementation is correct and well-scoped: the 7-line alias-branch ESM fallback is placed exactly right (after direct resolution, before suffix fallback), literal .js file precedence is preserved, suffix matching is not regressed, and the relative import path is unchanged. All CI checks are green on the final head dec773e1. Branch hygiene is clean — the net PR diff is exactly 2 files. The only finding is a minor type mismatch in the test helper ([string, string][] where Map<string, string> is expected), which is runtime-safe and not caught by CI because test files are excluded from the CI typecheck invocation. A maintainer may merge as-is and track the type-safe test helper and test-tsconfig CI coverage as follow-up tasks, or apply the fix before merge.


View CI run ·

magyargergo and others added 2 commits May 14, 2026 13:51
Matches TsconfigPaths.aliases from language-config. CI cannot run tsc -p tsconfig.test.json yet: the project has hundreds of pre-existing errors under test/ (fixtures + unit/integration); enable that step after backlog cleanup.

Co-authored-by: Cursor <cursoragent@cursor.com>
@magyargergo magyargergo merged commit 911a2ee into abhigyanpatwari:main May 14, 2026
29 of 30 checks passed
hohaivu pushed a commit to hohaivu/GitNexus that referenced this pull request May 19, 2026
…on (abhigyanpatwari#1530)

* fix: apply ESM .js extension fallback to tsconfig path alias resolution

Path alias imports (e.g. `@/utils.js` via tsconfig paths) now correctly
strip JS-family extensions and retry with TS equivalents when the literal
.js file does not exist. This applies the same stripJsExtension fallback
already used for relative imports to the alias resolution branch.

Fixes abhigyanpatwari#1528

* chore(autofix): apply prettier + eslint fixes via /autofix command

* test(esm): cover .mjs/.cjs path-alias extension resolution

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(esm): use Map for path aliases in resolveWithAlias helper

Matches TsconfigPaths.aliases from language-config. CI cannot run tsc -p tsconfig.test.json yet: the project has hundreds of pre-existing errors under test/ (fixtures + unit/integration); enable that step after backlog cleanup.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.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.

Path alias imports with .js extensions do not trigger ESM fallback resolution

2 participants