Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(compartment-mapper): Relax package discovery #2642

Merged
merged 1 commit into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/compartment-mapper/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
User-visible changes to `@endo/compartment-mapper`:

# Next release

- `mapNodeModules` and all functions that use it now tolerate the absence of
expected packages.
These packages are now omitted from the generated package skeleton map.
So, loading a physically missing module now occurs during the load phase
instead of the mapping phase.
- Adds a `strict` option to all functions that `mapNodeModules` to restore old
behavior, which produces an error early if, for example, a non-optional
peer dependency is missing.
Peer dependencies are strictly required unless `peerDependenciesMeta` has an
object with a truthy `optional` entry.
Correct interpretation of `peerDependencies` is not distributed evenly, so
this behavior is no longer the default.

# v1.4.0 (2024-11-13)

- Adds options `languageForExtension`, `moduleLanguageForExtension`,
Expand Down
8 changes: 8 additions & 0 deletions packages/compartment-mapper/src/archive.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export const makeArchive = async (powers, moduleLocation, options = {}) => {
dev,
tags,
conditions = tags,
strict = false,
commonDependencies,
policy,
languageForExtension,
Expand All @@ -99,6 +100,7 @@ export const makeArchive = async (powers, moduleLocation, options = {}) => {
} = assignParserForLanguage(options);
const compartmentMap = await mapNodeModules(powers, moduleLocation, {
dev,
strict,
conditions,
commonDependencies,
policy,
Expand Down Expand Up @@ -129,6 +131,7 @@ export const mapLocation = async (powers, moduleLocation, options = {}) => {
dev,
tags,
conditions = tags,
strict = false,
commonDependencies,
policy,
parserForLanguage,
Expand All @@ -144,6 +147,7 @@ export const mapLocation = async (powers, moduleLocation, options = {}) => {

const compartmentMap = await mapNodeModules(powers, moduleLocation, {
dev,
strict,
conditions,
commonDependencies,
policy,
Expand Down Expand Up @@ -174,6 +178,7 @@ export const hashLocation = async (powers, moduleLocation, options = {}) => {
dev,
tags,
conditions = tags,
strict = false,
commonDependencies,
policy,
parserForLanguage,
Expand All @@ -189,6 +194,7 @@ export const hashLocation = async (powers, moduleLocation, options = {}) => {

const compartmentMap = await mapNodeModules(powers, moduleLocation, {
dev,
strict,
conditions,
commonDependencies,
policy,
Expand Down Expand Up @@ -226,6 +232,7 @@ export const writeArchive = async (
dev,
tags,
conditions = tags,
strict = false,
commonDependencies,
policy,
parserForLanguage,
Expand All @@ -240,6 +247,7 @@ export const writeArchive = async (
} = assignParserForLanguage(options);
const compartmentMap = await mapNodeModules(readPowers, moduleLocation, {
dev,
strict,
conditions,
commonDependencies,
policy,
Expand Down
2 changes: 2 additions & 0 deletions packages/compartment-mapper/src/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const loadLocation = async (
const {
dev,
tags,
strict,
commonDependencies,
policy,
parserForLanguage,
Expand All @@ -93,6 +94,7 @@ export const loadLocation = async (
'conditions' in options ? options.conditions || tags : tags;
const compartmentMap = await mapNodeModules(readPowers, moduleLocation, {
dev,
strict,
conditions,
commonDependencies,
policy,
Expand Down
33 changes: 26 additions & 7 deletions packages/compartment-mapper/src/node-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ const inferParsers = (descriptor, location, languageOptions) => {
* @param {boolean | undefined} dev
* @param {CommonDependencyDescriptors} commonDependencyDescriptors
* @param {LanguageOptions} languageOptions
* @param {boolean} strict
* @param {Map<string, Array<string>>} preferredPackageLogicalPathMap
* @param {Array<string>} logicalPath
* @returns {Promise<undefined>}
Expand All @@ -310,6 +311,7 @@ const graphPackage = async (
dev,
commonDependencyDescriptors,
languageOptions,
strict,
preferredPackageLogicalPathMap = new Map(),
logicalPath = [],
) => {
Expand Down Expand Up @@ -344,14 +346,18 @@ const graphPackage = async (
devDependencies = {},
} = packageDescriptor;
const allDependencies = {};
assign(allDependencies, commonDependencyDescriptors);
for (const [name, { spec }] of Object.entries(commonDependencyDescriptors)) {
allDependencies[name] = spec;
for (const [name, descriptor] of Object.entries(
commonDependencyDescriptors,
)) {
if (Object(descriptor) === descriptor) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, Object(x) === x is new to me. what does it do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the terse way of checking typeof x === 'object' && x !== null. Object(x) returns x iff x is an object and not null, otherwise boxes primitives.

const { spec } = descriptor;
allDependencies[name] = spec;
}
}
assign(allDependencies, dependencies);
assign(allDependencies, peerDependencies);
for (const [name, { optional }] of Object.entries(peerDependenciesMeta)) {
if (optional) {
for (const [name, meta] of Object.entries(peerDependenciesMeta)) {
if (Object(meta) === meta && meta.optional) {
optionals.add(name);
}
}
Expand Down Expand Up @@ -380,6 +386,7 @@ const graphPackage = async (
conditions,
preferredPackageLogicalPathMap,
languageOptions,
strict,
childLogicalPath,
optional,
commonDependencyDescriptors,
Expand Down Expand Up @@ -481,6 +488,7 @@ const graphPackage = async (
* @param {Set<string>} conditions
* @param {Map<string, Array<string>>} preferredPackageLogicalPathMap
* @param {LanguageOptions} languageOptions
* @param {boolean} strict
* @param {Array<string>} [childLogicalPath]
* @param {boolean} [optional] - whether the dependency is optional
* @param {object} [commonDependencyDescriptors] - dependencies to be added to all packages
Expand All @@ -495,6 +503,7 @@ const gatherDependency = async (
conditions,
preferredPackageLogicalPathMap,
languageOptions,
strict,
childLogicalPath = [],
optional = false,
commonDependencyDescriptors = undefined,
Expand All @@ -507,7 +516,7 @@ const gatherDependency = async (
);
if (dependency === undefined) {
// allow the dependency to be missing if optional
if (optional) {
if (optional || !strict) {
return;
}
throw Error(`Cannot find dependency ${name} for ${packageLocation}`);
Expand All @@ -532,6 +541,7 @@ const gatherDependency = async (
false,
commonDependencyDescriptors,
languageOptions,
strict,
preferredPackageLogicalPathMap,
childLogicalPath,
);
Expand All @@ -556,6 +566,7 @@ const gatherDependency = async (
* only this package).
* @param {Record<string,string>} commonDependencies - dependencies to be added to all packages
* @param {LanguageOptions} languageOptions
* @param {boolean} strict
*/
const graphPackages = async (
maybeRead,
Expand All @@ -566,6 +577,7 @@ const graphPackages = async (
dev,
commonDependencies,
languageOptions,
strict,
) => {
const memo = create(null);
/**
Expand Down Expand Up @@ -623,6 +635,7 @@ const graphPackages = async (
dev,
commonDependencyDescriptors,
languageOptions,
strict,
);
return graph;
};
Expand Down Expand Up @@ -863,7 +876,12 @@ export const compartmentMapForNodeModules = async (
moduleSpecifier,
options = {},
) => {
const { dev = false, commonDependencies = {}, policy } = options;
const {
dev = false,
commonDependencies = {},
policy,
strict = false,
} = options;
const { maybeRead, canonical } = unpackReadPowers(readPowers);
const languageOptions = makeLanguageOptions(options);

Expand All @@ -883,6 +901,7 @@ export const compartmentMapForNodeModules = async (
dev || (conditions && conditions.has('development')),
commonDependencies,
languageOptions,
strict,
);

if (policy) {
Expand Down
7 changes: 7 additions & 0 deletions packages/compartment-mapper/src/types/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ type MapNodeModulesOptionsOmitPolicy = Partial<{
* of having the `"development"` condition.
*/
dev: boolean;
/**
* Indicates that the node_modules tree should fail to map if it does not
* strictly reach every expected package.
* By default, unreachable packages are simply omitted from the map,
* which defers some errors to when modules load.
*/
strict: boolean;
/** Dependencies to make reachable from any package */
commonDependencies: Record<string, string>;
/** Maps extensions to languages for all packages, like `txt` to `text` */
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions packages/compartment-mapper/test/scaffold.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export function scaffold(
knownFailure = false,
tags = undefined,
conditions = tags,
strict = false,
searchSuffixes = undefined,
commonDependencies = undefined,
parserForLanguage = undefined,
Expand Down Expand Up @@ -154,6 +155,7 @@ export function scaffold(
workspaceLanguageForExtension,
workspaceCommonjsLanguageForExtension,
workspaceModuleLanguageForExtension,
strict,
...additionalOptions,
});
const { namespace } = await application.import({
Expand Down Expand Up @@ -182,6 +184,7 @@ export function scaffold(
modules,
Compartment,
conditions: new Set(['development', ...(conditions || [])]),
strict,
commonDependencies,
languageForExtension,
commonjsLanguageForExtension,
Expand Down Expand Up @@ -226,6 +229,7 @@ export function scaffold(
modules,
Compartment,
conditions: new Set(['development', ...(conditions || [])]),
strict,
commonDependencies,
languageForExtension,
commonjsLanguageForExtension,
Expand Down Expand Up @@ -268,6 +272,7 @@ export function scaffold(
modules,
Compartment,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand All @@ -292,6 +297,7 @@ export function scaffold(
modules,
policy,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand Down Expand Up @@ -337,6 +343,7 @@ export function scaffold(
modules,
policy,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand Down Expand Up @@ -390,6 +397,7 @@ export function scaffold(
modules: { builtin: true },
policy,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand Down Expand Up @@ -451,6 +459,7 @@ export function scaffold(
policy,
modules,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
sourceMapHook,
Expand Down Expand Up @@ -500,6 +509,7 @@ export function scaffold(
const map = await mapNodeModules(readPowers, fixture, {
policy,
conditions: new Set(['development', ...(conditions || [])]),
strict,
commonDependencies,
languages,
languageForExtension,
Expand Down Expand Up @@ -553,6 +563,7 @@ export function scaffold(
modules,
Compartment,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand All @@ -568,6 +579,7 @@ export function scaffold(
const archiveBytes = await makeArchive(readPowers, fixture, {
modules,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand Down Expand Up @@ -605,6 +617,7 @@ export function scaffold(
modules,
Compartment,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand All @@ -620,6 +633,7 @@ export function scaffold(
const archive = await makeArchive(readPowers, fixture, {
modules,
conditions: new Set(['development', ...(conditions || [])]),
strict,
searchSuffixes,
commonDependencies,
parserForLanguage,
Expand Down
Loading
Loading