fix(javascript): honor package exports condition order (#591)
* fix(javascript): honor package exports condition order
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
* fix(javascript): resolve exports against Node's full condition set
The declared-order fix selected targets against {default, import|require},
but Node's active set for the resolver that owns installed packages also
includes "node" and "node-addons". Ordering only means something against the
full active set, so a package declaring {"node", "import"} resolved to the
wrong target, and a package declaring only {"import"} reported no target
where Node resolves ./node.cjs.
Nested condition objects and fallback arrays are also valid Node targets.
They are now flattened in Node's own order: a condition object stops at its
first active key, so a null or unmatched nested target ends the search, while
an array skips the entries it cannot resolve and concatenates the rest. Node
does not fall through a target whose file is absent, so the first flattened
target stands and its absence is reported rather than skipped.
A valid exports map that matches no active condition is reported as external
with the declared condition names, and one that selects an unsupported nested
or array target no longer borrows the "not valid package JSON" reason.
The differential test installs a real package and resolves the same bare
specifier through Node and through REA, mirroring file presence between the
filesystem and the inventory. 18 of the 25 cases fail without this change.
* style: apply Prettier to the conditional exports differential test
---------
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
Rudy Mizrahi Celekli
GitHub
parent
ffbde92f9a
commit
82895959d1
|
|
@ -319,6 +319,14 @@ const resolveCandidate = (
|
|||
`Directory package metadata ${packagePath} is not valid package JSON.`,
|
||||
],
|
||||
};
|
||||
if (main.status === "unmatched")
|
||||
return {
|
||||
resolvedPath: null,
|
||||
status: "external",
|
||||
limitations: [
|
||||
`Directory package metadata ${packagePath} declares no exports target for the active conditions; it declares ${main.declared.join(", ")}.`,
|
||||
],
|
||||
};
|
||||
if (packageChain.has(packagePath)) {
|
||||
return {
|
||||
resolvedPath: null,
|
||||
|
|
@ -401,7 +409,8 @@ const packageEntry = (
|
|||
readonly source: "legacy" | "exports";
|
||||
}
|
||||
| { readonly status: "missing" }
|
||||
| { readonly status: "invalid" } => {
|
||||
| { readonly status: "invalid" }
|
||||
| { readonly status: "unmatched"; readonly declared: readonly string[] } => {
|
||||
try {
|
||||
const value: unknown = JSON.parse(text);
|
||||
if (typeof value !== "object" || value === null)
|
||||
|
|
@ -429,9 +438,7 @@ const packageEntry = (
|
|||
const packageExport = (
|
||||
value: unknown,
|
||||
moduleKind: ResolveArtifactPathInput["moduleKind"],
|
||||
):
|
||||
| { readonly status: "value"; readonly value: string }
|
||||
| { readonly status: "invalid" } => {
|
||||
): PackageExportOutcome => {
|
||||
if (typeof value === "string") return packagePathValue(value);
|
||||
if (typeof value !== "object" || value === null || Array.isArray(value))
|
||||
return { status: "invalid" };
|
||||
|
|
@ -439,11 +446,71 @@ const packageExport = (
|
|||
if (typeof root === "string") return packagePathValue(root);
|
||||
if (typeof root !== "object" || root === null || Array.isArray(root))
|
||||
return { status: "invalid" };
|
||||
const condition =
|
||||
Reflect.get(root, moduleKind ?? "default") ?? Reflect.get(root, "default");
|
||||
return packagePathValue(condition);
|
||||
const flattened = exportTargets(root, packageExportConditions(moduleKind));
|
||||
if (flattened.kind === "invalid") return { status: "invalid" };
|
||||
const first = flattened.values[0];
|
||||
return first === undefined
|
||||
? { status: "unmatched", declared: Object.keys(root) }
|
||||
: { status: "value", value: first };
|
||||
};
|
||||
|
||||
type ExportTargets =
|
||||
| { readonly kind: "targets"; readonly values: readonly string[] }
|
||||
| { readonly kind: "invalid" };
|
||||
|
||||
/**
|
||||
* Flatten one exports target into the ordered strings Node would consider.
|
||||
* A condition object stops at its first active key, so a null or unmatched
|
||||
* nested target ends the search rather than falling through to a later key. An
|
||||
* array instead skips the entries it cannot resolve and concatenates the rest.
|
||||
*/
|
||||
const exportTargets = (
|
||||
value: unknown,
|
||||
conditions: ReadonlySet<string>,
|
||||
): ExportTargets => {
|
||||
if (typeof value === "string")
|
||||
return value.length > 0
|
||||
? { kind: "targets", values: [value] }
|
||||
: { kind: "invalid" };
|
||||
if (value === null) return { kind: "targets", values: [] };
|
||||
if (Array.isArray(value)) {
|
||||
const values: string[] = [];
|
||||
for (const entry of value) {
|
||||
const nested = exportTargets(entry, conditions);
|
||||
if (nested.kind === "invalid") return nested;
|
||||
values.push(...nested.values);
|
||||
}
|
||||
return { kind: "targets", values };
|
||||
}
|
||||
if (typeof value !== "object") return { kind: "invalid" };
|
||||
for (const [condition, target] of Object.entries(value)) {
|
||||
if (!conditions.has(condition)) continue;
|
||||
return exportTargets(target, conditions);
|
||||
}
|
||||
return { kind: "targets", values: [] };
|
||||
};
|
||||
|
||||
/**
|
||||
* Node selects an exports target by walking the declared keys in order and
|
||||
* taking the first whose condition is active for the calling resolver.
|
||||
* "default" is always active, and "node" plus "node-addons" are active for the
|
||||
* built-in resolver that owns installed-package imports and requires.
|
||||
*/
|
||||
const packageExportConditions = (
|
||||
moduleKind: ResolveArtifactPathInput["moduleKind"],
|
||||
): ReadonlySet<string> =>
|
||||
new Set([
|
||||
"node",
|
||||
"node-addons",
|
||||
...(moduleKind === undefined ? [] : [moduleKind]),
|
||||
"default",
|
||||
]);
|
||||
|
||||
type PackageExportOutcome =
|
||||
| { readonly status: "value"; readonly value: string }
|
||||
| { readonly status: "invalid" }
|
||||
| { readonly status: "unmatched"; readonly declared: readonly string[] };
|
||||
|
||||
const packagePathValue = (
|
||||
value: unknown,
|
||||
):
|
||||
|
|
|
|||
|
|
@ -0,0 +1,228 @@
|
|||
import { execFile } from "node:child_process";
|
||||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
resolveArtifactPathByContext,
|
||||
type ArtifactPathResolution,
|
||||
} from "../../../src/application/JavaScriptArtifactPathResolution.js";
|
||||
import type { JavaScriptArtifactFile } from "../../../src/application/JavaScriptArtifactFiles.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
const execute = promisify(execFile);
|
||||
const TARGETS = [
|
||||
"default.cjs",
|
||||
"specific.cjs",
|
||||
"node.cjs",
|
||||
"require.cjs",
|
||||
"array.cjs",
|
||||
"nested.cjs",
|
||||
];
|
||||
const SCRIPT =
|
||||
'import { createRequire } from "node:module"; console.log((await import("example")).default ?? createRequire(import.meta.url)("example"))';
|
||||
/** Resolve the bare `example` specifier through the Node resolver that owns it. */
|
||||
const nodeTarget = async (
|
||||
cwd: string,
|
||||
moduleKind: "import" | "require",
|
||||
): Promise<string | null> => {
|
||||
try {
|
||||
const { stdout } = await execute(
|
||||
process.execPath,
|
||||
[
|
||||
"--input-type=module",
|
||||
"-e",
|
||||
moduleKind === "import"
|
||||
? 'console.log((await import("example")).default)'
|
||||
: 'import { createRequire } from "node:module"; console.log(createRequire(import.meta.url)("example"))',
|
||||
],
|
||||
{ cwd },
|
||||
);
|
||||
return stdout.trim();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
};
|
||||
/**
|
||||
* Install one real package on disk and mirror it into an artifact inventory,
|
||||
* so Node and REA observe the same container, then resolve through both.
|
||||
*/
|
||||
const compareWithNode = async (
|
||||
exportsMap: unknown,
|
||||
moduleKind: "import" | "require",
|
||||
present: readonly string[] = TARGETS,
|
||||
): Promise<{
|
||||
readonly node: string | null;
|
||||
readonly outcome: ArtifactPathResolution;
|
||||
}> => {
|
||||
const root = await createTestTempDirectory("rea-exports-");
|
||||
const packageDirectory = join(root, "node_modules", "example");
|
||||
await mkdir(packageDirectory, { recursive: true });
|
||||
const metadata = JSON.stringify({ name: "example", exports: exportsMap });
|
||||
await writeFile(join(packageDirectory, "package.json"), metadata);
|
||||
const contents = new Map<string, string>([
|
||||
["main.js", ""],
|
||||
["node_modules/example/package.json", metadata],
|
||||
]);
|
||||
for (const target of present) {
|
||||
const source = `module.exports = ${JSON.stringify(target)};`;
|
||||
await writeFile(join(packageDirectory, target), source);
|
||||
contents.set(`node_modules/example/${target}`, source);
|
||||
}
|
||||
const files = new Map<string, JavaScriptArtifactFile>(
|
||||
[...contents].map(([path, value]) => [
|
||||
path,
|
||||
{
|
||||
path,
|
||||
container_sha256: "a".repeat(64),
|
||||
sha256: "b".repeat(64),
|
||||
bytes: Buffer.byteLength(value),
|
||||
inventory_artifact_id: path,
|
||||
kind: path.endsWith("package.json") ? "package-json" : "javascript",
|
||||
unpacked: false,
|
||||
text: { included: true, value },
|
||||
},
|
||||
]),
|
||||
);
|
||||
return {
|
||||
node: await nodeTarget(root, moduleKind),
|
||||
outcome: resolveArtifactPathByContext({
|
||||
declaredPath: "example",
|
||||
sourcePath: "main.js",
|
||||
context: "module-specifier",
|
||||
moduleKind,
|
||||
files,
|
||||
}),
|
||||
};
|
||||
};
|
||||
const resolvedLeaf = (outcome: ArtifactPathResolution): string | null =>
|
||||
outcome.resolved_path?.split("/").pop() ?? null;
|
||||
/** Pin the real Node result, then require REA to select the same target. */
|
||||
const expectNodeAgreement = async (
|
||||
exportsMap: unknown,
|
||||
expected: string | null,
|
||||
moduleKind: "import" | "require" = "import",
|
||||
present: readonly string[] = TARGETS,
|
||||
): Promise<ArtifactPathResolution> => {
|
||||
const { node, outcome } = await compareWithNode(
|
||||
exportsMap,
|
||||
moduleKind,
|
||||
present,
|
||||
);
|
||||
expect(node).toBe(expected);
|
||||
expect(resolvedLeaf(outcome)).toBe(expected);
|
||||
return outcome;
|
||||
};
|
||||
describe("ordered package exports conditions", () => {
|
||||
it.each(["import", "require"] as const)(
|
||||
"matches Node when default precedes %s",
|
||||
async (moduleKind) => {
|
||||
await expectNodeAgreement(
|
||||
{ default: "./default.cjs", [moduleKind]: "./specific.cjs" },
|
||||
"default.cjs",
|
||||
moduleKind,
|
||||
);
|
||||
},
|
||||
);
|
||||
it.each(["import", "require"] as const)(
|
||||
"matches Node when %s precedes default",
|
||||
async (moduleKind) => {
|
||||
await expectNodeAgreement(
|
||||
{ [moduleKind]: "./specific.cjs", default: "./default.cjs" },
|
||||
"specific.cjs",
|
||||
moduleKind,
|
||||
);
|
||||
},
|
||||
);
|
||||
it.each(["import", "require"] as const)(
|
||||
"matches Node when the node condition precedes %s",
|
||||
async (moduleKind) => {
|
||||
await expectNodeAgreement(
|
||||
{ node: "./node.cjs", [moduleKind]: "./specific.cjs" },
|
||||
"node.cjs",
|
||||
moduleKind,
|
||||
);
|
||||
},
|
||||
);
|
||||
it.each(["import", "require"] as const)(
|
||||
"matches Node when %s precedes the node condition",
|
||||
async (moduleKind) => {
|
||||
await expectNodeAgreement(
|
||||
{ [moduleKind]: "./specific.cjs", node: "./node.cjs" },
|
||||
"specific.cjs",
|
||||
moduleKind,
|
||||
);
|
||||
},
|
||||
);
|
||||
it.each(["import", "require"] as const)(
|
||||
"matches Node when node-addons precedes %s",
|
||||
async (moduleKind) => {
|
||||
await expectNodeAgreement(
|
||||
{ "node-addons": "./node.cjs", [moduleKind]: "./specific.cjs" },
|
||||
"node.cjs",
|
||||
moduleKind,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
describe("nested and array package export targets", () => {
|
||||
it.each([
|
||||
[
|
||||
{ import: { node: "./nested.cjs", default: "./default.cjs" } },
|
||||
"nested.cjs",
|
||||
],
|
||||
[{ import: { default: "./default.cjs" } }, "default.cjs"],
|
||||
[{ import: { node: { default: "./default.cjs" } } }, "default.cjs"],
|
||||
[{ import: [{ node: "./nested.cjs" }, "./array.cjs"] }, "nested.cjs"],
|
||||
[{ import: [{ browser: "./x.cjs" }, "./array.cjs"] }, "array.cjs"],
|
||||
[{ import: [null, "./array.cjs"] }, "array.cjs"],
|
||||
[{ import: ["./specific.cjs", "./array.cjs"] }, "specific.cjs"],
|
||||
] as const)(
|
||||
"selects the same target as Node for %j",
|
||||
async (exportsMap, expected) => {
|
||||
await expectNodeAgreement(exportsMap, expected);
|
||||
},
|
||||
);
|
||||
it.each([
|
||||
[{ import: { browser: "./x.cjs" } }, "external"],
|
||||
[{ import: { node: null, default: "./default.cjs" } }, "external"],
|
||||
[{ import: [] }, "external"],
|
||||
] as const)(
|
||||
"refuses %j with %s exactly as Node refuses it",
|
||||
async (exportsMap, status) => {
|
||||
const outcome = await expectNodeAgreement(exportsMap, null);
|
||||
expect(outcome.resolution_status).toBe(status);
|
||||
},
|
||||
);
|
||||
it.each([
|
||||
[{ import: ["./absent.cjs", "./array.cjs"] }],
|
||||
[{ import: [["./absent.cjs"], "./array.cjs"] }],
|
||||
] as const)(
|
||||
"reports %j as absent rather than falling through to a later entry",
|
||||
async (exportsMap) => {
|
||||
const outcome = await expectNodeAgreement(exportsMap, null);
|
||||
expect(outcome.resolution_status).toBe("not-found");
|
||||
},
|
||||
);
|
||||
});
|
||||
describe("unmatched package export conditions", () => {
|
||||
it.each(["import", "require"] as const)(
|
||||
"names the declared conditions for %s",
|
||||
async (moduleKind) => {
|
||||
const { node, outcome } = await compareWithNode(
|
||||
{ browser: "./default.cjs" },
|
||||
moduleKind,
|
||||
);
|
||||
expect(node).toBeNull();
|
||||
expect(outcome).toMatchObject({
|
||||
resolution_status: "external",
|
||||
resolved_path: null,
|
||||
});
|
||||
expect(outcome.limitations[0]).toContain("declares browser");
|
||||
},
|
||||
);
|
||||
it("does not report valid metadata as malformed", async () => {
|
||||
const { outcome } = await compareWithNode({ default: 7 }, "import");
|
||||
expect(outcome.resolution_status).toBe("unavailable");
|
||||
expect(outcome.limitations[0]).toContain("not valid package JSON");
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue