diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index 2a05af6eb7..e44d2666ac 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -121,7 +121,66 @@ describe("loader-owned path lookup", () => { }), ).toMatchObject({ resolved_path: "app/feature.cjs" }); }); +}); +describe("bare package root file lookup", () => { + it("uses loader-specific bare package roots when only a sibling .js file exists", () => { + const absentDirectory = fileMap([ + file("app/entry.mjs", "root"), + file("app/node_modules/fixture.js", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files: absentDirectory, + }), + ).toMatchObject({ resolution_status: "external", resolved_path: null }); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "require", + files: absentDirectory, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture.js", + }); + + const packageDirectory = fileMap([ + file("app/entry.mjs", "root"), + file("app/node_modules/fixture.js", "root"), + file("app/node_modules/fixture/package.json", "root", "{}"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files: packageDirectory, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "require", + files: packageDirectory, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture.js", + }); + }); +}); + +describe("legacy package main lookup", () => { it("treats an empty package main as absent and resolves the root index", () => { const files = fileMap([ file("app/src/entry.cjs", "root"), @@ -208,6 +267,133 @@ describe("loader-owned path lookup", () => { }); }); +describe("CommonJS bare package ancestor lookup", () => { + it.each([ + ["absent metadata", undefined, true], + ["empty metadata", "{}", true], + ["empty main", '{"main":""}', true], + ["truthy missing main", '{"main":"missing"}', false], + ] as const)( + "handles %s before an ancestor package for both loaders", + (_name, packageJson, commonJsUsesAncestor) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/placeholder.js", "root"), + ...(packageJson === undefined + ? [] + : [ + file( + "app/src/node_modules/fixture/package.json", + "root", + packageJson, + ), + ]), + file("app/node_modules/fixture/index.js", "root"), + ]); + for (const moduleKind of ["import", "require"] as const) { + const resolution = resolve({ + declaredPath: "fixture", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind, + files, + }); + if (moduleKind === "require" && commonJsUsesAncestor) { + expect(resolution).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture/index.js", + }); + } else { + expect(resolution).toMatchObject({ + resolution_status: "not-found", + resolved_path: null, + }); + } + } + }, + ); +}); + +describe("CommonJS bare package subpath ancestor lookup", () => { + it.each([ + ["absent subpath", undefined, true], + ["empty subpath metadata", "{}", true], + ["truthy missing subpath main", '{"main":"missing"}', false], + ] as const)( + "handles %s before an ancestor package for both loaders", + (_name, packageJson, commonJsUsesAncestor) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/placeholder.js", "root"), + ...(packageJson === undefined + ? [] + : [ + file( + "app/src/node_modules/fixture/parser/package.json", + "root", + packageJson, + ), + file( + "app/src/node_modules/fixture/parser/placeholder.js", + "root", + ), + ]), + file("app/node_modules/fixture/parser.js", "root"), + ]); + const commonJsResolution = resolve({ + declaredPath: "fixture/parser", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "require", + files, + }); + expect(commonJsResolution).toMatchObject({ + resolved_path: commonJsUsesAncestor + ? "app/node_modules/fixture/parser.js" + : null, + }); + const esmResolution = resolve({ + declaredPath: "fixture/parser", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "import", + files, + }); + expect(esmResolution.resolved_path).toBeNull(); + }, + ); +}); + +describe("CommonJS bare package exhausted lookup", () => { + it.each([ + ["empty main", "fixture", '{"main":""}', "app/src/node_modules/fixture"], + [ + "missing subpath", + "fixture/parser", + "{}", + "app/src/node_modules/fixture/parser", + ], + ] as const)( + "preserves not-found for an exhausted %s lookup", + (_name, declaredPath, packageJson, packageDirectory) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/package.json", "root", packageJson), + file(`${packageDirectory}/placeholder.js`, "root"), + ]); + expect( + resolve({ + declaredPath, + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + }, + ); +}); + describe("logical path identity", () => { it("resolves an NFD relative import to the NFC inventory path and rejects a root escape", () => { const nfc = "caf\u00e9".normalize("NFC"); diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index 481a40453e..9845e8c033 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -100,6 +100,8 @@ type CandidateResolution = readonly resolvedPath: null; readonly status: UnresolvedArtifactPathStatus; readonly limitations: readonly string[]; + /** A truthy package main failed, so Node must not try ancestor packages. */ + readonly stopsAncestorLookup?: true; }; /** Resolve one declaration while preserving its original input identity. */ @@ -274,23 +276,39 @@ const bareModuleCandidate = ( } const source = input.files.get(input.sourcePath); let directory = posix.dirname(input.sourcePath); + let nearestNotFound: CandidateResolution | null = null; while (true) { const candidate = posix.join(directory, dependencyDirectory); const metadata = readPackageMetadata(input, candidate); - if (request.dependencyExportsEligible && metadata.kind !== "absent") - return resolvePackageDependency(input, candidate, request, metadata); - if (request.packageSubpath === ".") { - const direct = resolveFileCandidates( - input, - fileCandidates(candidate, input.moduleKind), - ); - if (direct !== null) return direct; + const continueCommonJsLookup = + input.moduleKind === "require" && + (metadata.kind === "absent" || + (metadata.kind === "available" && + (!request.dependencyExportsEligible || + metadata.exports === undefined))); + const packageResolution = + metadata.kind !== "absent" || + hasContainerDirectory(input.files, candidate, source?.container_sha256) + ? resolvePackageDependency(input, candidate, request, metadata) + : request.packageSubpath === "." && input.moduleKind !== "import" + ? resolveFileCandidates( + input, + fileCandidates(candidate, input.moduleKind), + ) + : null; + if (packageResolution !== null) { + if ( + continueCommonJsLookup && + packageResolution.status === "not-found" && + packageResolution.stopsAncestorLookup !== true + ) { + nearestNotFound ??= packageResolution; + } else return packageResolution; } - if (hasContainerDirectory(input.files, candidate, source?.container_sha256)) - return resolvePackageDependency(input, candidate, request, metadata); if (directory === "." || directory === "") break; directory = posix.dirname(directory); } + if (nearestNotFound !== null) return nearestNotFound; return unresolvedCandidate("external", [ "No matching bare package was inventoried in an enclosing node_modules directory.", ]); @@ -425,10 +443,12 @@ const resolvePackageDependency = ( if (rawExports === undefined) { if (subpath === ".") return ( - resolveFileCandidates( - input, - fileCandidates(directory, input.moduleKind), - ) ?? resolveDirectory(input, directory, metadata, new Set()) + (input.moduleKind === "import" + ? null + : resolveFileCandidates( + input, + fileCandidates(directory, input.moduleKind), + )) ?? resolveDirectory(input, directory, metadata, new Set()) ); // Resolve legacy ESM subpaths as URLs: suffixes identify the loaded module // but do not belong to its filesystem path. Keep the declared specifier on @@ -831,6 +851,8 @@ const resolveDirectory = ( ]) ?? resolveFileCandidates(input, indexCandidates(directory, input.moduleKind)); if (selected !== null) return selected; + if (input.moduleKind === "require") + return { ...notFoundCandidate(), stopsAncestorLookup: true }; if (input.moduleKind !== undefined) return notFoundCandidate(); return resolveCandidate( entryInput, @@ -1102,7 +1124,10 @@ const packageExportConditions = ( "default", ]); -const notFoundCandidate = (): CandidateResolution => ({ +const notFoundCandidate = (): Extract< + CandidateResolution, + { readonly resolvedPath: null } +> => ({ resolvedPath: null, status: "not-found", limitations: [ diff --git a/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts index e57f96adb4..3e33183278 100644 --- a/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts +++ b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts @@ -21,6 +21,10 @@ const createModuleLookupFixture = async () => { 'import "./exact.js";', 'import fallback from "empty-main";', 'import "nested-main";', + 'import "file-only";', + 'import "ancestor-empty";', + 'import "ancestor-blocked";', + 'import "ancestor-sub/parser";', "void fallback;", ].join("\n"), "esm/extensionless.js": "module.exports = 1;", @@ -36,6 +40,10 @@ const createModuleLookupFixture = async () => { 'require("./default-node");', 'require("empty-main");', 'require("nested-main");', + 'require("file-only");', + 'require("ancestor-empty");', + 'require("ancestor-blocked");', + 'require("ancestor-sub/parser");', ].join("\n"), "cjs/extensionless.cjs": "module.exports = 1;", "cjs/feature.cjs": "module.exports = 2;", @@ -50,8 +58,12 @@ const createModuleLookupFixture = async () => { main: "other", }), "node_modules/nested-main/sub/other.js": "module.exports = 99;", - "runtime-control.mjs": [ - 'const specifiers = ["./esm/extensionless", "./esm/directory", "./esm/explicit.js", "./esm/exact.js", "empty-main", "nested-main"];', + "node_modules/file-only.js": "module.exports = 7;", + "node_modules/ancestor-empty/index.js": "module.exports = 8;", + "node_modules/ancestor-blocked/index.js": "module.exports = 9;", + "node_modules/ancestor-sub/parser.js": "module.exports = 10;", + "esm/runtime-control.mjs": [ + 'const specifiers = ["./extensionless", "./directory", "./explicit.js", "./exact.js", "empty-main", "nested-main", "file-only", "ancestor-empty", "ancestor-blocked", "ancestor-sub/parser"];', "const results = [];", "for (const specifier of specifiers) {", " try { await import(specifier); results.push({ specifier, ok: true }); }", @@ -59,8 +71,8 @@ const createModuleLookupFixture = async () => { "}", "console.log(JSON.stringify(results));", ].join("\n"), - "runtime-control.cjs": [ - 'const specifiers = ["./cjs/extensionless", "./cjs/feature", "./cjs/feature.cjs", "./cjs/default-js", "./cjs/default-json", "empty-main", "nested-main"];', + "cjs/runtime-control.cjs": [ + 'const specifiers = ["./extensionless", "./feature", "./feature.cjs", "./default-js", "./default-json", "empty-main", "nested-main", "file-only", "ancestor-empty", "ancestor-blocked", "ancestor-sub/parser"];', "const results = [];", "for (const specifier of specifiers) {", " try { require(specifier); results.push({ specifier, ok: true }); }", @@ -69,6 +81,15 @@ const createModuleLookupFixture = async () => { "console.log(JSON.stringify(results));", ].join("\n"), }); + for (const directory of ["esm", "cjs"]) + await writeFixtureFiles(root, { + [`${directory}/node_modules/ancestor-empty/package.json`]: JSON.stringify( + { main: "" }, + ), + [`${directory}/node_modules/ancestor-blocked/package.json`]: + JSON.stringify({ main: "missing" }), + [`${directory}/node_modules/ancestor-sub/package.json`]: "{}", + }); return root; }; @@ -77,11 +98,11 @@ cliTest.for(["CLI", "stdio MCP"] as const)( async (surface, { cli }) => { const root = await createModuleLookupFixture(); const [esmControl, cjsControl] = await Promise.all([ - execute(process.execPath, ["runtime-control.mjs"], { + execute(process.execPath, ["esm/runtime-control.mjs"], { cwd: root, timeout: 5_000, }), - execute(process.execPath, ["runtime-control.cjs"], { + execute(process.execPath, ["cjs/runtime-control.cjs"], { cwd: root, timeout: 5_000, }), @@ -98,40 +119,56 @@ cliTest.for(["CLI", "stdio MCP"] as const)( }>; expect(esmResults).toEqual([ { - specifier: "./esm/extensionless", + specifier: "./extensionless", ok: false, code: "ERR_MODULE_NOT_FOUND", }, { - specifier: "./esm/directory", + specifier: "./directory", ok: false, code: "ERR_UNSUPPORTED_DIR_IMPORT", }, { - specifier: "./esm/explicit.js", + specifier: "./explicit.js", ok: false, code: "ERR_MODULE_NOT_FOUND", }, - { specifier: "./esm/exact.js", ok: true }, + { specifier: "./exact.js", ok: true }, { specifier: "empty-main", ok: true }, { specifier: "nested-main", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { specifier: "file-only", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { specifier: "ancestor-empty", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { + specifier: "ancestor-blocked", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, + { + specifier: "ancestor-sub/parser", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, ]); expect(cjsResults).toEqual([ { - specifier: "./cjs/extensionless", + specifier: "./extensionless", ok: false, code: "MODULE_NOT_FOUND", }, { - specifier: "./cjs/feature", + specifier: "./feature", ok: false, code: "MODULE_NOT_FOUND", }, - { specifier: "./cjs/feature.cjs", ok: true }, - { specifier: "./cjs/default-js", ok: true }, - { specifier: "./cjs/default-json", ok: true }, + { specifier: "./feature.cjs", ok: true }, + { specifier: "./default-js", ok: true }, + { specifier: "./default-json", ok: true }, { specifier: "empty-main", ok: true }, { specifier: "nested-main", ok: false, code: "MODULE_NOT_FOUND" }, + { specifier: "file-only", ok: true }, + { specifier: "ancestor-empty", ok: true }, + { specifier: "ancestor-blocked", ok: false, code: "MODULE_NOT_FOUND" }, + { specifier: "ancestor-sub/parser", ok: true }, ]); const { graph } = await analyzeJavaScriptDirectory(surface, cli, root); @@ -142,6 +179,10 @@ cliTest.for(["CLI", "stdio MCP"] as const)( ["import", "./exact.js", "esm/exact.js", "resolved"], ["import", "empty-main", "node_modules/empty-main/index.js", "resolved"], ["import", "nested-main", null, "not-found"], + ["import", "file-only", null, "external"], + ["import", "ancestor-empty", null, "not-found"], + ["import", "ancestor-blocked", null, "not-found"], + ["import", "ancestor-sub/parser", null, "not-found"], ["require", "./extensionless", null, "not-found"], ["require", "./feature", null, "not-found"], ["require", "./feature.cjs", "cjs/feature.cjs", "resolved"], @@ -150,6 +191,20 @@ cliTest.for(["CLI", "stdio MCP"] as const)( ["require", "./default-node", "cjs/default-node.node", "resolved"], ["require", "empty-main", "node_modules/empty-main/index.js", "resolved"], ["require", "nested-main", null, "not-found"], + ["require", "file-only", "node_modules/file-only.js", "resolved"], + [ + "require", + "ancestor-empty", + "node_modules/ancestor-empty/index.js", + "resolved", + ], + ["require", "ancestor-blocked", null, "not-found"], + [ + "require", + "ancestor-sub/parser", + "node_modules/ancestor-sub/parser.js", + "resolved", + ], ] as const; for (const [kind, specifier, resolvedPath, status] of expectations)