esm: support nested loader chains

Fixes nodejs#48515
izaakschroeder · Jun 27, 2023 · 30cb005 · 30cb005
1 parent 42d8143
commit 30cb005
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 23 deletions.
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -54,6 +54,7 @@ const {
 const {
   getDefaultConditions,
   loaderWorkerId,
+  createHooksLoader,
 } = require('internal/modules/esm/utils');
 const { deserializeError } = require('internal/error_serdes');
 const {
@@ -136,6 +137,10 @@ class Hooks {
     this.addCustomLoader(urlOrSpecifier, keyedExports);
   }
 
+  getChains() {
+    return this.#chains;
+  }
+
   /**
    * Collect custom/user-defined module loader hook(s).
    * After all hooks have been collected, the global preload hook(s) must be initialized.
@@ -220,16 +225,25 @@ class Hooks {
     originalSpecifier,
     parentURL,
     importAssertions = { __proto__: null },
+  ) {
+    return this.resolveWithChain(this.#chains.resolve, originalSpecifier, parentURL, importAssertions);
+  }
+
+  async resolveWithChain(
+    chain,
+    originalSpecifier,
+    parentURL,
+    importAssertions = { __proto__: null },
   ) {
     throwIfInvalidParentURL(parentURL);
 
-    const chain = this.#chains.resolve;
     const context = {
       conditions: getDefaultConditions(),
       importAssertions,
       parentURL,
     };
     const meta = {
+      hooks: this,
       chainFinished: null,
       context,
       hookErrIdentifier: '',
@@ -344,8 +358,12 @@ class Hooks {
    * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }>}
    */
   async load(url, context = {}) {
-    const chain = this.#chains.load;
+    return this.loadWithChain(this.#chains.load, url, context)
+  }
+
+  async loadWithChain(chain, url, context = {}) {
     const meta = {
+      hooks: this,
       chainFinished: null,
       context,
       hookErrIdentifier: '',
@@ -749,7 +767,31 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
         ObjectAssign(meta.context, context);
       }
 
-      const output = await hook(arg0, meta.context, nextNextHook);
+      const withESMLoader = require('internal/process/esm_loader').withESMLoader;
+
+      const chains = meta.hooks.getChains();
+      const loadChain = chain === chains.load ? chains.load.slice(0, generatedHookIndex) : chains.load;
+      const resolveChain = chain === chains.resolve ? chains.resolve.slice(0, generatedHookIndex) : chains.resolve;
+      const loader = createHooksLoader({
+        async resolve(
+          originalSpecifier,
+          parentURL,
+          importAssertions = { __proto__: null }
+        ) {
+          return await meta.hooks.resolveWithChain(
+            resolveChain,
+            originalSpecifier,
+            parentURL,
+            importAssertions,
+          );
+        },
+        async load(url, context = {}) {
+          return await meta.hooks.loadWithChain(loadChain, url, context);
+        },
+      })
+      const output = await withESMLoader(loader, async () => {
+        return await hook(arg0, meta.context, nextNextHook);
+      });
 
       validateOutput(outputErrIdentifier, output);
 

diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
@@ -24,6 +24,7 @@ const {
 } = require('internal/vm/module');
 const assert = require('internal/assert');
 
+
 const callbackMap = new SafeWeakMap();
 function setCallbackForWrap(wrap, data) {
   callbackMap.set(wrap, data);
@@ -107,26 +108,19 @@ function isLoaderWorker() {
   return _isLoaderWorker;
 }
 
-async function initializeHooks() {
-  const customLoaderURLs = getOptionValue('--experimental-loader');
-
-  let cwd;
-  try {
-    // `process.cwd()` can fail if the parent directory is deleted while the process runs.
-    cwd = process.cwd() + '/';
-  } catch {
-    cwd = '/';
-  }
-
-
-  const { Hooks } = require('internal/modules/esm/hooks');
-  const hooks = new Hooks();
-
+const createHooksLoader = (hooks) => {
+  // TODO: HACK: `DefaultModuleLoader` depends on `getDefaultConditions` defined in
+  // this file so we have a circular reference going on. If that function was in
+  // it's on file we could just expose this class generically.
   const { DefaultModuleLoader } = require('internal/modules/esm/loader');
-  class ModuleLoader extends DefaultModuleLoader {
-    loaderType = 'internal';
+  class HooksModuleLoader extends DefaultModuleLoader {
+    #hooks;
+    constructor(hooks) {
+      super();
+      this.#hooks = hooks;
+    }
     async #getModuleJob(specifier, parentURL, importAssertions) {
-      const resolveResult = await hooks.resolve(specifier, parentURL, importAssertions);
+      const resolveResult = await this.#hooks.resolve(specifier, parentURL, importAssertions);
       return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions);
     }
     getModuleJob(specifier, parentURL, importAssertions) {
@@ -143,9 +137,42 @@ async function initializeHooks() {
         },
       };
     }
-    load(url, context) { return hooks.load(url, context); }
+    resolve(
+      originalSpecifier,
+      parentURL,
+      importAssertions = { __proto__: null },
+    ) {
+      return this.#hooks.resolve(
+        originalSpecifier,
+        parentURL,
+        importAssertions
+      );
+    }
+    load(url, context = {}) {
+      return this.#hooks.load(url, context);
+    }
   }
-  const privateModuleLoader = new ModuleLoader();
+  return new HooksModuleLoader(hooks);
+}
+
+async function initializeHooks() {
+  const customLoaderURLs = getOptionValue('--experimental-loader');
+
+  let cwd;
+  try {
+    // `process.cwd()` can fail if the parent directory is deleted while the process runs.
+    cwd = process.cwd() + '/';
+  } catch {
+    cwd = '/';
+  }
+
+
+  const { Hooks } = require('internal/modules/esm/hooks');
+  const hooks = new Hooks();
+
+
+  const privateModuleLoader = createHooksLoader(hooks);
+  privateModuleLoader.loaderType = 'internal';
   const parentURL = pathToFileURL(cwd).href;
 
   // TODO(jlenon7): reuse the `Hooks.register()` method for registering loaders.
@@ -175,4 +202,5 @@ module.exports = {
   getConditionsSet,
   loaderWorkerId: 'internal/modules/esm/worker',
   isLoaderWorker,
+  createHooksLoader,
 };
diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
@@ -15,6 +15,15 @@ const { kEmptyObject } = require('internal/util');
 let esmLoader;
 
 module.exports = {
+  async withESMLoader(loader, fn) {
+    const oldLoader = esmLoader;
+    esmLoader = loader;
+    try {
+      return await fn();
+    } finally {
+      esmLoader = oldLoader;
+    }
+  },
   get esmLoader() {
     return esmLoader ??= createModuleLoader(true);
   },

diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs
@@ -470,4 +470,38 @@ describe('ESM: loader chaining', { concurrency: true }, () => {
     assert.match(stderr, /'load' hook's nextLoad\(\) context/);
     assert.strictEqual(code, 1);
   });
+
+  it('should allow loaders to influence subsequent loader `import()` calls in `resolve`', async () => {
+    const { code, stderr, stdout } = await spawnPromisified(
+      execPath,
+      [
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-resolve-strip-xxx.mjs'),
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-resolve-dynamic-import.mjs'),
+        ...commonArgs,
+      ],
+      { encoding: 'utf8' },
+    );
+    assert.strictEqual(stderr, '');
+    assert.match(stdout, /resolve dynamic import/); // It did go thru resolve-dynamic
+    assert.strictEqual(code, 0);
+  });
+
+  it('should allow loaders to influence subsequent loader `import()` calls in `load`', async () => {
+    const { code, stderr, stdout } = await spawnPromisified(
+      execPath,
+      [
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-resolve-strip-xxx.mjs'),
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-load-dynamic-import.mjs'),
+        ...commonArgs,
+      ],
+      { encoding: 'utf8' },
+    );
+    assert.strictEqual(stderr, '');
+    assert.match(stdout, /load dynamic import/); // It did go thru load-dynamic
+    assert.strictEqual(code, 0);
+  });
 });
diff --git a/test/fixtures/es-module-loaders/loader-load-dynamic-import.mjs b/test/fixtures/es-module-loaders/loader-load-dynamic-import.mjs
@@ -0,0 +1,14 @@
+import { writeSync } from 'node:fs';
+
+
+export async function load(url, context, next) {
+  if (url === 'node:fs' || url.includes('loader')) {
+    return next(url);
+  }
+
+  // Here for asserting dynamic import
+  await import('xxx/loader-load-passthru.mjs');
+
+  writeSync(1, 'load dynamic import' + '\n'); // Signal that this specific hook ran
+  return next(url, context);
+}
diff --git a/test/fixtures/es-module-loaders/loader-resolve-dynamic-import.mjs b/test/fixtures/es-module-loaders/loader-resolve-dynamic-import.mjs
@@ -0,0 +1,14 @@
+import { writeSync } from 'node:fs';
+
+
+export async function resolve(specifier, context, next) {
+  if (specifier === 'node:fs' || specifier.includes('loader')) {
+    return next(specifier);
+  }
+
+  // Here for asserting dynamic import
+  await import('xxx/loader-resolve-passthru.mjs');
+
+  writeSync(1, 'resolve dynamic import' + '\n'); // Signal that this specific hook ran
+  return next(specifier);
+}