test_runner: do not expose internal loader

aduh95 · targos · commit fe793a61033e · 2024-08-14T08:37:04.000+02:00
PR-URL: #54106 Fixes: #54071 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -145,13 +145,15 @@ class Hooks {
    * @param {any} [data] Arbitrary data to be passed from the custom
    * loader (user-land) to the worker.
    */
-  async register(urlOrSpecifier, parentURL, data) {
+  async register(urlOrSpecifier, parentURL, data, isInternal) {
     const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
-    const keyedExports = await cascadedLoader.import(
-      urlOrSpecifier,
-      parentURL,
-      kEmptyObject,
-    );
+    const keyedExports = isInternal ?
+      require(urlOrSpecifier) :
+      await cascadedLoader.import(
+        urlOrSpecifier,
+        parentURL,
+        kEmptyObject,
+      );
     await this.addCustomLoader(urlOrSpecifier, keyedExports, data);
   }
 
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -491,15 +491,15 @@ class ModuleLoader {
   /**
    * @see {@link CustomizedModuleLoader.register}
    */
-  register(specifier, parentURL, data, transferList) {
+  register(specifier, parentURL, data, transferList, isInternal) {
     if (!this.#customizations) {
       // `CustomizedModuleLoader` is defined at the bottom of this file and
       // available well before this line is ever invoked. This is here in
       // order to preserve the git diff instead of moving the class.
       // eslint-disable-next-line no-use-before-define
       this.setCustomizations(new CustomizedModuleLoader());
     }
-    return this.#customizations.register(`${specifier}`, `${parentURL}`, data, transferList);
+    return this.#customizations.register(`${specifier}`, `${parentURL}`, data, transferList, isInternal);
   }
 
   /**
@@ -636,10 +636,11 @@ class CustomizedModuleLoader {
    * @param {any} [data] Arbitrary data to be passed from the custom loader
    * (user-land) to the worker.
    * @param {any[]} [transferList] Objects in `data` that are changing ownership
+   * @param {boolean} [isInternal] For internal loaders that should not be publicly exposed.
    * @returns {{ format: string, url: URL['href'] }}
    */
-  register(originalSpecifier, parentURL, data, transferList) {
-    return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data);
+  register(originalSpecifier, parentURL, data, transferList, isInternal) {
+    return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data, isInternal);
   }
 
   /**
diff --git a/lib/internal/test_runner/mock/loader.js b/lib/internal/test_runner/mock/loader.js
@@ -26,12 +26,6 @@ const { createRequire, isBuiltin } = require('module');
 const { defaultGetFormatWithoutErrors } = require('internal/modules/esm/get_format');
 const { defaultResolve } = require('internal/modules/esm/resolve');
 
-// TODO(cjihrig): This file should not be exposed publicly, but register() does
-// not handle internal loaders. Before marking this API as stable, one of the
-// following issues needs to be implemented:
-// https://github.com/nodejs/node/issues/49473
-// or https://github.com/nodejs/node/issues/52219
-
 // TODO(cjihrig): The mocks need to be thread aware because the exports are
 // evaluated on the thread that creates the mock. Before marking this API as
 // stable, one of the following issues needs to be implemented:
diff --git a/lib/internal/test_runner/mock/mock.js b/lib/internal/test_runner/mock/mock.js
@@ -53,9 +53,9 @@ const {
 } = require('internal/validators');
 const { MockTimers } = require('internal/test_runner/mock/mock_timers');
 const { strictEqual, notStrictEqual } = require('assert');
-const { isBuiltin, Module, register } = require('module');
+const { Module } = require('internal/modules/cjs/loader');
 const { MessageChannel } = require('worker_threads');
-const { _load, _nodeModulePaths, _resolveFilename } = Module;
+const { _load, _nodeModulePaths, _resolveFilename, isBuiltin } = Module;
 function kDefaultFunction() {}
 const enableModuleMocking = getOptionValue('--experimental-test-module-mocks');
 const kMockSearchParam = 'node-test-mock';
@@ -650,19 +650,22 @@ function setupSharedModuleState() {
     const { mock } = require('test');
     const mockExports = new SafeMap();
     const { port1, port2 } = new MessageChannel();
-
-    register('node:test/mock_loader', {
-      __proto__: null,
-      data: { __proto__: null, port: port2 },
-      transferList: [port2],
-    });
+    const moduleLoader = esmLoader.getOrInitializeCascadedLoader();
+
+    moduleLoader.register(
+      'internal/test_runner/mock/loader',
+      'node:',
+      { __proto__: null, port: port2 },
+      [port2],
+      true,
+    );
 
     sharedModuleState = {
       __proto__: null,
       loaderPort: port1,
       mockExports,
       mockMap: new SafeMap(),
-      moduleLoader: esmLoader.getOrInitializeCascadedLoader(),
+      moduleLoader,
     };
     mock._mockExports = mockExports;
     Module._load = FunctionPrototypeBind(cjsMockModuleLoad, sharedModuleState);
