Skip to content

fix: resolve TypeScript ESM .js extension imports to .ts source files#1525

Merged
magyargergo merged 4 commits into
abhigyanpatwari:mainfrom
chouzz:fix/esm-js-extension-resolution
May 12, 2026
Merged

fix: resolve TypeScript ESM .js extension imports to .ts source files#1525
magyargergo merged 4 commits into
abhigyanpatwari:mainfrom
chouzz:fix/esm-js-extension-resolution

Conversation

@chouzz

@chouzz chouzz commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1503 — TypeScript ESM imports with .js extensions now correctly resolve to .ts source files.

Problem

TypeScript ESM (moduleResolution: node16/bundler) requires imports to use .js extensions:

import { estimateTokens } from './utils.js'; // source file is utils.ts

The import resolver tried utils.js directly and appended more extensions (utils.js.ts, etc.), but never stripped .js to try .ts. Result: 0 call edges for any ESM project.

Fix

In resolveImportPath (TS/JS relative path branch only):

  1. If direct resolution fails and the path ends with a JS-family extension (.js/.jsx/.mjs/.cjs), strip it and re-resolve — matching TypeScript compiler behavior.
  2. Added .mts/.cts to the EXTENSIONS list.

The fallback is scoped to TypeScript/JavaScript only — no impact on other languages.

Testing

  • 15 new unit tests covering all extension mappings, priority (real .js wins), non-TS languages, and edge cases.
  • Manual verification: indexed a minimal ESM repo with ./utils.js import → context estimateTokens correctly shows the caller.

Verification

$ gitnexus context --repo esm-test estimateTokens
{
  "status": "found",
  "symbol": { "name": "estimateTokens", "filePath": "src/utils.ts" },
  "incoming": { "calls": [{ "name": "index.ts", "filePath": "src/index.ts" }] }
}

@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

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
8757 8756 0 1 424s

✅ All 8756 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 77.57% 26309/33915 N/A% 🟢 ███████████████░░░░░
Branches 66.1% 16579/25080 N/A% 🟢 █████████████░░░░░░░
Functions 82.33% 2652/3221 N/A% 🟢 ████████████████░░░░
Lines 80.63% 23782/29493 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 5m 0s —— View job


PR #1525 Production-Readiness Review

Tasks

  • Post initial comment
  • Read repo context files (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, ARCHITECTURE.md)
  • Verify branch hygiene (git log, diff stats, changed files)
  • Inspect changed files (standard.ts, utils.ts, test file)
  • Inspect related resolver architecture files
  • Run targeted searches (resolveImportPath, EXTENSIONS, cache, scope-resolver, etc.)
  • Check Unicode/hygiene
  • Assess production graph / call-edge path
  • Assess test adequacy
  • Formulate final verdict and post full review

Review Bar for This PR

  • ./utils.js resolves to utils.ts only when no literal utils.js exists; .jsx.tsx, .mjs.mts, .cjs.cts analogously.
  • Real .js files win over TypeScript fallback (precedence preserved).
  • Non-relative imports, non-TS/JS languages, and all other resolver paths are unaffected.
  • The fix is active on the primary production CALLS path (TypeScript scope-resolution pipeline → resolveTsTargetresolveImportPath).
  • All added tests are included in the CI test suite.
  • No hidden/bidi Unicode controls in changed files.
  • CI passes on the head SHA (typecheck + unit tests on 3 platforms).

Current PR State

Title fix: resolve TypeScript ESM .js extension imports to .ts source files
Branches chouzz:fix/esm-js-extension-resolutionabhigyanpatwari:main
Head SHA (local) 2bc4b01 (single commit; verified as the only commit since divergence)
Changed files 3 files: standard.ts (+28/-1), utils.ts (+4/-0), esm-extension-resolution.test.ts (+110/-0)
CI ✅ All checks passed per bot report (typecheck, unit tests on 3 platforms, E2E) — 8 696 tests passed
Reviews None
TypeScript in MIGRATED_LANGUAGES Verified in registry-primary-flag.ts — TS uses scope-resolution pipeline

Branch Hygiene Assessment

Classification: clean feature/fix PR

Exactly one focused commit touching three files, all scoped to the ESM import-resolver fix and its tests. No lockfile changes, no workflow churn, no unrelated formatting. The branch is up-to-date with main.


Understanding of the Change

Before: resolveImportPath for a relative import path like ./utils.js would build the base path src/utils.js, then call tryResolveWithExtensions which probes src/utils.js, src/utils.js.tsx, src/utils.js.ts, etc. It never stripped the .js suffix. Result: 0 resolved edges for any ESM TypeScript repo.

After: If tryResolveWithExtensions fails AND the language is TS or JS AND the base path ends with a JS-family extension (.js, .jsx, .mjs, .cjs), stripJsExtension strips the suffix to get src/utils, then calls tryResolveWithExtensions again. This probes src/utils.tsx, src/utils.ts, src/utils.mts, src/utils.cts, src/utils.jsx, etc. — matching TypeScript compiler behavior.

utils.ts adds .mts and .cts to EXTENSIONS so those source extensions are probed.

Critical production path confirmed: TypeScript is in MIGRATED_LANGUAGES (scope-resolution pipeline is primary for TS CALLS edges). The TS scope-resolver's resolveImportTarget calls makeTsResolveImportTarget()resolveTsTarget (languages/typescript/import-target.ts:78) → resolveImportPath from standard.ts. This PR's fix directly affects the production CALLS path — it is not just the legacy import-processor path.


Findings

Finding 1 — Extension-pair precedence for .mjs/.cjs is globally ordered, not pair-specific

  • Severity: medium
  • File: gitnexus/src/core/ingestion/import-resolvers/utils.ts:7-54
  • Risk: After stripping .mjs to get stem config, tryResolveWithExtensions tries .tsx.ts.mts.cts. If both config.ts and config.mts exist, config.ts wins — even though .mjs semantically corresponds to .mts. Same applies to .cjs/.cts.
  • Evidence: EXTENSIONS order: .tsx, .ts, .mts, .cts, .jsx, .js. No existing test covers the case where both config.ts and config.mts exist simultaneously.
  • Recommended fix: Add tests asserting the resolution result explicitly when both a .ts and .mts sibling exist for a .mjs import. If the global order is intentional (GitNexus is a source-analysis tool, not a runtime), document it. If .mjs should prefer .mts, add pair-specific fallback ordering.
  • Blocks merge: no — in practice, codebases with simultaneous config.ts and config.mts are extremely rare; the primary bug (.js.ts) is fixed correctly.

Finding 2 — ./dir.js can accidentally resolve to dir/index.ts

  • Severity: low
  • File: gitnexus/src/core/ingestion/import-resolvers/standard.ts:130-142, utils.ts:19-20
  • Risk: After stripping ./dir.js to ./dir, tryResolveWithExtensions probes /index.tsx, /index.ts etc. So import './dir.js' can resolve to dir/index.ts. In strict Node ESM this would not happen (Node16/nodenext .js specifiers don't do bare-directory resolution), creating potential false edges.
  • Evidence: EXTENSIONS includes /index.tsx, /index.ts. This behavior existed pre-PR for non-.js relative imports; the fix extends it to .js-suffixed imports.
  • Recommended fix: Add a test asserting that ./dir.js does NOT resolve to dir/index.ts if dir.js doesn't exist and only dir/index.ts does — or add a test accepting that behavior and document it as intentional (bundler-mode compatibility).
  • Blocks merge: no.

Finding 3 — makeCtx normalizes to lowercase; production normalizes backslashes only

  • Severity: low
  • File: gitnexus/test/unit/esm-extension-resolution.test.ts:16-21
  • Risk: makeCtx uses files.map((f) => f.toLowerCase()) while buildImportResolutionContext uses p.replace(/\\/g, '/'). Doesn't affect the ESM extension tests (relative resolution uses allFilesSet.has(...), not normalizedFileList), but may hide case-sensitivity differences if future tests add non-relative imports.
  • Recommended fix: Either use the same normalization as production (p.replace(/\\/g, '/')) or add a comment documenting why lowercase is intentional. Fix this →
  • Blocks merge: no.

Finding 4 — No end-to-end test verifying CALLS/context edges for a TS ESM project

  • Severity: medium
  • Risk: The bug manifests as missing caller/callee relationships in context/impact, not just wrong file paths. A unit test on resolveImportPath verifies the file-path result but not that the edge reaches CALLS.
  • Evidence: Confirmed the production path: scope-resolver → resolveTsTargetresolveImportPath (verified in import-target.ts:78). The fix is architecturally sound and will propagate to CALLS edges. However, no fixture test exercises the full stack.
  • Recommended fix: Add an integration fixture in test/integration/ that indexes a minimal ESM project with ./utils.js imports and asserts context estimateTokens shows callers. (The manual verification in the PR description is good but not automated.)
  • Blocks merge: no — the production path linkage is verified by code inspection and existing CI scope-parity tests.

Finding 5 — Path aliases with .js specifiers remain unresolved

  • Severity: medium (follow-up, out of scope)
  • File: gitnexus/src/core/ingestion/import-resolvers/standard.ts:56-81
  • Risk: The tsconfig alias branch (@/utils.jssrc/utils.js) calls tryResolveWithExtensions(rewritten, allFiles) directly with no stripJsExtension fallback. If a TS ESM project uses both path aliases AND .js specifiers, those imports remain unresolved.
  • Evidence: PR description does not claim to fix alias-based .js imports. The fix is explicitly scoped to relative imports. Issue TypeScript extension imports fail to resolve call edges #1503's reproduction uses ./utils.js (relative), not aliases.
  • Recommended fix: This is a follow-up issue worth filing. The alias branch would need the same strip-and-retry logic applied to the rewritten path.
  • Blocks merge: no — out of scope for this PR, but worth filing as a follow-up.

PR-Specific Assessment Sections

TypeScript/Node ESM import-resolution correctness

stripJsExtension uses /\.(js|jsx|mjs|cjs)$/ — a terminal-only regex that cannot strip .js from directory components in the middle of a path. Safe. The fallback is gated on importPath.startsWith('.') AND (language === TypeScript || language === JavaScript) — correctly scoped to relative imports in TS/JS files only.

.jsx.tsx precedence: After stripping .jsx, tryResolveWithExtensions tries .tsx before .ts. So component.jsxcomponent.tsx ✅ (.tsx wins).

Real .js wins: tryResolveWithExtensions is called first with the original basePath including the .js extension. If utils.js is in allFiles, it returns immediately before the strip logic is reached. ✅

Vue behavior: vueImportConfig uses createStandardStrategy(SupportedLanguages.TypeScript). Vue files run through the TypeScript language path and benefit from this fix. ✅

Existing resolver compatibility and precedence

Direct resolution cache hit returns immediately before the new fallback code — no performance penalty for already-resolved paths. Non-relative imports fall through to suffix resolution without touching stripJsExtension. Rust, Go, Python, C/C++, Java, Kotlin, PHP, Swift, Ruby are unaffected (language guard on lines 135-140). ✅

Production graph/call-edge verification

Primary path (TypeScript scope-resolution pipeline):
runScopeResolutionmakeTsResolveImportTarget() in scope-resolver.tsresolveTsTarget in import-target.ts:78resolveImportPath from standard.ts (the changed file).

The PR fix reaches production CALLS edges for TypeScript. This is the correct and complete path. ✅

Legacy path (for non-migrated TS files / JavaScript):
import-processor.ts:processImportsresolveStandardresolveImportPath. Also correctly fixed. ✅

Test adequacy

15 unit tests cover: .js.ts, .jsx.tsx, .mjs.mts, .cjs.cts, real .js wins, ../ paths, JavaScript language, non-TS/JS returns null, no match returns null, and stripJsExtension unit cases.

Missing coverage (non-blocking):

  • ./config.mjs when both config.ts AND config.mts exist (resolution order)
  • ./dir/index.jsdir/index.ts (should work, untested)
  • ./dir.js → no false dir/index.ts edge (boundary not tested)
  • Path alias with .js suffix (out of scope)

Test file is included in npm run test:unit via vitest run test/unit glob and the include: ['test/**/*.test.ts'] pattern in vitest.config.ts. ✅

CI/checks verification

CI bot reports: ✅ typecheck (tsc --noEmit), ✅ unit tests on 3 platforms, ✅ E2E (no gitnexus changes). 8 696 tests passed, 1 known skip (Ruby block parameter). New test file adds 15 tests, consistent with CI count (8 696 = baseline + new tests). ✅

Security/hygiene/hidden Unicode

standard.ts and esm-extension-resolution.test.ts are UTF-8 with visible non-ASCII in comments only:

  • standard.ts: (U+2248) and (em dash) in inline comments
  • esm-extension-resolution.test.ts: (U+2192 rightward arrow) in file header comment

No bidi controls. No hidden characters. No dependency or lockfile changes. ✅

Performance and cache behavior

The new fallback executes only when tryResolveWithExtensions fails for a relative import AND the language is TS/JS AND the path ends with a JS-family extension. The fallback is O(|EXTENSIONS|) ≈ 30 probes — bounded and negligible.

Cache key: currentFile::importPath. Since each file has exactly one language (determined by extension), the same currentFile::importPath pair is always resolved with the same language. No cross-language cache poisoning. The scope-resolution pipeline (import-target.ts:76) creates its own resolveCache per workspace pass, isolated from the legacy importCtx.resolveCache (cleared after parse phase per parse-impl.ts:601). ✅


Final Verdict

production-ready with minor follow-ups

The fix is architecturally correct and well-scoped. The TypeScript scope-resolution pipeline (the primary CALLS path since TypeScript joined MIGRATED_LANGUAGES per AGENTS.md v1.7.0) routes through resolveTsTargetresolveImportPath in standard.ts, confirming the fix reaches production CALLS/context/impact edges. The "real .js wins" precedence is preserved. All 15 unit tests pass and are CI-included. The extension-pair precedence ambiguity (.mjs may resolve to .ts before .mts) and the unguarded directory index fallback (./dir.jsdir/index.ts) are unlikely to cause incorrect graph edges in real codebases and are acceptable for a source-analysis tool. Suggested follow-ups: (1) add tests for .mjs/.cjs with competing .ts/.mts siblings, (2) clarify ./dir.js → directory-index behavior, (3) file a follow-up issue for path aliases with .js specifiers.

@magyargergo

Copy link
Copy Markdown
Collaborator

@chouzz Could you please look into claude's findings? 🙏

chouzz added 2 commits May 12, 2026 21:06
TypeScript ESM requires imports to use .js extensions even when source
files are .ts (moduleResolution: node16/bundler). The import resolver
now strips JS-family extensions (.js/.jsx/.mjs/.cjs) and retries with
TS equivalents (.ts/.tsx/.mts/.cts) when the literal .js file does not
exist. This fallback only applies to TypeScript/JavaScript languages.

Also adds .mts/.cts to the EXTENSIONS list for completeness.

Fixes abhigyanpatwari#1503
…ation test

- Fix makeCtx to use production normalization (.replace backslash)
  instead of .toLowerCase() (Finding 3)
- Add tests for .mjs/.cjs with competing .ts/.mts siblings (Finding 1)
- Add tests for ./dir.js → dir/index.ts boundary (Finding 2)
- Add integration test verifying full pipeline CALLS edges for ESM
  .js imports (Finding 4)
- Document path alias .js limitation as known follow-up (Finding 5)
@chouzz chouzz force-pushed the fix/esm-js-extension-resolution branch from f7fe336 to 999b724 Compare May 12, 2026 13:08
@chouzz

chouzz commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @magyargergo! I've addressed all of Claude's findings:

Fixed in this update:

  1. Finding 3 (makeCtx normalization) — Changed makeCtx to use .replace(/\/g, '/') matching
    production buildImportResolutionContext behavior instead of .toLowerCase().
  2. Finding 1 (.mjs/.cjs precedence) — Added 3 tests documenting the resolution order when competing
    .ts/.mts siblings exist. The global EXTENSIONS order (.ts before .mts) is intentional — in practice,
    having both config.ts and config.mts in the same directory is extremely rare.
  3. Finding 2 (directory index boundary) — Added 2 tests documenting that ./dir.js → dir/index.ts is
    accepted behavior (bundler-mode compatibility, consistent with existing non-ESM resolution).
  4. Finding 4 (end-to-end CALLS edges) — Added an integration test
    (typescript-esm-js-extension.test.ts) that runs the full pipeline on a minimal ESM repo and asserts
    both the CALLS edge (processText → estimateTokens) and the IMPORTS edge are emitted correctly.
  5. Finding 5 (path alias + .js) — Added a code comment documenting this as a known limitation. The
    tsconfig alias branch does not yet apply stripJsExtension fallback. I'll file a separate issue for
    this as it expands scope beyond TypeScript extension imports fail to resolve call edges #1503.

Follow-up issue to file:

  • Path alias imports with .js extensions (e.g. @/utils.js via tsconfig paths) do not trigger the ESM
    fallback. This requires applying the same strip-and-retry logic in the alias resolution branch. Will
    open a separate PR for this.

// .ts/.tsx/.mts/.cts. Strip the JS-family extension and re-resolve.
// NOTE: This fallback only applies to relative imports. Path alias imports
// (e.g. @/utils.js via tsconfig paths) do not yet strip .js extensions —
// that is a known limitation tracked for follow-up.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a ticket for this

@github-actions

Copy link
Copy Markdown
Contributor

✨ PR Autofix

Found fixable formatting / unused-import issues across 15 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":1525,"changed_lines":15,"head_sha":"999b724a70675806c9c6bd1c6e79a5af5caaebf0","run_id":"25736630179","apply_command":"/autofix"}

@magyargergo

Copy link
Copy Markdown
Collaborator

/autofix

@github-actions

Copy link
Copy Markdown
Contributor

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

@chouzz

chouzz commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Created #1528 to track the path alias + .js extension limitation. Thanks for the review!

@magyargergo

magyargergo commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Created #1528 to track the path alias + .js extension limitation. Thanks for the review!

Are you happy to work on it?

@chouzz

chouzz commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Created #1528 to track the path alias + .js extension limitation. Thanks for the review!

Are you happy to work on it?

Sure, you can assign to me, I will fix it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@magyargergo magyargergo merged commit a2f1b07 into abhigyanpatwari:main May 12, 2026
27 of 28 checks passed
dp-web4 pushed a commit to dp-web4/GitNexus that referenced this pull request May 13, 2026
…abhigyanpatwari#1525)

* fix: resolve TypeScript ESM .js extension imports to .ts source files

TypeScript ESM requires imports to use .js extensions even when source
files are .ts (moduleResolution: node16/bundler). The import resolver
now strips JS-family extensions (.js/.jsx/.mjs/.cjs) and retries with
TS equivalents (.ts/.tsx/.mts/.cts) when the literal .js file does not
exist. This fallback only applies to TypeScript/JavaScript languages.

Also adds .mts/.cts to the EXTENSIONS list for completeness.

Fixes abhigyanpatwari#1503

* fix: address review findings — normalization, edge-case tests, integration test

- Fix makeCtx to use production normalization (.replace backslash)
  instead of .toLowerCase() (Finding 3)
- Add tests for .mjs/.cjs with competing .ts/.mts siblings (Finding 1)
- Add tests for ./dir.js → dir/index.ts boundary (Finding 2)
- Add integration test verifying full pipeline CALLS edges for ESM
  .js imports (Finding 4)
- Document path alias .js limitation as known follow-up (Finding 5)

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

* chore: retrigger CI after bot-only tip commit

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

---------

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

TypeScript extension imports fail to resolve call edges

2 participants