module: make CJS load from ESM loader

PR-URL: #47999 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me>
nodejs · Aug 13, 2023 · 3d20582 · anonrig · Aug 13, 2023 · 3d20582
1 parent a4928b0
commit 3d20582
Show file tree

Hide file tree

Showing 10 changed files with 426 additions and 105 deletions.
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -908,6 +908,9 @@ export function resolve(specifier, context, nextResolve) {
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47999
+    description: Add support for `source` with format `commonjs`.
   - version:
     - v18.6.0
     - v16.17.0
@@ -945,20 +948,43 @@ validating the import assertion.
 
 The final value of `format` must be one of the following:
 
-| `format`     | Description                    | Acceptable types for `source` returned by `load`      |
-| ------------ | ------------------------------ | ----------------------------------------------------- |
-| `'builtin'`  | Load a Node.js builtin module  | Not applicable                                        |
-| `'commonjs'` | Load a Node.js CommonJS module | Not applicable                                        |
-| `'json'`     | Load a JSON file               | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] } |
-| `'module'`   | Load an ES module              | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] } |
-| `'wasm'`     | Load a WebAssembly module      | { [`ArrayBuffer`][], [`TypedArray`][] }               |
+| `format`     | Description                    | Acceptable types for `source` returned by `load`                           |
+| ------------ | ------------------------------ | -------------------------------------------------------------------------- |
+| `'builtin'`  | Load a Node.js builtin module  | Not applicable                                                             |
+| `'commonjs'` | Load a Node.js CommonJS module | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][], `null`, `undefined` } |
+| `'json'`     | Load a JSON file               | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] }                      |
+| `'module'`   | Load an ES module              | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] }                      |
+| `'wasm'`     | Load a WebAssembly module      | { [`ArrayBuffer`][], [`TypedArray`][] }                                    |
 
 The value of `source` is ignored for type `'builtin'` because currently it is
-not possible to replace the value of a Node.js builtin (core) module. The value
-of `source` is ignored for type `'commonjs'` because the CommonJS module loader
-does not provide a mechanism for the ES module loader to override the
-[CommonJS module return value](#commonjs-namespaces). This limitation might be
-overcome in the future.
+not possible to replace the value of a Node.js builtin (core) module.
+
+The value of `source` can be omitted for type `'commonjs'`. When a `source` is
+provided, all `require` calls from this module will be processed by the ESM
+loader with registered `resolve` and `load` hooks; all `require.resolve` calls
+from this module will be processed by the ESM loader with registered `resolve`
+hooks; `require.extensions` and monkey-patching on the CommonJS module loader
+will not apply. If `source` is undefined or `null`, it will be handled by the
+CommonJS module loader and `require`/`require.resolve` calls will not go through
+the registered hooks. This behavior for nullish `source` is temporary — in the
+future, nullish `source` will not be supported.
+
+The Node.js own `load` implementation, which is the value of `next` for the last
+loader in the `load` chain, returns `null` for `source` when `format` is
+`'commonjs'` for backward compatibility. Here is an example loader that would
+opt-in to using the non-default behavior:
+
+```js
+import { readFile } from 'node:fs/promises';
+
+export async function load(url, context, nextLoad) {
+  const result = await nextLoad(url, context);
+  if (result.format === 'commonjs') {
+    result.source ??= await readFile(new URL(result.responseURL ?? url));
+  }
+  return result;
+}
+```
 
 > **Caveat**: The ESM `load` hook and namespaced exports from CommonJS modules
 > are incompatible. Attempting to use them together will result in an empty

diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js
@@ -10,6 +10,7 @@ const { kEmptyObject } = require('internal/util');
 const { defaultGetFormat } = require('internal/modules/esm/get_format');
 const { validateAssertions } = require('internal/modules/esm/assert');
 const { getOptionValue } = require('internal/options');
+const { readFileSync } = require('fs');
 
 // Do not eagerly grab .manifest, it may be in TDZ
 const policy = getOptionValue('--experimental-policy') ?
@@ -69,12 +70,35 @@ async function getSource(url, context) {
   return { __proto__: null, responseURL, source };
 }
 
+function getSourceSync(url, context) {
+  const parsed = new URL(url);
+  const responseURL = url;
+  let source;
+  if (parsed.protocol === 'file:') {
+    source = readFileSync(parsed);
+  } else if (parsed.protocol === 'data:') {
+    const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname);
+    if (!match) {
+      throw new ERR_INVALID_URL(url);
+    }
+    const { 1: base64, 2: body } = match;
+    source = BufferFrom(decodeURIComponent(body), base64 ? 'base64' : 'utf8');
+  } else {
+    const supportedSchemes = ['file', 'data'];
+    throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, supportedSchemes);
+  }
+  if (policy?.manifest) {
+    policy.manifest.assertIntegrity(parsed, source);
+  }
+  return { __proto__: null, responseURL, source };
+}
+
 
 /**
  * Node.js default load hook.
  * @param {string} url
- * @param {object} context
- * @returns {object}
+ * @param {LoadContext} context
+ * @returns {LoadReturn}
  */
 async function defaultLoad(url, context = kEmptyObject) {
   let responseURL = url;
@@ -108,6 +132,51 @@ async function defaultLoad(url, context = kEmptyObject) {
     source,
   };
 }
+/**
+ * @typedef LoadContext
+ * @property {string} [format] A hint (possibly returned from `resolve`)
+ * @property {string | Buffer | ArrayBuffer} [source] source
+ * @property {Record<string, string>} [importAssertions] import attributes
+ */
+
+/**
+ * @typedef LoadReturn
+ * @property {string} format format
+ * @property {URL['href']} responseURL The module's fully resolved URL
+ * @property {Buffer} source source
+ */
+
+/**
+ * @param {URL['href']} url
+ * @param {LoadContext} [context]
+ * @returns {LoadReturn}
+ */
+function defaultLoadSync(url, context = kEmptyObject) {
+  let responseURL = url;
+  const { importAssertions } = context;
+  let {
+    format,
+    source,
+  } = context;
+
+  format ??= defaultGetFormat(new URL(url), context);
+
+  validateAssertions(url, format, importAssertions);
+
+  if (format === 'builtin') {
+    source = null;
+  } else if (source == null) {
+    ({ responseURL, source } = getSourceSync(url, context));
+  }
+
+  return {
+    __proto__: null,
+    format,
+    responseURL,
+    source,
+  };
+}
+
 
 /**
  * throws an error if the protocol is not one of the protocols
@@ -160,5 +229,6 @@ function throwUnknownModuleFormat(url, format) {
 
 module.exports = {
   defaultLoad,
+  defaultLoadSync,
   throwUnknownModuleFormat,
 };
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -10,6 +10,7 @@ const {
 } = primordials;
 
 const {
+  ERR_REQUIRE_ESM,
   ERR_UNKNOWN_MODULE_FORMAT,
 } = require('internal/errors').codes;
 const { getOptionValue } = require('internal/options');
@@ -18,7 +19,7 @@ const { emitExperimentalWarning } = require('internal/util');
 const {
   getDefaultConditions,
 } = require('internal/modules/esm/utils');
-let defaultResolve, defaultLoad, importMetaInitializer;
+let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer;
 
 function newResolveCache() {
   const { ResolveCache } = require('internal/modules/esm/module_map');
@@ -220,7 +221,12 @@ class ModuleLoader {
     return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions);
   }
 
-  getJobFromResolveResult(resolveResult, parentURL, importAssertions) {
+  getModuleJobSync(specifier, parentURL, importAssertions) {
+    const resolveResult = this.resolveSync(specifier, parentURL, importAssertions);
+    return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions, true);
+  }
+
+  getJobFromResolveResult(resolveResult, parentURL, importAssertions, sync) {
     const { url, format } = resolveResult;
     const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions;
     let job = this.loadCache.get(url, resolvedImportAssertions.type);
@@ -231,7 +237,7 @@ class ModuleLoader {
     }
 
     if (job === undefined) {
-      job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format);
+      job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format, sync);
     }
 
     return job;
@@ -248,17 +254,8 @@ class ModuleLoader {
    *                          `resolve` hook
    * @returns {Promise<ModuleJob>} The (possibly pending) module job
    */
-  #createModuleJob(url, importAssertions, parentURL, format) {
-    const moduleProvider = async (url, isMain) => {
-      const {
-        format: finalFormat,
-        responseURL,
-        source,
-      } = await this.load(url, {
-        format,
-        importAssertions,
-      });
-
+  #createModuleJob(url, importAssertions, parentURL, format, sync) {
+    const callTranslator = ({ format: finalFormat, responseURL, source }, isMain) => {
       const translator = getTranslators().get(finalFormat);
 
       if (!translator) {
@@ -267,6 +264,10 @@ class ModuleLoader {
 
       return FunctionPrototypeCall(translator, this, responseURL, source, isMain);
     };
+    const context = { format, importAssertions };
+    const moduleProvider = sync ?
+      (url, isMain) => callTranslator(this.loadSync(url, context), isMain) :
+      async (url, isMain) => callTranslator(await this.load(url, context), isMain);
 
     const inspectBrk = (
       parentURL === undefined &&
@@ -285,6 +286,7 @@ class ModuleLoader {
       moduleProvider,
       parentURL === undefined,
       inspectBrk,
+      sync,
     );
 
     this.loadCache.set(url, importAssertions.type, job);
@@ -388,6 +390,24 @@ class ModuleLoader {
     return result;
   }
 
+  loadSync(url, context) {
+    defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
+
+    let result = this.#customizations ?
+      this.#customizations.loadSync(url, context) :
+      defaultLoadSync(url, context);
+    let format = result?.format;
+    if (format === 'module') {
+      throw new ERR_REQUIRE_ESM(url, true);
+    }
+    if (format === 'commonjs') {
+      format = 'require-commonjs';
+      result = { __proto__: result, format };
+    }
+    this.validateLoadResult(url, format);
+    return result;
+  }
+
   validateLoadResult(url, format) {
     if (format == null) {
       require('internal/modules/esm/load').throwUnknownModuleFormat(url, format);
@@ -465,6 +485,9 @@ class CustomizedModuleLoader {
   load(url, context) {
     return hooksProxy.makeAsyncRequest('load', undefined, url, context);
   }
+  loadSync(url, context) {
+    return hooksProxy.makeSyncRequest('load', undefined, url, context);
+  }
 
   importMetaInitialize(meta, context, loader) {
     hooksProxy.importMetaInitialize(meta, context, loader);

diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -51,17 +51,26 @@ class ModuleJob {
   // `loader` is the Loader instance used for loading dependencies.
   // `moduleProvider` is a function
   constructor(loader, url, importAssertions = { __proto__: null },
-              moduleProvider, isMain, inspectBrk) {
+              moduleProvider, isMain, inspectBrk, sync = false) {
     this.loader = loader;
     this.importAssertions = importAssertions;
     this.isMain = isMain;
     this.inspectBrk = inspectBrk;
 
+    this.url = url;
+
     this.module = undefined;
     // Expose the promise to the ModuleWrap directly for linking below.
     // `this.module` is also filled in below.
     this.modulePromise = ReflectApply(moduleProvider, loader, [url, isMain]);
 
+    if (sync) {
+      this.module = this.modulePromise;
+      this.modulePromise = PromiseResolve(this.module);
+    } else {
+      this.modulePromise = PromiseResolve(this.modulePromise);
+    }
+
     // Wait for the ModuleWrap instance being linked with all dependencies.
     const link = async () => {
       this.module = await this.modulePromise;
@@ -186,6 +195,20 @@ class ModuleJob {
     }
   }
 
+  runSync() {
+    assert(this.module instanceof ModuleWrap);
+    if (this.instantiated !== undefined) {
+      return { __proto__: null, module: this.module };
+    }
+
+    this.module.instantiate();
+    this.instantiated = PromiseResolve();
+    const timeout = -1;
+    const breakOnSigint = false;
+    this.module.evaluate(timeout, breakOnSigint);
+    return { __proto__: null, module: this.module };
+  }
+
   async run() {
     await this.instantiate();
     const timeout = -1;