From 034342ba28c8e56c3746be92b78aa7299afa02bd Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Wed, 14 Oct 2026 22:02:45 -0700 Subject: [PATCH] fix(reference): select a package main before its directory index (#3844) * fix(reference): select a package main before its directory index import-reference-source resolved a relative directory import such as require("./dep") from path candidates alone, so it reported dep/index.js as the internal, parsed dependency even when dep/package.json declared "main": "actual.js". Node loads actual.js. parseReferenceSourceEntries now reads each inventoried package.json once. When an internal specifier resolves through a directory index, a non-empty string main is loaded first, as a file and then as a directory index, as Node does. A main whose target is absent falls back to the index, matching Node's legacy fallback. An unreadable or invalid manifest leaves the relationship unresolved instead of asserting the index. Fixes #3383 Co-Authored-By: Claude Opus 5.5 * fix(reference): resolve package main the way path.resolve does A trailing separator in main ("lib/") kept directory intent, so the declared entry selected lib/index.js where Node loads lib.js. An absolute main was joined under the package directory instead of leaving the imported root. Strip trailing separators after joining, and report an absolute main as unresolved. Both cases join the Node-oracle CLI test. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- .../ReferenceSourceImportEntries.ts | 115 ++++++++++++++++-- .../referenceSourceDirectoryImports.test.ts | 29 ++++- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/src/application/ReferenceSourceImportEntries.ts b/src/application/ReferenceSourceImportEntries.ts index c53df6a9d1..fbea6e2e25 100644 --- a/src/application/ReferenceSourceImportEntries.ts +++ b/src/application/ReferenceSourceImportEntries.ts @@ -94,10 +94,95 @@ const failedEntry = ( }; }; +/** A package directory's legacy `main` entry point, as its manifest declares it. */ +type PackageMain = + | { readonly state: "declared"; readonly main: string } + | { readonly state: "none" } + | { readonly state: "unknown" }; + +/** + * Read each inventoried `package.json` once. Node selects a non-empty string + * `main` before a directory index; an unreadable or invalid manifest leaves + * that selection unknown. + */ +const readPackageMains = ( + read: ReferenceSourceRead, +): ReadonlyMap => { + const mains = new Map(); + for (const entry of read.entries) { + if (posix.basename(entry.path) !== "package.json") continue; + const directory = posix.dirname(entry.path); + if (entry.status !== "read" || entry.kind !== "file") { + mains.set(directory, { state: "unknown" }); + continue; + } + try { + const manifest: unknown = JSON.parse( + new TextDecoder("utf-8", { fatal: true }).decode(entry.bytes), + ); + const main = + typeof manifest === "object" && manifest !== null + ? Reflect.get(manifest, "main") + : undefined; + mains.set( + directory, + typeof main === "string" && main !== "" + ? { state: "declared", main } + : { state: "none" }, + ); + } catch { + mains.set(directory, { state: "unknown" }); + } + } + return mains; +}; + +const RESOLUTION_SUFFIXES = ["", ".ts", ".js", ".mjs", ".cjs", ".tsx", ".jsx"]; + +/** Inventory facts an internal specifier resolves against. */ +interface SpecifierContext { + readonly lookupPath: ReferenceSourcePathLookup; + readonly packageMains: ReadonlyMap; +} + +/** + * Apply a package `main` to a directory resolved through its index. Node loads + * the declared entry as a file, then as a directory index, and falls back to + * the package's own index only when neither exists. + */ +const selectPackageMain = ( + index: string, + context: SpecifierContext, +): { to: string; resolution: "internal" | "unresolved" } => { + const directory = posix.dirname(index); + const manifest = context.packageMains.get(directory); + if (manifest === undefined || manifest.state === "none") + return { to: index, resolution: "internal" }; + if (manifest.state === "unknown") + return { to: directory, resolution: "unresolved" }; + // Node resolves `main` with path.resolve: an absolute main leaves the + // package, and a trailing separator does not make the entry a directory. + if (posix.isAbsolute(manifest.main)) + return { to: posix.normalize(manifest.main), resolution: "unresolved" }; + const target = posix + .normalize(posix.join(directory, manifest.main)) + .replace(/(?<=.)\/+$/u, ""); + // Node would load a declared entry outside the imported root. + if (logicalPathEscapesRoot(target)) + return { to: target, resolution: "unresolved" }; + const selected = context.lookupPath.resolveCandidates([ + ...RESOLUTION_SUFFIXES.map((suffix) => `${target}${suffix}`), + ...RESOLUTION_SUFFIXES.filter((suffix) => suffix !== "").map((suffix) => + posix.join(target, `index${suffix}`), + ), + ]); + return { to: selected ?? index, resolution: "internal" }; +}; + const resolveInternalSpecifier = ( fromPath: string, specifier: string, - lookupPath: ReferenceSourcePathLookup, + context: SpecifierContext, ): { to: string; resolution: "internal" | "unresolved" } => { if (posix.isAbsolute(specifier)) return { to: specifier, resolution: "unresolved" }; @@ -109,10 +194,9 @@ const resolveInternalSpecifier = ( // Preserve directory intent before normalization removes terminal dot segments. const directoryReference = specifier.endsWith("/") || [".", ".."].includes(posix.basename(specifier)); - const suffixes = ["", ".ts", ".js", ".mjs", ".cjs", ".tsx", ".jsx"]; // A directory resolves only to an index with an extension; neither Node nor // TypeScript selects an extensionless `index` file. - const candidates = suffixes.flatMap((suffix) => [ + const candidates = RESOLUTION_SUFFIXES.flatMap((suffix) => [ ...(directoryReference ? [] : [`${normalized}${suffix}`]), ...(suffix === "" ? [] : [posix.join(normalized, `index${suffix}`)]), ]); @@ -124,9 +208,13 @@ const resolveInternalSpecifier = ( ({ path }) => path, ), ); - const match = lookupPath.resolveCandidates(candidates); - return match === undefined - ? { to: normalized, resolution: "unresolved" } + const match = context.lookupPath.resolveCandidates(candidates); + if (match === undefined) return { to: normalized, resolution: "unresolved" }; + const resolvedThroughIndex = + posix.basename(match).startsWith("index.") && + posix.dirname(match).normalize("NFC") === normalized.normalize("NFC"); + return resolvedThroughIndex + ? selectPackageMain(match, context) : { to: match, resolution: "internal" }; }; @@ -134,7 +222,7 @@ type ReadFile = Extract; const appendFile = ( entry: ReadFile, - lookupPath: ReferenceSourcePathLookup, + context: SpecifierContext, output: ParsedReferenceSourceEntries, ): void => { const language = detectReferenceSourceLanguage(entry.path); @@ -161,7 +249,7 @@ const appendFile = ( const resolved = resolveInternalSpecifier( relationship.from_path, relationship.to, - lookupPath, + context, ); output.relationships.push({ ...relationship, @@ -174,11 +262,11 @@ const appendFile = ( const appendReadEntry = ( entry: Extract, - lookupPath: ReferenceSourcePathLookup, + context: SpecifierContext, output: ParsedReferenceSourceEntries, ): void => { if (entry.kind === "file") { - appendFile(entry, lookupPath, output); + appendFile(entry, context, output); return; } const classifications = classifyReferenceSourcePath(entry.path); @@ -221,11 +309,14 @@ export const parseReferenceSourceEntries = ( parseFailures: [], limitations: [...read.limitations], }; - const lookupPath = createReferenceSourcePathLookup(filePaths); + const context: SpecifierContext = { + lookupPath: createReferenceSourcePathLookup(filePaths), + packageMains: readPackageMains(read), + }; for (const entry of read.entries) { if (signal?.aborted === true) break; if (entry.status === "failed") output.entries.push(failedEntry(entry)); - else appendReadEntry(entry, lookupPath, output); + else appendReadEntry(entry, context, output); } return output; }; diff --git a/tests/boundary/cli/referenceSourceDirectoryImports.test.ts b/tests/boundary/cli/referenceSourceDirectoryImports.test.ts index 8fbd8a421c..0d2f72421c 100644 --- a/tests/boundary/cli/referenceSourceDirectoryImports.test.ts +++ b/tests/boundary/cli/referenceSourceDirectoryImports.test.ts @@ -19,6 +19,7 @@ describe.skipIf(process.platform === "win32")( "rea-reference-directory-imports-", ); const root = join(workspace, "source"); + const outside = join(workspace, "outside", "index.js"); const imports = [ ["slash.cjs", "./lib/", "lib/index.js"], ["dot.cjs", "./lib/.", "lib/index.js"], @@ -30,12 +31,17 @@ describe.skipIf(process.platform === "win32")( ["fallback.cjs", "./fallback", "fallback/index.js"], ["legacy-order.cjs", "./dep", "dep/index.js"], ["bare-index.cjs", "./bare", "bare.js"], + ["package-main.cjs", "./pkg", "pkg/actual.js"], + ["package-main-fallback.cjs", "./stale", "stale/index.js"], + ["package-main-slash.cjs", "./slashed", "slashed/lib.js"], ] as const; const unresolved = [ ["missing.cjs", "./missing/", "missing/"], ["escape.cjs", "../outside/", "../outside/"], ["main.ts", "./folder.js/.", "folder.js"], ["only-bare.cjs", "./only/", "only/"], + ["invalid-manifest.cjs", "./broken", "broken"], + ["absolute-main.cjs", "./absolute", outside], ] as const; const files = new Map([ ["index.js", 'module.exports = "root index";\n'], @@ -53,6 +59,22 @@ describe.skipIf(process.platform === "win32")( ["bare.js", 'module.exports = "file before directory";\n'], ["bare/index", 'module.exports = "extensionless index";\n'], ["only/index", 'module.exports = "extensionless index";\n'], + // A package main outranks the index; a missing main falls back to it. + ["pkg/package.json", '{"main":"actual.js"}\n'], + ["pkg/actual.js", 'module.exports = "declared main";\n'], + ["pkg/index.js", 'module.exports = "ignored index";\n'], + ["stale/package.json", '{"main":"gone.js"}\n'], + ["stale/index.js", 'module.exports = "legacy fallback";\n'], + ["broken/package.json", '{"main":\n'], + ["broken/index.js", 'module.exports = "unknown selection";\n'], + // Node resolves main with path.resolve: no trailing-slash directory + // intent, and an absolute main leaves the imported root. + ["slashed/package.json", '{"main":"lib/"}\n'], + ["slashed/lib.js", 'module.exports = "file main";\n'], + ["slashed/lib/index.js", 'module.exports = "not selected";\n'], + ["slashed/index.js", 'module.exports = "not selected";\n'], + ["absolute/package.json", `${JSON.stringify({ main: outside })}\n`], + ["absolute/index.js", 'module.exports = "not selected";\n'], ...[...imports, ...unresolved].map( ([path, specifier]) => [path, `require(${JSON.stringify(specifier)});\n`] as const, @@ -65,7 +87,6 @@ describe.skipIf(process.platform === "win32")( await writeFile(target, contents); }), ); - const outside = join(workspace, "outside", "index.js"); await mkdir(dirname(outside), { recursive: true }); await writeFile(outside, 'module.exports = "outside import root";\n'); @@ -83,9 +104,15 @@ describe.skipIf(process.platform === "win32")( expect(() => createRequire(join(root, "only-bare.cjs")).resolve("./only/"), ).toThrow(); + expect(() => + createRequire(join(root, "invalid-manifest.cjs")).resolve("./broken"), + ).toThrow(); expect( createRequire(join(root, "escape.cjs")).resolve("../outside/"), ).toBe(outside); + expect( + createRequire(join(root, "absolute-main.cjs")).resolve("./absolute"), + ).toBe(outside); const result = await cli.run({ arguments: ["import-reference-source", root, "--json"],