fix(ses): naming consistency, types, defensive iterator

packages/ses/README.md

@@ -384,6 +384,41 @@ const odd = new Compartment({}, {}, {
 });
 ```
 
+### importNowHook
+
+Additionally, an `importNowHook` may be provided that the compartment will use as means to synchronously load modules not seen before in situations where calling out to asynchronous `importHook` is not possible.
+Specifically, when `compartmentInstance.importNow('specifier')` is called, the compartment will first look up module records it's already aware of and call `moduleMapHook` and if none of that is successful in finding a module record matching the specifier, it will call `importNowHook` expecting to synchronously receive the same record type as from `importHook`
+
+```js
+import 'ses';
+import { StaticModuleRecord } from '@endo/static-module-record';
+
+const c1 = new Compartment({}, {
+  'c2': c2.module('./main.js'),
+}, {
+  name: "first compartment",
+  resolveHook: (moduleSpecifier, moduleReferrer) => {
+    return resolve(moduleSpecifier, moduleReferrer);
+  },
+  importHook: async moduleSpecifier => {
+    const moduleLocation = locate(moduleSpecifier);
+    const moduleText = await retrieve(moduleLocation);
+    return new StaticModuleRecord(moduleText, moduleLocation);
+  },
+  importNowHook: moduleSpecifier => {
+    const moduleLocation = locate(moduleSpecifier);
+    // Platform specific synchronous read API can be used
+    const moduleText = fs.readFileSync(moduleLocation);
+    return new StaticModuleRecord(moduleText, moduleLocation);
+  },
+});
+//...                   | importHook | importNowHook
+await c1.import('a'); //| called     | not called
+c1.importNow('b');    //| not called | called
+c1.importNow('a');    //| not called | not called
+c1.importNow('c2');   //| not called | not called
+```
+
 ### Third-party modules
 
 To incorporate modules not implemented as JavaScript modules, third-parties may

packages/ses/src/commons.js

@@ -133,6 +133,8 @@ export const { prototype: weakmapPrototype } = WeakMap;
 export const { prototype: weaksetPrototype } = WeakSet;
 export const { prototype: functionPrototype } = Function;
 export const { prototype: promisePrototype } = Promise;
+// eslint-disable-next-line no-empty-function
+export const { prototype: iteratorPrototype } = getPrototypeOf(function* () {});
 
 export const typedArrayPrototype = getPrototypeOf(Uint8Array.prototype);
 
@@ -191,6 +193,10 @@ export const stringEndsWith = uncurryThis(stringPrototype.endsWith);
 export const stringIncludes = uncurryThis(stringPrototype.includes);
 export const stringIndexOf = uncurryThis(stringPrototype.indexOf);
 export const stringMatch = uncurryThis(stringPrototype.match);
+//
+export const iteratorNext = uncurryThis(iteratorPrototype.next);
+export const iteratorThrow = uncurryThis(iteratorPrototype.throw);
+
 /**
  * @type { &
  *   ((thisArg: string, searchValue: { [Symbol.replace](string: string, replaceValue: string): string; }, replaceValue: string) => string) &

packages/ses/src/compartment.js

@@ -22,7 +22,7 @@ import {
   setGlobalObjectEvaluators,
 } from './global-object.js';
 import { sharedGlobalPropertyNames } from './permits.js';
-import { load, loadSync } from './module-load.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';
@@ -160,7 +160,7 @@ export const CompartmentPrototype = {
     }
 
     assertModuleHooks(this);
-    loadSync(privateFields, moduleAliases, this, specifier);
+    loadNow(privateFields, moduleAliases, this, specifier);
     return compartmentImportNow(/** @type {Compartment} */ (this), specifier);
   },
 };

packages/ses/src/module-load.js

@@ -23,6 +23,8 @@ import {
   promiseThen,
   values,
   weakmapGet,
+  iteratorNext,
+  iteratorThrow,
 } from './commons.js';
 import { assert } from './error/assert.js';
 
@@ -33,27 +35,27 @@ const noop = () => {};
 async function asyncTrampoline(generatorFunc, args, errorWrapper) {
   // TODO: add iterator prototype methods to commons
   const iterator = generatorFunc(...args);
-  let result = iterator.next();
+  let result = iteratorNext(iterator);
   while (!result.done) {
     try {
       // eslint-disable-next-line no-await-in-loop
       const val = await result.value;
-      result = iterator.next(val);
+      result = iteratorNext(iterator, val);
     } catch (error) {
-      result = iterator.throw(errorWrapper(error));
+      result = iteratorThrow(iterator, errorWrapper(error));
     }
   }
   return result.value;
 }
 
 function syncTrampoline(generatorFunc, args) {
-  let iterator = generatorFunc(...args);
-  let result = iterator.next();
+  const iterator = generatorFunc(...args);
+  let result = iteratorNext(iterator);
   while (!result.done) {
     try {
-      result = iterator.next(result.value);
+      result = iteratorNext(iterator, result.value);
     } catch (error) {
-      result = iterator.throw(error); 
+      result = iteratorThrow(iterator, error);
     }
   }
   return result.value;
@@ -322,27 +324,31 @@ const memoizedLoadWithErrorAnnotation = (
 
 function asyncOverseer() {
   /** @type {Set<Promise<undefined>>} */
-  const pendingJobs = new Set(); // TODO: why is this a Set? Order seems to matter and duplicates don't seem possible
+  const pendingJobs = new Set();
   /** @type {Array<Error>} */
   const errors = [];
 
+  /**
+   * Enqueues a job that starts immediately but won't be awaited until drainQueue is called.
+   *
+   * @template {any[]} T
+   * @param {(...args: T)=>Promise<*>} func
+   * @param {T} args
+   */
   const enqueueJob = (func, args) => {
     setAdd(
       pendingJobs,
-      // WARNING: synchronously thrown errors will not be captured. That's
-      // deliberate - synchronous errors are not loading errors that are
-      // worth aggregating, they're implementation errors we want them
-      // thrown immedaitely.
       promiseThen(func(...args), noop, error => {
-        // TODO: wrapping func instead of passing noop to then might be more performant for ensuring nothing usable is returned
         arrayPush(errors, error);
       }),
     );
   };
+  /**
+   * Sequentially awaits pending jobs and returns an array of errors
+   *
+   * @returns {Promise<Array<Error>>}
+   */
   const drainQueue = async () => {
-    // Each job is a promise for undefined, regardless of success or failure.
-    // Before we add a job to the queue, we catch any error and push it into the
-    // `errors` accumulator.
     for (const job of pendingJobs) {
       // eslint-disable-next-line no-await-in-loop
       await job;
@@ -352,6 +358,12 @@ function asyncOverseer() {
   return { enqueueJob, drainQueue };
 }
 
+/**
+ * @param {object} options
+ * @param {Array<Error>} options.errors
+ * @param {string} options.errorPrefix
+ * @param {boolean} [options.debug]
+ */
 function throwAggregateError({ errors, errorPrefix, debug = false }) {
   // Throw an aggregate error if there were any errors.
   if (errors.length > 0) {
@@ -364,6 +376,9 @@ function throwAggregateError({ errors, errorPrefix, debug = false }) {
   }
 }
 
+const preferSync = (_asyncImpl, syncImpl) => syncImpl;
+const preferAsync = (asyncImpl, _syncImpl) => asyncImpl;
+
 /*
  * `load` asynchronously gathers the `StaticModuleRecord`s for a module and its
  * transitive dependencies.
@@ -393,7 +408,7 @@ export const load = async (
     compartment,
     moduleSpecifier,
     enqueueJob,
-    (asyncImpl, _syncImpl) => asyncImpl,
+    preferAsync,
     moduleLoads,
   ]);
 
@@ -410,13 +425,13 @@ export const load = async (
 };
 
 /*
- * `loadSync` synchronously gathers the `StaticModuleRecord`s for a module and its
+ * `loadNow` synchronously gathers the `StaticModuleRecord`s for a module and its
  * transitive dependencies.
  * The module records refer to each other by a reference to the dependency's
  * compartment and the specifier of the module within its own compartment.
  * This graph is then ready to be synchronously linked and executed.
  */
-export const loadSync = (
+export const loadNow = (
   compartmentPrivateFields,
   moduleAliases,
   compartment,
@@ -447,7 +462,7 @@ export const loadSync = (
     compartment,
     moduleSpecifier,
     enqueueJob,
-    (_asyncImpl, syncImpl) => syncImpl,
+    preferSync,
     moduleLoads,
   ]);
 

packages/ses/test/test-import-cjs.js

@@ -526,7 +526,6 @@ test('export name as default from CommonJS module', async t => {
   await compartment.import('./main.js');
 });
 
-
 test('synchronous loading via importNowHook', async t => {
   t.plan(1);
 
@@ -561,7 +560,7 @@ test('synchronous loading via importNowHook', async t => {
     {},
     {
       resolveHook: resolveNode,
-      importHook: async ()=>{},
+      importHook: async () => {},
       importNowHook,
     },
   );