esm: leverage loaders when resolving subsequent loaders

Author: arcanis

lib/internal/modules/esm/loader.js

@@ -306,7 +306,7 @@ class ESMLoader {
 
   /**
    * Collect custom/user-defined hook(s). After all hooks have been collected,
-   * calls global preload hook(s).
+   * the global preload hook(s) must be called.
    * @param {KeyedExports} customLoaders
    *  A list of exports from user-defined loaders (as returned by
    *  ESMLoader.import()).
@@ -353,8 +353,6 @@ class ESMLoader {
         );
       }
     }
-
-    this.preload();
   }
 
   async eval(

lib/internal/process/esm_loader.js

@@ -2,7 +2,7 @@
 
 const {
   ArrayIsArray,
-  ObjectCreate,
+  ArrayPrototypePushApply,
 } = primordials;
 
 const {
@@ -13,6 +13,7 @@ const {
   hasUncaughtExceptionCaptureCallback,
 } = require('internal/process/execution');
 const { pathToFileURL } = require('internal/url');
+const { kEmptyObject } = require('internal/util');
 const {
   getModuleFromWrap,
 } = require('internal/vm/module');
@@ -58,15 +59,41 @@ async function initializeLoader() {
   const { getOptionValue } = require('internal/options');
   const customLoaders = getOptionValue('--experimental-loader');
   const preloadModules = getOptionValue('--import');
-  const loaders = await loadModulesInIsolation(customLoaders);
+
+  let cwd;
+  try {
+    cwd = process.cwd() + '/';
+  } catch {
+    cwd = '/';
+  }
+
+  const internalEsmLoader = new ESMLoader();
+  const allLoaders = [];
+
+  const parentUrl = pathToFileURL(cwd).href;
+
+  for (let i = 0; i < customLoaders.length; i++) {
+    const customLoader = customLoaders[i];
+
+    // Importation must be handled by internal loader to avoid polluting user-land
+    const keyedExportsSublist = await internalEsmLoader.import(
+      [customLoader],
+      parentUrl,
+      kEmptyObject,
+    );
+
+    internalEsmLoader.addCustomLoaders(keyedExportsSublist);
+    ArrayPrototypePushApply(allLoaders, keyedExportsSublist);
+  }
 
   // Hooks must then be added to external/public loader
   // (so they're triggered in userland)
-  esmLoader.addCustomLoaders(loaders);
+  esmLoader.addCustomLoaders(allLoaders);
+  esmLoader.preload();
 
   // Preload after loaders are added so they can be used
   if (preloadModules?.length) {
-    await loadModulesInIsolation(preloadModules, loaders);
+    await loadModulesInIsolation(preloadModules, allLoaders);
   }
 
   isESMInitialized = true;
@@ -79,20 +106,21 @@ function loadModulesInIsolation(specifiers, loaders = []) {
   try {
     cwd = process.cwd() + '/';
   } catch {
-    cwd = 'file:///';
+    cwd = '/';
   }
 
   // A separate loader instance is necessary to avoid cross-contamination
   // between internal Node.js and userland. For example, a module with internal
   // state (such as a counter) should be independent.
   const internalEsmLoader = new ESMLoader();
   internalEsmLoader.addCustomLoaders(loaders);
+  internalEsmLoader.preload();
 
   // Importation must be handled by internal loader to avoid poluting userland
   return internalEsmLoader.import(
     specifiers,
     pathToFileURL(cwd).href,
-    ObjectCreate(null),
+    kEmptyObject,
   );
 }
 

test/es-module/test-esm-loader-chaining.mjs

@@ -1,10 +1,10 @@
 import { spawnPromisified } from '../common/index.mjs';
 import * as fixtures from '../common/fixtures.mjs';
 import assert from 'node:assert';
+import { relative } from 'node:path';
 import { execPath } from 'node:process';
 import { describe, it } from 'node:test';
 
-
 const setupArgs = [
   '--no-warnings',
   '--input-type=module',
@@ -253,6 +253,23 @@ describe('ESM: loader chaining', { concurrency: true }, () => {
     assert.strictEqual(code, 0);
   });
 
+  it('should allow loaders to influence subsequent loader resolutions', async () => {
+    const { code, stderr } = await spawnPromisified(
+      execPath,
+      [
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-resolve-strip-xxx.mjs'),
+        '--loader',
+        `xxx/${relative(process.cwd(), fixtures.path('es-module-loaders', 'loader-resolve-strip-yyy.mjs'))}`,
+        ...commonArgs,
+      ],
+      { encoding: 'utf8' },
+    );
+
+    assert.strictEqual(stderr, '');
+    assert.strictEqual(code, 0);
+  });
+
   it('should throw when the resolve chain is broken', async () => {
     const { code, stderr, stdout } = await spawnPromisified(
       execPath,

test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs

@@ -1,4 +1,10 @@
-export async function load(url) {
+export async function load(url, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (url.includes('loader')) {
+    return next(url);
+  }
+
   const val = url.includes('42')
     ? '42'
     : '"foo"';

test/fixtures/es-module-loaders/loader-load-incomplete.mjs

@@ -1,4 +1,10 @@
-export async function load() {
+export async function load(url, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (url.includes('loader')) {
+    return next(url);
+  }
+
   return {
     format: 'module',
     source: 'export default 42',

test/fixtures/es-module-loaders/loader-load-passthru.mjs

@@ -1,4 +1,10 @@
 export async function load(url, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (url.includes('loader')) {
+    return next(url);
+  }
+
   console.log('load passthru'); // This log is deliberate
   return next(url);
 }

test/fixtures/es-module-loaders/loader-resolve-42.mjs

@@ -1,4 +1,10 @@
 export async function resolve(specifier, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (specifier.includes('loader')) {
+    return next(specifier);
+  }
+
   console.log('resolve 42'); // This log is deliberate
   console.log('next<HookName>:', next.name); // This log is deliberate
 

test/fixtures/es-module-loaders/loader-resolve-foo.mjs

@@ -1,4 +1,10 @@
 export async function resolve(specifier, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (specifier.includes('loader')) {
+    return next(specifier);
+  }
+
   console.log('resolve foo'); // This log is deliberate
   return next('file:///foo.mjs');
 }

test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs

@@ -1,4 +1,10 @@
-export async function resolve() {
+export async function resolve(specifier, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (specifier.includes('loader')) {
+    return next(specifier);
+  }
+
   return {
     url: 'file:///incomplete-resolve-chain.js',
   };

test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs

@@ -1,4 +1,10 @@
 export async function resolve(url, context, next) {
+  // Otherwise we can't use this loader in chaining context, as it would
+  // overwrite other subsequent loaders
+  if (url.includes('loader')) {
+    return next(url);
+  }
+
   const {
     format,
     url: nextUrl,