chore(compartment-mapper): post-rebase fixes

Author: boneskull

packages/compartment-mapper/src/import-hook.js

@@ -15,7 +15,8 @@
  *   ImportHook,
  *   ImportNowHook,
  *   RedirectStaticModuleInterface,
- *   StaticModuleType
+ *   StaticModuleType,
+ VirtualModuleSource
  * } from 'ses'
  * @import {
  *   CompartmentDescriptor,
@@ -691,19 +692,13 @@ export function makeImportNowHookMaker(
 
     /** @type {ImportNowHook} */
     const importNowHook = moduleSpecifier => {
-      // many dynamically-required specifiers will be absolute paths owing to use of `require.resolve()` and `path.resolve()`
-      if (isAbsolute(moduleSpecifier)) {
-        const record = findRedirect({
-          compartmentDescriptor,
-          compartmentDescriptors,
-          compartments,
-          absoluteModuleSpecifier: moduleSpecifier,
-        });
-        if (record) {
-          return record;
-        }
-        // if there is no record found this way, we will search for it instead of considering it to be an exit module
-      } else if (moduleSpecifier !== '.' && !moduleSpecifier.startsWith('./')) {
+      /**
+       * Attempt to load the moduleSpecifier via an {@link ExitModuleImportNowHook}, if one exists.
+       *
+       * If it doesn't exist, then throw an exception.
+       * @returns {VirtualModuleSource}
+       */
+      const tryExitModuleImportNowHook = () => {
         if (exitModuleImportNowHook) {
           // This hook is responsible for ensuring that the moduleSpecifier
           // actually refers to an exit module.
@@ -722,14 +717,38 @@ export function makeImportNowHookMaker(
             });
             return exitRecord;
           }
+          throw Error(
+            `Cannot find external module ${q(
+              moduleSpecifier,
+            )} in package at ${packageLocation}`,
+          );
+        } else {
+          throw Error(
+            `Cannot find external module ${q(
+              moduleSpecifier,
+            )} from package at ${packageLocation}; try providing an importNowHook`,
+          );
         }
-        throw Error(
-          `Cannot find external module ${q(
-            moduleSpecifier,
-          )} in package ${packageLocation}`,
-        );
+      };
+
+      // many dynamically-required specifiers will be absolute paths owing to use of `require.resolve()` and `path.resolve()`
+      if (isAbsolute(moduleSpecifier)) {
+        const record = findRedirect({
+          compartmentDescriptor,
+          compartmentDescriptors,
+          compartments,
+          absoluteModuleSpecifier: moduleSpecifier,
+        });
+        if (record) {
+          return record;
+        }
+      } else if (moduleSpecifier !== '.' && !moduleSpecifier.startsWith('./')) {
+        // could be a builtin, which means we should not bother bouncing on the trampoline to find it.
+        return tryExitModuleImportNowHook();
       }
 
+      // we might have an absolute path here, but it might be within the compartment, so
+      // we will try to find it.
       const candidates = nominateCandidates(moduleSpecifier, searchSuffixes);
 
       const record = syncTrampoline(
@@ -759,11 +778,8 @@ export function makeImportNowHookMaker(
         return record;
       }
 
-      throw new Error(
-        `Could not import module: ${q(
-          moduleSpecifier,
-        )}; try providing an importNowHook`,
-      );
+      // at this point, we haven't found the module by guessing, so we'll try the importer-of-last-resort
+      return tryExitModuleImportNowHook();
     };
 
     return importNowHook;

packages/compartment-mapper/test/dynamic-require.test.js

@@ -407,7 +407,12 @@ test('inter-package and exit module dynamic require policy is enforced', async t
           },
           'hooked>dynamic': {
             packages: {
-              'is-ok': true,
+              'hooked>dynamic>is-ok': true,
+            },
+          },
+          'hooked>dynamic>is-ok': {
+            packages: {
+              'hooked>dynamic>is-ok>is-not-ok': true,
             },
           },
         },
@@ -469,7 +474,12 @@ test('inter-package and exit module dynamic require works ("node:"-namespaced)',
         },
         'hooked>dynamic': {
           packages: {
-            'is-ok': true,
+            'hooked>dynamic>is-ok': true,
+          },
+        },
+        'hooked>dynamic>is-ok': {
+          packages: {
+            'hooked>dynamic>is-ok>is-not-ok': true,
           },
         },
       },
@@ -619,5 +629,3 @@ test('dynamic require of missing module falls through to importNowHook', async t
     },
   );
 });
-
-// test('dynamic require of external module which imports a third module', async t => {});