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

Closes: #2636 

## Description

The compartment mapper is currently strict about the interpretation of
`peerDependencies` and `peerDependenciesMeta.optional` as opposed to
`optionalDependencies` and is technically correct, but that correctness
and understanding is not evenly distributed among tools in the
ecosystem.
If the Compartment Mapper instead tolerates the physical absence of
expected packages, it defers an error that would be experienced under
`mapNodeModules` to `link/load` since that might attempt to load a
module from the missing package. This is not a particularly observable
difference in behavior, but the error message will be slightly less
informative.

In this change, we relax the behavior by default and provide a `strict`
option to provide the more informative error and tolerate fewer
ecosystem defficiencies.

### Security Considerations

None.

### Scaling Considerations

None.

### Documentation Considerations

News updated.

We do not yet publish API documentation for Compartment Mapper, but the
necessary annotations are present when these devices are more public.

### Testing Considerations

Includes positive and negative tests for the `strict` behavior.

### Compatibility Considerations

The nature of some errors during bundling may change. Bundling errors
are not observed on Agoric’s chain, so that sensitivity should not cause
problems.

### Upgrade Considerations

None.

packages/compartment-mapper/NEWS.md

@@ -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`,

packages/compartment-mapper/src/archive.js

@@ -85,6 +85,7 @@ export const makeArchive = async (powers, moduleLocation, options = {}) => {
     dev,
     tags,
     conditions = tags,
+    strict = false,
     commonDependencies,
     policy,
     languageForExtension,
@@ -99,6 +100,7 @@ export const makeArchive = async (powers, moduleLocation, options = {}) => {
   } = assignParserForLanguage(options);
   const compartmentMap = await mapNodeModules(powers, moduleLocation, {
     dev,
+    strict,
     conditions,
     commonDependencies,
     policy,
@@ -129,6 +131,7 @@ export const mapLocation = async (powers, moduleLocation, options = {}) => {
     dev,
     tags,
     conditions = tags,
+    strict = false,
     commonDependencies,
     policy,
     parserForLanguage,
@@ -144,6 +147,7 @@ export const mapLocation = async (powers, moduleLocation, options = {}) => {
 
   const compartmentMap = await mapNodeModules(powers, moduleLocation, {
     dev,
+    strict,
     conditions,
     commonDependencies,
     policy,
@@ -174,6 +178,7 @@ export const hashLocation = async (powers, moduleLocation, options = {}) => {
     dev,
     tags,
     conditions = tags,
+    strict = false,
     commonDependencies,
     policy,
     parserForLanguage,
@@ -189,6 +194,7 @@ export const hashLocation = async (powers, moduleLocation, options = {}) => {
 
   const compartmentMap = await mapNodeModules(powers, moduleLocation, {
     dev,
+    strict,
     conditions,
     commonDependencies,
     policy,
@@ -226,6 +232,7 @@ export const writeArchive = async (
     dev,
     tags,
     conditions = tags,
+    strict = false,
     commonDependencies,
     policy,
     parserForLanguage,
@@ -240,6 +247,7 @@ export const writeArchive = async (
   } = assignParserForLanguage(options);
   const compartmentMap = await mapNodeModules(readPowers, moduleLocation, {
     dev,
+    strict,
     conditions,
     commonDependencies,
     policy,

packages/compartment-mapper/src/import.js

@@ -76,6 +76,7 @@ export const loadLocation = async (
   const {
     dev,
     tags,
+    strict,
     commonDependencies,
     policy,
     parserForLanguage,
@@ -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,

packages/compartment-mapper/src/node-modules.js

@@ -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>}
@@ -310,6 +311,7 @@ const graphPackage = async (
   dev,
   commonDependencyDescriptors,
   languageOptions,
+  strict,
   preferredPackageLogicalPathMap = new Map(),
   logicalPath = [],
 ) => {
@@ -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) {
+      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);
     }
   }
@@ -380,6 +386,7 @@ const graphPackage = async (
         conditions,
         preferredPackageLogicalPathMap,
         languageOptions,
+        strict,
         childLogicalPath,
         optional,
         commonDependencyDescriptors,
@@ -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
@@ -495,6 +503,7 @@ const gatherDependency = async (
   conditions,
   preferredPackageLogicalPathMap,
   languageOptions,
+  strict,
   childLogicalPath = [],
   optional = false,
   commonDependencyDescriptors = undefined,
@@ -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}`);
@@ -532,6 +541,7 @@ const gatherDependency = async (
     false,
     commonDependencyDescriptors,
     languageOptions,
+    strict,
     preferredPackageLogicalPathMap,
     childLogicalPath,
   );
@@ -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,
@@ -566,6 +577,7 @@ const graphPackages = async (
   dev,
   commonDependencies,
   languageOptions,
+  strict,
 ) => {
   const memo = create(null);
   /**
@@ -623,6 +635,7 @@ const graphPackages = async (
     dev,
     commonDependencyDescriptors,
     languageOptions,
+    strict,
   );
   return graph;
 };
@@ -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);
 
@@ -883,6 +901,7 @@ export const compartmentMapForNodeModules = async (
     dev || (conditions && conditions.has('development')),
     commonDependencies,
     languageOptions,
+    strict,
   );
 
   if (policy) {

packages/compartment-mapper/src/types/external.ts

@@ -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` */

packages/compartment-mapper/test/scaffold.js

@@ -95,6 +95,7 @@ export function scaffold(
     knownFailure = false,
     tags = undefined,
     conditions = tags,
+    strict = false,
     searchSuffixes = undefined,
     commonDependencies = undefined,
     parserForLanguage = undefined,
@@ -154,6 +155,7 @@ export function scaffold(
       workspaceLanguageForExtension,
       workspaceCommonjsLanguageForExtension,
       workspaceModuleLanguageForExtension,
+      strict,
       ...additionalOptions,
     });
     const { namespace } = await application.import({
@@ -182,6 +184,7 @@ export function scaffold(
         modules,
         Compartment,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         commonDependencies,
         languageForExtension,
         commonjsLanguageForExtension,
@@ -226,6 +229,7 @@ export function scaffold(
         modules,
         Compartment,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         commonDependencies,
         languageForExtension,
         commonjsLanguageForExtension,
@@ -268,6 +272,7 @@ export function scaffold(
       modules,
       Compartment,
       conditions: new Set(['development', ...(conditions || [])]),
+      strict,
       searchSuffixes,
       commonDependencies,
       parserForLanguage,
@@ -292,6 +297,7 @@ export function scaffold(
         modules,
         policy,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -337,6 +343,7 @@ export function scaffold(
         modules,
         policy,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -390,6 +397,7 @@ export function scaffold(
         modules: { builtin: true },
         policy,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -451,6 +459,7 @@ export function scaffold(
         policy,
         modules,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         sourceMapHook,
@@ -500,6 +509,7 @@ export function scaffold(
       const map = await mapNodeModules(readPowers, fixture, {
         policy,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         commonDependencies,
         languages,
         languageForExtension,
@@ -553,6 +563,7 @@ export function scaffold(
         modules,
         Compartment,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -568,6 +579,7 @@ export function scaffold(
       const archiveBytes = await makeArchive(readPowers, fixture, {
         modules,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -605,6 +617,7 @@ export function scaffold(
         modules,
         Compartment,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,
@@ -620,6 +633,7 @@ export function scaffold(
       const archive = await makeArchive(readPowers, fixture, {
         modules,
         conditions: new Set(['development', ...(conditions || [])]),
+        strict,
         searchSuffixes,
         commonDependencies,
         parserForLanguage,