refactor(ses): Retool for module descriptors (#2321)

Refs: #400 #2251 ## Description Pursuant to arriving at parity with XS, this change internally reörients the SES module loader around the concept of a “module descriptor”. This creates some clarity and removes some existing duplication of work for “alias” module descriptors, and prepares the material for a richer module descriptor schema to match those implemented by XS. ### Security Considerations No changes. ### Scaling Considerations No changes. ### Documentation Considerations No changes. ### Testing Considerations No changes. ### Compatibility Considerations No changes. ### Upgrade Considerations No changes.
endojs · Jul 15, 2024 · a62e766 · a62e766
2 parents fc569ce + 5df0f1a
commit a62e766
Show file tree

Hide file tree

Showing 7 changed files with 447 additions and 234 deletions.
diff --git a/packages/ses/src/compartment.js b/packages/ses/src/compartment.js
@@ -4,12 +4,10 @@
 
 import {
   Map,
-  ReferenceError,
   TypeError,
   WeakMap,
   assign,
   defineProperties,
-  entries,
   promiseThen,
   toStringTagSymbol,
   weakmapGet,
@@ -25,12 +23,9 @@ import { sharedGlobalPropertyNames } from './permits.js';
 import { load, loadNow } from './module-load.js';
 import { link } from './module-link.js';
 import { getDeferredExports } from './module-proxy.js';
-import { assert } from './error/assert.js';
 import { compartmentEvaluate } from './compartment-evaluate.js';
 import { makeSafeEvaluator } from './make-safe-evaluator.js';
 
-const { quote: q } = assert;
-
 // moduleAliases associates every public module exports namespace with its
 // corresponding compartment and specifier so they can be used to link modules
 // across compartments.
@@ -42,19 +37,6 @@ const moduleAliases = new WeakMap();
 // privateFields captures the private state for each compartment.
 const privateFields = new WeakMap();
 
-// Compartments do not need an importHook or resolveHook to be useful
-// as a vessel for evaluating programs.
-// However, any method that operates the module system will throw an exception
-// if these hooks are not available.
-const assertModuleHooks = compartment => {
-  const { importHook, resolveHook } = weakmapGet(privateFields, compartment);
-  if (typeof importHook !== 'function' || typeof resolveHook !== 'function') {
-    throw TypeError(
-      'Compartment must be constructed with an importHook and a resolveHook for it to be able to load modules',
-    );
-  }
-};
-
 export const InertCompartment = function Compartment(
   _endowments = {},
   _modules = {},
@@ -111,8 +93,6 @@ export const CompartmentPrototype = {
       throw TypeError('first argument of module() must be a string');
     }
 
-    assertModuleHooks(this);
-
     const { exportsProxy } = getDeferredExports(
       this,
       weakmapGet(privateFields, this),
@@ -128,8 +108,6 @@ export const CompartmentPrototype = {
       throw TypeError('first argument of import() must be a string');
     }
 
-    assertModuleHooks(this);
-
     return promiseThen(
       load(privateFields, moduleAliases, this, specifier),
       () => {
@@ -149,8 +127,6 @@ export const CompartmentPrototype = {
       throw TypeError('first argument of load() must be a string');
     }
 
-    assertModuleHooks(this);
-
     return load(privateFields, moduleAliases, this, specifier);
   },
 
@@ -159,7 +135,6 @@ export const CompartmentPrototype = {
       throw TypeError('first argument of importNow() must be a string');
     }
 
-    assertModuleHooks(this);
     loadNow(privateFields, moduleAliases, this, specifier);
     return compartmentImportNow(/** @type {Compartment} */ (this), specifier);
   },
@@ -222,30 +197,6 @@ export const makeCompartmentConstructor = (
     // Map<FullSpecifier, {ExportsProxy, ProxiedExports, activate()}>
     const deferredExports = new Map();
 
-    // Validate given moduleMap.
-    // The module map gets translated on-demand in module-load.js and the
-    // moduleMap can be invalid in ways that cannot be detected in the
-    // constructor, but these checks allow us to throw early for a better
-    // developer experience.
-    for (const [specifier, aliasNamespace] of entries(moduleMap || {})) {
-      if (typeof aliasNamespace === 'string') {
-        // TODO implement parent module record retrieval.
-        throw TypeError(
-          `Cannot map module ${q(specifier)} to ${q(
-            aliasNamespace,
-          )} in parent compartment`,
-        );
-      } else if (weakmapGet(moduleAliases, aliasNamespace) === undefined) {
-        // TODO create and link a synthetic module instance from the given
-        // namespace object.
-        throw ReferenceError(
-          `Cannot map module ${q(
-            specifier,
-          )} because it has no known compartment in this realm`,
-        );
-      }
-    }
-
     const globalObject = {};
 
     setGlobalObjectSymbolUnscopables(globalObject);

diff --git a/packages/ses/src/error/assert.js b/packages/ses/src/error/assert.js
@@ -564,3 +564,7 @@ export { makeAssert };
 /** @type {Assert} */
 const assert = makeAssert();
 export { assert };
+
+// Internal, to obviate polymorphic dispatch, but may become rigorously
+// consistent with @endo/error.
+export { makeError, note as annotateError, redactedDetails as X, quote as q };
diff --git a/packages/ses/src/module-instance.js b/packages/ses/src/module-instance.js
@@ -24,9 +24,9 @@ import { compartmentEvaluate } from './compartment-evaluate.js';
 
 const { quote: q } = assert;
 
-export const makeThirdPartyModuleInstance = (
+export const makeVirtualModuleInstance = (
   compartmentPrivateFields,
-  staticModuleRecord,
+  moduleSource,
   compartment,
   moduleAliases,
   moduleSpecifier,
@@ -41,16 +41,16 @@ export const makeThirdPartyModuleInstance = (
 
   const notifiers = create(null);
 
-  if (staticModuleRecord.exports) {
+  if (moduleSource.exports) {
     if (
-      !isArray(staticModuleRecord.exports) ||
-      arraySome(staticModuleRecord.exports, name => typeof name !== 'string')
+      !isArray(moduleSource.exports) ||
+      arraySome(moduleSource.exports, name => typeof name !== 'string')
     ) {
       throw TypeError(
-        `SES third-party static module record "exports" property must be an array of strings for module ${moduleSpecifier}`,
+        `SES virtual module source "exports" property must be an array of strings for module ${moduleSpecifier}`,
       );
     }
-    arrayForEach(staticModuleRecord.exports, name => {
+    arrayForEach(moduleSource.exports, name => {
       let value = exportsTarget[name];
       const updaters = [];
 
@@ -96,11 +96,7 @@ export const makeThirdPartyModuleInstance = (
         localState.activated = true;
         try {
           // eslint-disable-next-line @endo/no-polymorphic-call
-          staticModuleRecord.execute(
-            exportsTarget,
-            compartment,
-            resolvedImports,
-          );
+          moduleSource.execute(exportsTarget, compartment, resolvedImports);
         } catch (err) {
           localState.errorFromExecute = err;
           throw err;
@@ -126,7 +122,7 @@ export const makeModuleInstance = (
   const {
     compartment,
     moduleSpecifier,
-    staticModuleRecord,
+    moduleSource,
     importMeta: moduleRecordMeta,
   } = moduleRecord;
   const {
@@ -137,7 +133,7 @@ export const makeModuleInstance = (
     __reexportMap__: reexportMap = {},
     __needsImportMeta__: needsImportMeta = false,
     __syncModuleFunctor__,
-  } = staticModuleRecord;
+  } = moduleSource;
 
   const compartmentFields = weakmapGet(privateFields, compartment);
 

diff --git a/packages/ses/src/module-link.js b/packages/ses/src/module-link.js
@@ -1,17 +1,9 @@
 /* eslint-disable no-underscore-dangle */
 
-// For brevity, in this file, as in module-load.js, the term "moduleRecord"
-// without qualification means "module compartment record".
-// This is a super-set of the "static module record", that is reusable between
-// compartments with different hooks.
-// The "module compartment record" captures the compartment and overlays the
-// module's "imports" with the more specific "resolvedImports" as inferred from
-// the particular compartment's "resolveHook".
-
 import { assert } from './error/assert.js';
 import {
   makeModuleInstance,
-  makeThirdPartyModuleInstance,
+  makeVirtualModuleInstance,
 } from './module-instance.js';
 import {
   Map,
@@ -61,56 +53,50 @@ export const link = (
   return instantiate(compartmentPrivateFields, moduleAliases, moduleRecord);
 };
 
-function isPrecompiled(staticModuleRecord) {
-  return typeof staticModuleRecord.__syncModuleProgram__ === 'string';
+function mayBePrecompiledModuleSource(moduleSource) {
+  return typeof moduleSource.__syncModuleProgram__ === 'string';
 }
 
-function validatePrecompiledStaticModuleRecord(
-  staticModuleRecord,
-  moduleSpecifier,
-) {
-  const { __fixedExportMap__, __liveExportMap__ } = staticModuleRecord;
+function validatePrecompiledModuleSource(moduleSource, moduleSpecifier) {
+  const { __fixedExportMap__, __liveExportMap__ } = moduleSource;
   isObject(__fixedExportMap__) ||
-    Fail`Property '__fixedExportMap__' of a precompiled module record must be an object, got ${q(
+    Fail`Property '__fixedExportMap__' of a precompiled module source must be an object, got ${q(
       __fixedExportMap__,
     )}, for module ${q(moduleSpecifier)}`;
   isObject(__liveExportMap__) ||
-    Fail`Property '__liveExportMap__' of a precompiled module record must be an object, got ${q(
+    Fail`Property '__liveExportMap__' of a precompiled module source must be an object, got ${q(
       __liveExportMap__,
     )}, for module ${q(moduleSpecifier)}`;
 }
 
-function isThirdParty(staticModuleRecord) {
-  return typeof staticModuleRecord.execute === 'function';
+function mayBeVirtualModuleSource(moduleSource) {
+  return typeof moduleSource.execute === 'function';
 }
 
-function validateThirdPartyStaticModuleRecord(
-  staticModuleRecord,
-  moduleSpecifier,
-) {
-  const { exports } = staticModuleRecord;
+function validateVirtualModuleSource(moduleSource, moduleSpecifier) {
+  const { exports } = moduleSource;
   isArray(exports) ||
-    Fail`Property 'exports' of a third-party static module record must be an array, got ${q(
+    Fail`Property 'exports' of a third-party module source must be an array, got ${q(
       exports,
     )}, for module ${q(moduleSpecifier)}`;
 }
 
-function validateStaticModuleRecord(staticModuleRecord, moduleSpecifier) {
-  isObject(staticModuleRecord) ||
-    Fail`Static module records must be of type object, got ${q(
-      staticModuleRecord,
+function validateModuleSource(moduleSource, moduleSpecifier) {
+  isObject(moduleSource) ||
+    Fail`Module sources must be of type object, got ${q(
+      moduleSource,
     )}, for module ${q(moduleSpecifier)}`;
-  const { imports, exports, reexports = [] } = staticModuleRecord;
+  const { imports, exports, reexports = [] } = moduleSource;
   isArray(imports) ||
-    Fail`Property 'imports' of a static module record must be an array, got ${q(
+    Fail`Property 'imports' of a module source must be an array, got ${q(
       imports,
     )}, for module ${q(moduleSpecifier)}`;
   isArray(exports) ||
-    Fail`Property 'exports' of a precompiled module record must be an array, got ${q(
+    Fail`Property 'exports' of a precompiled module source must be an array, got ${q(
       exports,
     )}, for module ${q(moduleSpecifier)}`;
   isArray(reexports) ||
-    Fail`Property 'reexports' of a precompiled module record must be an array if present, got ${q(
+    Fail`Property 'reexports' of a precompiled module source must be an array if present, got ${q(
       reexports,
     )}, for module ${q(moduleSpecifier)}`;
 }
@@ -120,7 +106,7 @@ export const instantiate = (
   moduleAliases,
   moduleRecord,
 ) => {
-  const { compartment, moduleSpecifier, resolvedImports, staticModuleRecord } =
+  const { compartment, moduleSpecifier, resolvedImports, moduleSource } =
     moduleRecord;
   const { instances } = weakmapGet(compartmentPrivateFields, compartment);
 
@@ -129,33 +115,31 @@ export const instantiate = (
     return mapGet(instances, moduleSpecifier);
   }
 
-  validateStaticModuleRecord(staticModuleRecord, moduleSpecifier);
+  validateModuleSource(moduleSource, moduleSpecifier);
 
   const importedInstances = new Map();
   let moduleInstance;
-  if (isPrecompiled(staticModuleRecord)) {
-    validatePrecompiledStaticModuleRecord(staticModuleRecord, moduleSpecifier);
+  if (mayBePrecompiledModuleSource(moduleSource)) {
+    validatePrecompiledModuleSource(moduleSource, moduleSpecifier);
     moduleInstance = makeModuleInstance(
       compartmentPrivateFields,
       moduleAliases,
       moduleRecord,
       importedInstances,
     );
-  } else if (isThirdParty(staticModuleRecord)) {
-    validateThirdPartyStaticModuleRecord(staticModuleRecord, moduleSpecifier);
-    moduleInstance = makeThirdPartyModuleInstance(
+  } else if (mayBeVirtualModuleSource(moduleSource)) {
+    validateVirtualModuleSource(moduleSource, moduleSpecifier);
+    moduleInstance = makeVirtualModuleInstance(
       compartmentPrivateFields,
-      staticModuleRecord,
+      moduleSource,
       compartment,
       moduleAliases,
       moduleSpecifier,
       resolvedImports,
     );
   } else {
     throw TypeError(
-      `importHook must return a static module record, got ${q(
-        staticModuleRecord,
-      )}`,
+      `importHook must provide a module source, got ${q(moduleSource)}`,
     );
   }