fix(javascript): prefer package entrypoints over directory indexes (#575)
This commit is contained in:
Kaoru0822
GitHub
parent
063cd25a93
commit
bb91abd3df
|
|
@ -326,6 +326,149 @@ describe("contextual JavaScript package path resolution", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("directory package entrypoint precedence", () => {
|
||||
it.each([
|
||||
["require", '{"main":"actual.cjs"}', "actual.cjs"],
|
||||
["require", '{"exports":{"require":"./actual.cjs"}}', "actual.cjs"],
|
||||
["import", '{"exports":{"import":"./actual.mjs"}}', "actual.mjs"],
|
||||
] as const)(
|
||||
"prefers declared %s entrypoints over index files",
|
||||
(moduleKind, metadata, entry) => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file("app/node_modules/fixture/package.json", "root", metadata),
|
||||
file(`app/node_modules/fixture/${entry}`, "root"),
|
||||
file("app/node_modules/fixture/index.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "fixture",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind,
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: `app/node_modules/fixture/${entry}`,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["{}", '{"main":"."}'])(
|
||||
"preserves index fallback for package metadata %s",
|
||||
(metadata) => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file("app/entry/package.json", "root", metadata),
|
||||
file("app/entry/index.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./entry",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: "app/entry/index.js",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each([null, "{"])(
|
||||
"keeps unreadable or invalid metadata explicit even when an index exists",
|
||||
(metadata) => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file("app/entry/package.json", "root", metadata),
|
||||
file("app/entry/index.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./entry",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "unavailable",
|
||||
resolved_path: null,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("does not replace a missing exports target with an index", () => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file(
|
||||
"app/node_modules/fixture/package.json",
|
||||
"root",
|
||||
'{"exports":"./missing.js"}',
|
||||
),
|
||||
file("app/node_modules/fixture/index.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "fixture",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "not-found",
|
||||
resolved_path: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps direct files ahead of a relative directory package", () => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file("app/entry.js", "root"),
|
||||
file("app/entry/package.json", "root", '{"main":"actual.js"}'),
|
||||
file("app/entry/actual.js", "root"),
|
||||
file("app/entry/index.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./entry",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: "app/entry.js",
|
||||
});
|
||||
});
|
||||
|
||||
it("retains cycle diagnostics when directory packages refer to each other", () => {
|
||||
const files = fileMap([
|
||||
file("app/consumer.js", "root"),
|
||||
file("app/a/package.json", "root", '{"main":"../b"}'),
|
||||
file("app/b/package.json", "root", '{"main":"../a"}'),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./a",
|
||||
sourcePath: "app/consumer.js",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "unavailable",
|
||||
resolved_path: null,
|
||||
limitations: [expect.stringContaining("cycle")],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("contextual JavaScript module identity", () => {
|
||||
it("retains CommonJS and ESM file-base identity during inert extraction", () => {
|
||||
const analysis = analyzeJavaScriptStaticSource(
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ export const resolveArtifactPathByContext = (
|
|||
const resolvePackagePath = (
|
||||
input: ResolveArtifactPathInput,
|
||||
packageChain: ReadonlySet<string>,
|
||||
mode: "full" | "files-and-index" = "full",
|
||||
): ArtifactPathResolution => {
|
||||
const rejected = rejectDeclaration(input);
|
||||
if (rejected !== null) return rejected;
|
||||
|
|
@ -85,7 +86,13 @@ const resolvePackagePath = (
|
|||
if (typeof candidate !== "string") return candidate;
|
||||
const confined = confineCandidate(input, candidate);
|
||||
if (typeof confined !== "string") return confined;
|
||||
const resolved = resolveCandidate(input, confined, packageChain);
|
||||
const resolved =
|
||||
mode === "files-and-index"
|
||||
? (resolveFileCandidates(input, [
|
||||
...fileCandidates(confined),
|
||||
...indexCandidates(confined),
|
||||
]) ?? notFoundCandidate())
|
||||
: resolveCandidate(input, confined, packageChain);
|
||||
return outcome(input, resolved);
|
||||
};
|
||||
|
||||
|
|
@ -182,16 +189,18 @@ const hasContainerCandidate = (
|
|||
candidate: string,
|
||||
containerSha256: string | undefined,
|
||||
): boolean =>
|
||||
[...directCandidates(candidate), posix.join(candidate, "package.json")].some(
|
||||
(path) => {
|
||||
const file = files.get(path);
|
||||
return (
|
||||
file !== undefined &&
|
||||
(containerSha256 === undefined ||
|
||||
file.container_sha256 === containerSha256)
|
||||
);
|
||||
},
|
||||
);
|
||||
[
|
||||
...fileCandidates(candidate),
|
||||
...indexCandidates(candidate),
|
||||
posix.join(candidate, "package.json"),
|
||||
].some((path) => {
|
||||
const file = files.get(path);
|
||||
return (
|
||||
file !== undefined &&
|
||||
(containerSha256 === undefined ||
|
||||
file.container_sha256 === containerSha256)
|
||||
);
|
||||
});
|
||||
|
||||
const htmlCandidate = (
|
||||
input: ResolveArtifactPathInput,
|
||||
|
|
@ -237,16 +246,9 @@ const resolveCandidate = (
|
|||
candidate: string,
|
||||
packageChain: ReadonlySet<string>,
|
||||
): CandidateResolution => {
|
||||
const direct = resolveFileCandidates(input, fileCandidates(candidate));
|
||||
if (direct !== null) return direct;
|
||||
const source = input.files.get(input.sourcePath);
|
||||
for (const path of directCandidates(candidate)) {
|
||||
const target = input.files.get(path);
|
||||
if (
|
||||
target !== undefined &&
|
||||
(source === undefined ||
|
||||
target.container_sha256 === source.container_sha256)
|
||||
)
|
||||
return { resolvedPath: path, status: "resolved", limitations: [] };
|
||||
}
|
||||
const packagePath = posix.join(candidate, "package.json");
|
||||
const packageFile = input.files.get(packagePath);
|
||||
if (
|
||||
|
|
@ -254,7 +256,10 @@ const resolveCandidate = (
|
|||
(source !== undefined &&
|
||||
packageFile.container_sha256 !== source.container_sha256)
|
||||
)
|
||||
return notFoundCandidate();
|
||||
return (
|
||||
resolveFileCandidates(input, indexCandidates(candidate)) ??
|
||||
notFoundCandidate()
|
||||
);
|
||||
if (!packageFile.text.included)
|
||||
return {
|
||||
resolvedPath: null,
|
||||
|
|
@ -263,15 +268,14 @@ const resolveCandidate = (
|
|||
`Directory package metadata ${packagePath} was inventoried but its text is unavailable: ${packageFile.text.reason}.`,
|
||||
],
|
||||
};
|
||||
if (packageChain.has(packagePath))
|
||||
return {
|
||||
resolvedPath: null,
|
||||
status: "unavailable",
|
||||
limitations: [
|
||||
`Directory package entrypoint cycle includes ${packagePath}.`,
|
||||
],
|
||||
};
|
||||
const main = packageEntry(packageFile.text.value, input.moduleKind);
|
||||
const main = packageEntry(
|
||||
packageFile.text.value,
|
||||
input.moduleKind,
|
||||
input.context === "module-specifier" &&
|
||||
!input.declaredPath.startsWith(".") &&
|
||||
!input.declaredPath.startsWith("/") &&
|
||||
!hasScheme(input.declaredPath),
|
||||
);
|
||||
if (main.status === "invalid")
|
||||
return {
|
||||
resolvedPath: null,
|
||||
|
|
@ -280,17 +284,41 @@ const resolveCandidate = (
|
|||
`Directory package metadata ${packagePath} is not valid package JSON.`,
|
||||
],
|
||||
};
|
||||
if (main.status === "missing") return notFoundCandidate();
|
||||
const nested = resolvePackagePath(
|
||||
{
|
||||
declaredPath: main.value,
|
||||
sourcePath: packagePath,
|
||||
context: "package-entrypoint",
|
||||
files: input.files,
|
||||
},
|
||||
new Set([...packageChain, packagePath]),
|
||||
);
|
||||
return nested.resolution_status === "resolved"
|
||||
if (packageChain.has(packagePath)) {
|
||||
return {
|
||||
resolvedPath: null,
|
||||
status: "unavailable",
|
||||
limitations: [
|
||||
`Directory package entrypoint cycle includes ${packagePath}.`,
|
||||
],
|
||||
};
|
||||
}
|
||||
if (main.status === "missing")
|
||||
return (
|
||||
resolveFileCandidates(input, indexCandidates(candidate)) ??
|
||||
notFoundCandidate()
|
||||
);
|
||||
const entryInput: ResolveArtifactPathInput = {
|
||||
declaredPath: main.value,
|
||||
sourcePath: packagePath,
|
||||
context: "package-entrypoint",
|
||||
files: input.files,
|
||||
};
|
||||
const chain = new Set([...packageChain, packagePath]);
|
||||
if (main.source === "legacy") {
|
||||
const entry = resolvePackagePath(entryInput, chain, "files-and-index");
|
||||
if (entry.resolution_status !== "not-found") return candidateOutcome(entry);
|
||||
const index = resolveFileCandidates(input, indexCandidates(candidate));
|
||||
if (index !== null) return index;
|
||||
}
|
||||
// Preserve recursive artifact package lookup only after legacy file/index fallbacks.
|
||||
return candidateOutcome(resolvePackagePath(entryInput, chain));
|
||||
};
|
||||
|
||||
const candidateOutcome = (
|
||||
nested: ArtifactPathResolution,
|
||||
): CandidateResolution =>
|
||||
nested.resolution_status === "resolved"
|
||||
? {
|
||||
resolvedPath: nested.resolved_path,
|
||||
status: nested.resolution_status,
|
||||
|
|
@ -301,19 +329,42 @@ const resolveCandidate = (
|
|||
status: nested.resolution_status,
|
||||
limitations: nested.limitations,
|
||||
};
|
||||
|
||||
const resolveFileCandidates = (
|
||||
input: ResolveArtifactPathInput,
|
||||
candidates: readonly string[],
|
||||
): CandidateResolution | null => {
|
||||
const source = input.files.get(input.sourcePath);
|
||||
for (const path of candidates) {
|
||||
const target = input.files.get(path);
|
||||
if (
|
||||
target !== undefined &&
|
||||
(source === undefined ||
|
||||
target.container_sha256 === source.container_sha256)
|
||||
)
|
||||
return { resolvedPath: path, status: "resolved", limitations: [] };
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
const directCandidates = (candidate: string): readonly string[] => [
|
||||
const fileCandidates = (candidate: string): readonly string[] => [
|
||||
candidate,
|
||||
...EXTENSIONS.map((extension) => `${candidate}${extension}`),
|
||||
...EXTENSIONS.map((extension) => posix.join(candidate, `index${extension}`)),
|
||||
];
|
||||
|
||||
const indexCandidates = (candidate: string): readonly string[] =>
|
||||
EXTENSIONS.map((extension) => posix.join(candidate, `index${extension}`));
|
||||
|
||||
const packageEntry = (
|
||||
text: string,
|
||||
moduleKind: ResolveArtifactPathInput["moduleKind"],
|
||||
useExports: boolean,
|
||||
):
|
||||
| { readonly status: "value"; readonly value: string }
|
||||
| {
|
||||
readonly status: "value";
|
||||
readonly value: string;
|
||||
readonly source: "legacy" | "exports";
|
||||
}
|
||||
| { readonly status: "missing" }
|
||||
| { readonly status: "invalid" } => {
|
||||
try {
|
||||
|
|
@ -321,14 +372,20 @@ const packageEntry = (
|
|||
if (typeof value !== "object" || value === null)
|
||||
return { status: "invalid" };
|
||||
const rawExports = Reflect.get(value, "exports");
|
||||
if (rawExports !== undefined) return packageExport(rawExports, moduleKind);
|
||||
if (useExports && rawExports !== undefined && rawExports !== null) {
|
||||
const exported = packageExport(rawExports, moduleKind);
|
||||
return exported.status === "value"
|
||||
? { ...exported, source: "exports" }
|
||||
: exported;
|
||||
}
|
||||
const preferred =
|
||||
moduleKind === "import"
|
||||
? [Reflect.get(value, "module"), Reflect.get(value, "main")]
|
||||
: [Reflect.get(value, "main"), Reflect.get(value, "module")];
|
||||
const entry = preferred.find((candidate) => candidate !== undefined);
|
||||
if (entry === undefined) return { status: "missing" };
|
||||
return packagePathValue(entry);
|
||||
const legacy = packagePathValue(entry);
|
||||
return legacy.status === "value" ? { ...legacy, source: "legacy" } : legacy;
|
||||
} catch {
|
||||
return { status: "invalid" };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,142 @@
|
|||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import { createRequire } from "node:module";
|
||||
import { dirname, join, relative, sep } from "node:path";
|
||||
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
import { reconstructJavaScriptArtifact } from "../../../src/application/JavaScriptArtifactReconstruction.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
const cases: readonly {
|
||||
name: string;
|
||||
metadata: Readonly<Record<string, unknown>>;
|
||||
index: boolean;
|
||||
specifier?: string;
|
||||
extraFiles?: Readonly<Record<string, string>>;
|
||||
}[] = [
|
||||
{ name: "main before index", metadata: { main: "actual.cjs" }, index: true },
|
||||
{
|
||||
name: "exports before index",
|
||||
metadata: { exports: { require: "./actual.cjs" } },
|
||||
index: true,
|
||||
},
|
||||
{
|
||||
name: "main without an index",
|
||||
metadata: { main: "actual.cjs" },
|
||||
index: false,
|
||||
},
|
||||
{ name: "index fallback", metadata: {}, index: true },
|
||||
{
|
||||
name: "missing legacy main fallback",
|
||||
metadata: { main: "missing.cjs" },
|
||||
index: true,
|
||||
},
|
||||
{ name: "self-directory main", metadata: { main: "." }, index: true },
|
||||
{
|
||||
name: "bare exports with legacy main",
|
||||
metadata: { main: "legacy.cjs", exports: "./actual.cjs" },
|
||||
index: true,
|
||||
extraFiles: {
|
||||
"node_modules/fixture/legacy.cjs": "module.exports = 'legacy';",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "relative directory ignores exports",
|
||||
metadata: { main: "legacy.cjs", exports: "./actual.cjs" },
|
||||
index: true,
|
||||
specifier: "./node_modules/fixture",
|
||||
extraFiles: {
|
||||
"node_modules/fixture/legacy.cjs": "module.exports = 'legacy';",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "relative exports-only directory uses index",
|
||||
metadata: { exports: "./actual.cjs" },
|
||||
index: true,
|
||||
specifier: "./node_modules/fixture",
|
||||
},
|
||||
{
|
||||
name: "legacy main directory uses its index",
|
||||
metadata: { main: "./lib" },
|
||||
index: true,
|
||||
extraFiles: {
|
||||
"node_modules/fixture/lib/package.json": '{"main":"actual.cjs"}',
|
||||
"node_modules/fixture/lib/actual.cjs": "module.exports = 'nested';",
|
||||
"node_modules/fixture/lib/index.js": "module.exports = 'nested index';",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "legacy main directory falls back to root index",
|
||||
metadata: { main: "./lib" },
|
||||
index: true,
|
||||
extraFiles: {
|
||||
"node_modules/fixture/lib/package.json": '{"main":"actual.cjs"}',
|
||||
"node_modules/fixture/lib/actual.cjs": "module.exports = 'nested';",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "legacy main cycle falls back to root index",
|
||||
metadata: { main: "./lib" },
|
||||
index: true,
|
||||
extraFiles: { "node_modules/fixture/lib/package.json": '{"main":".."}' },
|
||||
},
|
||||
{
|
||||
name: "null exports preserves legacy main",
|
||||
metadata: { exports: null, main: "actual.cjs" },
|
||||
index: true,
|
||||
},
|
||||
];
|
||||
|
||||
it.each(cases)(
|
||||
"matches Node package resolution for $name",
|
||||
async ({ metadata, index, specifier = "fixture", extraFiles = {} }) => {
|
||||
const root = await createTestTempDirectory("rea-package-precedence-");
|
||||
const files = {
|
||||
...extraFiles,
|
||||
"package.json": JSON.stringify({
|
||||
name: "app",
|
||||
type: "commonjs",
|
||||
main: "main.cjs",
|
||||
}),
|
||||
"main.cjs": `const value = require(${JSON.stringify(specifier)}); module.exports = value;`,
|
||||
"node_modules/fixture/package.json": JSON.stringify({
|
||||
name: "fixture",
|
||||
...metadata,
|
||||
}),
|
||||
"node_modules/fixture/actual.cjs": "module.exports = 'actual entry';",
|
||||
...(index
|
||||
? {
|
||||
"node_modules/fixture/index.js":
|
||||
"module.exports = 'fallback entry';",
|
||||
}
|
||||
: {}),
|
||||
};
|
||||
await Promise.all(
|
||||
Object.entries(files).map(async ([path, text]) => {
|
||||
await mkdir(dirname(join(root, path)), { recursive: true });
|
||||
await writeFile(join(root, path), text);
|
||||
}),
|
||||
);
|
||||
const expected = relative(
|
||||
root,
|
||||
createRequire(join(root, "main.cjs")).resolve(specifier),
|
||||
)
|
||||
.split(sep)
|
||||
.join("/");
|
||||
const result = await reconstructJavaScriptArtifact({
|
||||
input_path: root,
|
||||
format: "directory",
|
||||
});
|
||||
const edge = result.graph.edges.find(
|
||||
({ properties }) =>
|
||||
properties.module_link_kind === "require" &&
|
||||
properties.specifier === specifier,
|
||||
);
|
||||
|
||||
expect(edge?.properties).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: expected,
|
||||
});
|
||||
expect(result.graph.coverage.status).toBe("complete");
|
||||
},
|
||||
);
|
||||
Loading…
Reference in New Issue