From a6c653e36b528acb8d2d401fc8fbca6d64fd72cc Mon Sep 17 00:00:00 2001 From: Tirth Kanani Date: Sun, 31 May 2026 22:31:23 +0100 Subject: [PATCH] =?UTF-8?q?fix(extract-import-map):=20apply=20NodeNext=20.?= =?UTF-8?q?js=E2=86=92.ts=20rewrite=20(#294)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the silent near-edgeless-graph regression on any modern ESM TypeScript project. Reported in #294 with full repro + root-cause analysis. ### Why this matters Under `moduleResolution: NodeNext` (or `Node16` / `Bundler` with explicit extensions — the default for new TS-ESM projects since 2023), TypeScript does NOT rewrite import specifiers during compilation: // src/index.ts — real, idiomatic NodeNext source import { x } from './config.js'; // on disk: config.ts Before this fix, `probeWithExtensions` only tried APPENDING extensions to the import specifier: './config.js' → not in fileSet './config.js.ts', './config.js.tsx', './config.js.js', ... → all miss → returns null → edge dropped at merge as dangling Net result on the reporter's repro: a knowledge graph with hundreds of file nodes and almost no `imports` edges between them — silently removing exactly the dependency structure the graph is meant to show. ### Fix New `NODENEXT_REWRITES` table maps each compiled-output extension to the TypeScript source extensions that could have produced it: .js → [.ts, .tsx, .js, .jsx] .jsx → [.tsx, .jsx] .mjs → [.mts, .mjs, .ts] .cjs → [.cts, .cjs, .ts] `probeWithExtensions` now applies the rewrite when the import already ends with one of these extensions and no such file exists on disk. The rewrite runs BEFORE the legacy append-extensions loop — otherwise `./foo.js` would generate the nonsense candidate `foo.js.ts` and the append loop would never reach the actual `foo.ts`. ### Disambiguation If both `config.ts` and `config.js` exist on disk (rare, but possible during a partial migration), `import './config.js'` still resolves to the .js — that's an exact-disk match and what NodeNext compilation actually does. The rewrite only kicks in when the .js doesn't exist. ### Tests 6 new tests in `test_extract_import_map.test.mjs`: - The main #294 case (`.js → .ts`) - `.jsx → .tsx` and `.mjs → .mts` rewrites - Disambiguation when both `.ts` and `.js` exist on disk - Pure-JS projects still work (real `.js → .js` imports) - Historical no-extension probes unaffected - Missing files still return null (rewrite can't invent targets) Total: 202 tests passing (was 196). Closes #294 --- .../test_extract_import_map.test.mjs | 144 ++++++++++++++++++ .../skills/understand/extract-import-map.mjs | 42 ++++- 2 files changed, 185 insertions(+), 1 deletion(-) diff --git a/tests/skill/understand/test_extract_import_map.test.mjs b/tests/skill/understand/test_extract_import_map.test.mjs index ee64136..e25a644 100644 --- a/tests/skill/understand/test_extract_import_map.test.mjs +++ b/tests/skill/understand/test_extract_import_map.test.mjs @@ -237,6 +237,150 @@ describe('extract-import-map.mjs — TypeScript / JavaScript resolver', () => { 'packages/foo/src/y.ts', ); }); + + // ── #294: NodeNext / ESM TypeScript `.js → .ts` rewrite ──────────────── + // + // Under `moduleResolution: NodeNext`, TypeScript does NOT rewrite import + // specifiers during compilation — what you write in the .ts source is + // emitted verbatim. Because Node's ESM loader requires explicit file + // extensions at runtime, the TS source must already spell the import with + // the `.js` extension that will only be correct AFTER compilation: + // + // import { x } from './config.js'; // on disk: config.ts + // + // Before the fix, every such import resolved to null, leaving ESM-TS + // projects with a near-edgeless knowledge graph. + + it('resolves NodeNext .js → .ts relative imports (the main #294 case)', () => { + projectRoot = setupTree({ + 'src/index.ts': `import { resolveBackend } from './llm-backend-selector.js';\nimport { loadConfig } from './config.js';\n`, + 'src/llm-backend-selector.ts': `export function resolveBackend() {}\n`, + 'src/config.ts': `export function loadConfig() {}\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/index.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/llm-backend-selector.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/config.ts', language: 'typescript', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.ts']).toEqual([ + 'src/config.ts', + 'src/llm-backend-selector.ts', + ]); + }); + + it('resolves NodeNext .jsx → .tsx and .mjs → .mts rewrites', () => { + projectRoot = setupTree({ + 'src/index.ts': `import Comp from './Comp.jsx';\nimport { fn } from './worker.mjs';\n`, + 'src/Comp.tsx': `export default function Comp() {}\n`, + 'src/worker.mts': `export function fn() {}\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/index.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/Comp.tsx', language: 'typescript', fileCategory: 'code' }, + { path: 'src/worker.mts', language: 'typescript', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.ts']).toEqual([ + 'src/Comp.tsx', + 'src/worker.mts', + ]); + }); + + it('resolves to the .js when both .ts and .js exist on disk', () => { + // Rare but possible during a partial migration: both `config.ts` and + // `config.js` exist. An `import './config.js'` is an exact-disk match + // and should resolve to that exact file — the NodeNext rewrite only + // kicks in when the .js *doesn't* exist on disk. We assert this to + // pin the disambiguation and avoid future regressions where the rewrite + // accidentally prefers `.ts` over an existing `.js`. + projectRoot = setupTree({ + 'src/index.ts': `import { x } from './config.js';\n`, + 'src/config.ts': `export const x = 1;\n`, + 'src/config.js': `export const x = 1;\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/index.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/config.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/config.js', language: 'javascript', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.ts']).toEqual(['src/config.js']); + }); + + it('still resolves traditional .js → .js imports unchanged', () => { + // The rewrite must not break the case where `.js` IS the real file on + // disk (pure JavaScript projects, untyped libraries). + projectRoot = setupTree({ + 'src/index.js': `import { x } from './util.js';\n`, + 'src/util.js': `export const x = 1;\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/index.js', language: 'javascript', fileCategory: 'code' }, + { path: 'src/util.js', language: 'javascript', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.js']).toEqual(['src/util.js']); + }); + + it('leaves the historical "no extension" probe behaviour intact', () => { + // An import like `./utils` (no extension) must still go through the + // append-extensions loop and resolve to `./utils.ts` — the new rewrite + // path is only triggered when the import already ends with a compiled + // extension. + projectRoot = setupTree({ + 'src/index.ts': `import { foo } from './utils';\n`, + 'src/utils.ts': `export function foo() {}\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [ + { path: 'src/index.ts', language: 'typescript', fileCategory: 'code' }, + { path: 'src/utils.ts', language: 'typescript', fileCategory: 'code' }, + ], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.ts']).toEqual(['src/utils.ts']); + }); + + it('returns null (no resolution) for a .js import whose .ts source is missing', () => { + // The rewrite must NOT silently invent a target when neither the .js nor + // the .ts file exists. The old behaviour would also return null for this + // case — we're verifying the rewrite path doesn't regress it. + projectRoot = setupTree({ + 'src/index.ts': `import './completely-missing.js';\n`, + }); + + const result = runScript(projectRoot, { + projectRoot, + files: [{ path: 'src/index.ts', language: 'typescript', fileCategory: 'code' }], + }); + + expect(result.status).toBe(0); + expect(result.output.importMap['src/index.ts']).toEqual([]); + }); }); describe('extract-import-map.mjs — Python resolver', () => { diff --git a/understand-anything-plugin/skills/understand/extract-import-map.mjs b/understand-anything-plugin/skills/understand/extract-import-map.mjs index 6c547d3..42dd7d4 100644 --- a/understand-anything-plugin/skills/understand/extract-import-map.mjs +++ b/understand-anything-plugin/skills/understand/extract-import-map.mjs @@ -369,15 +369,55 @@ const TS_EXT_PROBES = [ '/index.ts', '/index.tsx', '/index.js', '/index.jsx', ]; +/** + * NodeNext / Node16 / Bundler-with-explicit-extensions ESM TypeScript convention: + * TypeScript does NOT rewrite import specifiers during compilation, so source + * files import their COMPILED specifier (`./config.js`) even when only + * `./config.ts` exists on disk. We map each compiled-output extension to the + * TS source extensions that could have produced it, in priority order. + * + * Without this rewrite, ESM-TS projects (which is now the default for any new + * TS project) end up with a near-edgeless knowledge graph because every + * project-internal import fails to resolve. (#294) + */ +const NODENEXT_REWRITES = { + '.js': ['.ts', '.tsx', '.js', '.jsx'], + '.jsx': ['.tsx', '.jsx'], + '.mjs': ['.mts', '.mjs', '.ts'], + '.cjs': ['.cts', '.cjs', '.ts'], +}; + /** * Try ext probes against the file set for the given base path. Returns the * first matching project-relative path, or null. If the base path already has * a code extension AND exists in the file set, returns it directly. + * + * For NodeNext-style imports (`./foo.js` where only `./foo.ts` exists), apply + * the source-extension rewrite — see NODENEXT_REWRITES above. */ function probeWithExtensions(basePath, fileSet) { if (!basePath) return null; - // Exact match (import already had an extension) + // Exact match (import already had an extension that resolves on disk) if (fileSet.has(basePath)) return basePath; + + // NodeNext rewrite: if the basePath ends with a compiled-output extension + // but no such file exists, try the corresponding source extensions. We do + // this BEFORE the legacy "append extensions" loop because for an import + // like `./foo.js`, appending `.ts` would produce `foo.js.ts` (always wrong) + // while the correct candidate is `foo.ts`. + for (const [outExt, srcExts] of Object.entries(NODENEXT_REWRITES)) { + if (!basePath.endsWith(outExt)) continue; + const stem = basePath.slice(0, -outExt.length); + for (const srcExt of srcExts) { + const candidate = stem + srcExt; + if (fileSet.has(candidate)) return candidate; + } + // The basePath had an explicit compiled extension — don't fall through + // to the "append extensions" loop, which would produce nonsense like + // `foo.js.ts`. If NodeNext rewrite didn't find anything, return null. + return null; + } + for (const ext of TS_EXT_PROBES) { const candidate = basePath + ext; if (fileSet.has(candidate)) return candidate;