From 0cfced12eaff9c92684e5da7d1e1d21a9efb75b9 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Thu, 15 Oct 2026 09:34:00 -0700 Subject: [PATCH] fix(reference): resolve extensionless CommonJS requires to JSON files (#4164) The reference-source resolver probed only JavaScript and TypeScript extensions, so require("./data") stayed unresolved when the tree held data.json, which Node's CommonJS loader selects. Resolution now uses extension slots: a requires edge also tries X.json after X.js and before any directory index, and X/index.json after X/index.js, including a package main. ESM and TypeScript imports still need an explicit .json extension. Fixes #3915 Co-authored-by: Claude Opus 5.5 --- .../ReferenceSourceImportEntries.ts | 44 +++++++++++++++---- .../referenceSourceDirectoryImports.test.ts | 16 +++++++ .../referenceSourceDynamicImports.test.ts | 43 ++++++++++++++++++ 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/application/ReferenceSourceImportEntries.ts b/src/application/ReferenceSourceImportEntries.ts index fbea6e2e25..13b8662720 100644 --- a/src/application/ReferenceSourceImportEntries.ts +++ b/src/application/ReferenceSourceImportEntries.ts @@ -137,7 +137,23 @@ const readPackageMains = ( return mains; }; -const RESOLUTION_SUFFIXES = ["", ".ts", ".js", ".mjs", ".cjs", ".tsx", ".jsx"]; +/** + * Extension slots in probe order. Within a slot, files precede directory + * indexes. The CommonJS loader tries `X.json` after `X.js` and before any + * directory index; ESM and TypeScript require an explicit `.json`. + */ +const RESOLUTION_SUFFIXES: readonly (readonly string[])[] = [ + [""], + [".ts"], + [".js"], + [".mjs"], + [".cjs"], + [".tsx"], + [".jsx"], +]; +const REQUIRE_RESOLUTION_SUFFIXES = RESOLUTION_SUFFIXES.map((slot) => + slot.includes(".js") ? [...slot, ".json"] : slot, +); /** Inventory facts an internal specifier resolves against. */ interface SpecifierContext { @@ -153,6 +169,7 @@ interface SpecifierContext { const selectPackageMain = ( index: string, context: SpecifierContext, + slots: readonly (readonly string[])[], ): { to: string; resolution: "internal" | "unresolved" } => { const directory = posix.dirname(index); const manifest = context.packageMains.get(directory); @@ -170,11 +187,12 @@ const selectPackageMain = ( // Node would load a declared entry outside the imported root. if (logicalPathEscapesRoot(target)) return { to: target, resolution: "unresolved" }; + const suffixes = slots.flat(); const selected = context.lookupPath.resolveCandidates([ - ...RESOLUTION_SUFFIXES.map((suffix) => `${target}${suffix}`), - ...RESOLUTION_SUFFIXES.filter((suffix) => suffix !== "").map((suffix) => - posix.join(target, `index${suffix}`), - ), + ...suffixes.map((suffix) => `${target}${suffix}`), + ...suffixes + .filter((suffix) => suffix !== "") + .map((suffix) => posix.join(target, `index${suffix}`)), ]); return { to: selected ?? index, resolution: "internal" }; }; @@ -183,6 +201,7 @@ const resolveInternalSpecifier = ( fromPath: string, specifier: string, context: SpecifierContext, + slots: readonly (readonly string[])[], ): { to: string; resolution: "internal" | "unresolved" } => { if (posix.isAbsolute(specifier)) return { to: specifier, resolution: "unresolved" }; @@ -196,9 +215,13 @@ const resolveInternalSpecifier = ( specifier.endsWith("/") || [".", ".."].includes(posix.basename(specifier)); // A directory resolves only to an index with an extension; neither Node nor // TypeScript selects an extensionless `index` file. - const candidates = RESOLUTION_SUFFIXES.flatMap((suffix) => [ - ...(directoryReference ? [] : [`${normalized}${suffix}`]), - ...(suffix === "" ? [] : [posix.join(normalized, `index${suffix}`)]), + const candidates = slots.flatMap((slot) => [ + ...(directoryReference + ? [] + : slot.map((suffix) => `${normalized}${suffix}`)), + ...slot + .filter((suffix) => suffix !== "") + .map((suffix) => posix.join(normalized, `index${suffix}`)), ]); // Retain present runtime files first; substitute only a missing counterpart. // Counterparts cover explicit filenames, not directory/index resolution. @@ -214,7 +237,7 @@ const resolveInternalSpecifier = ( posix.basename(match).startsWith("index.") && posix.dirname(match).normalize("NFC") === normalized.normalize("NFC"); return resolvedThroughIndex - ? selectPackageMain(match, context) + ? selectPackageMain(match, context, slots) : { to: match, resolution: "internal" }; }; @@ -250,6 +273,9 @@ const appendFile = ( relationship.from_path, relationship.to, context, + relationship.kind === "requires" + ? REQUIRE_RESOLUTION_SUFFIXES + : RESOLUTION_SUFFIXES, ); output.relationships.push({ ...relationship, diff --git a/tests/boundary/cli/referenceSourceDirectoryImports.test.ts b/tests/boundary/cli/referenceSourceDirectoryImports.test.ts index 0d2f72421c..8bf5d94e51 100644 --- a/tests/boundary/cli/referenceSourceDirectoryImports.test.ts +++ b/tests/boundary/cli/referenceSourceDirectoryImports.test.ts @@ -34,6 +34,12 @@ describe.skipIf(process.platform === "win32")( ["package-main.cjs", "./pkg", "pkg/actual.js"], ["package-main-fallback.cjs", "./stale", "stale/index.js"], ["package-main-slash.cjs", "./slashed", "slashed/lib.js"], + ["json.cjs", "./data", "data.json"], + ["json-explicit.cjs", "./data.json", "data.json"], + ["json-before-index.cjs", "./conf", "conf.json"], + ["js-before-json.cjs", "./both", "both.js"], + ["json-index.cjs", "./settings", "settings/index.json"], + ["json-main.cjs", "./jpkg", "jpkg/config.json"], ] as const; const unresolved = [ ["missing.cjs", "./missing/", "missing/"], @@ -75,6 +81,16 @@ describe.skipIf(process.platform === "win32")( ["slashed/index.js", 'module.exports = "not selected";\n'], ["absolute/package.json", `${JSON.stringify({ main: outside })}\n`], ["absolute/index.js", 'module.exports = "not selected";\n'], + // CommonJS loads X.json after X.js and before any directory index. + ["data.json", '{"value":1}\n'], + ["conf.json", '{"value":"file before index"}\n'], + ["conf/index.js", 'module.exports = "not selected";\n'], + ["both.js", 'module.exports = "JavaScript first";\n'], + ["both.json", '{"value":"not selected"}\n'], + ["settings/index.json", '{"value":"JSON index"}\n'], + ["jpkg/package.json", '{"main":"config"}\n'], + ["jpkg/config.json", '{"value":"JSON main"}\n'], + ["jpkg/index.js", 'module.exports = "not selected";\n'], ...[...imports, ...unresolved].map( ([path, specifier]) => [path, `require(${JSON.stringify(specifier)});\n`] as const, diff --git a/tests/boundary/filesystem/referenceSourceDynamicImports.test.ts b/tests/boundary/filesystem/referenceSourceDynamicImports.test.ts index 91eab7ec60..b826e1dbae 100644 --- a/tests/boundary/filesystem/referenceSourceDynamicImports.test.ts +++ b/tests/boundary/filesystem/referenceSourceDynamicImports.test.ts @@ -63,3 +63,46 @@ it("resolves dynamic source imports while retaining computed targets as unknown" parse_state: "parsed", }); }); + +it("loads an extensionless JSON target only through CommonJS require", async () => { + const root = await createTestTempDirectory("rea-reference-json-require-"); + await Promise.all([ + writeFile(join(root, "main.cjs"), 'require("./data");\n'), + writeFile(join(root, "main.mjs"), 'import "./data";\n'), + writeFile(join(root, "data.json"), '{"value":1}\n'), + ]); + + const result = await importReferenceSource({ + root, + caller: "reference-json-require-test", + policy: { secretPatterns: [] }, + }); + if (!result.ok) throw result.error; + // ESM, unlike the CommonJS loader, requires the explicit `.json` extension. + expect(result.value.relationships).toEqual([ + { + from_path: "main.cjs", + to: "data.json", + kind: "requires", + resolution: "internal", + parse_state: "parsed", + }, + { + from_path: "main.mjs", + to: "data", + kind: "imports", + resolution: "unresolved", + parse_state: "parsed", + }, + ]); + expect( + result.value.entries.map((entry) => [ + entry.path, + "content_state" in entry ? entry.content_state : null, + ]), + ).toEqual([ + ["data.json", "hashed"], + ["main.cjs", "hashed"], + ["main.mjs", "hashed"], + ]); +});