From d0f5943363fd6b7dd78cfd288b59af7727d79ce7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 13 Aug 2024 15:17:50 +0200 Subject: [PATCH] test_runner: do not expose internal loader PR-URL: https://github.com/nodejs/node/pull/54106 Fixes: https://github.com/nodejs/node/issues/54071 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow --- lib/internal/modules/esm/hooks.js | 14 ++++++++------ lib/internal/modules/esm/loader.js | 9 +++++---- .../test_runner/mock/loader.js} | 6 ------ lib/internal/test_runner/mock/mock.js | 19 +++++++++++-------- 4 files changed, 24 insertions(+), 24 deletions(-) rename lib/{test/mock_loader.js => internal/test_runner/mock/loader.js} (95%) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 206db0b3c67..5984c707f16 100644 --- 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 index e0d39808332..1fac67191b8 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -491,7 +491,7 @@ 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 @@ -499,7 +499,7 @@ class ModuleLoader { // 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/test/mock_loader.js b/lib/internal/test_runner/mock/loader.js similarity index 95% rename from lib/test/mock_loader.js rename to lib/internal/test_runner/mock/loader.js index 6393ee1182c..a434248bcce 100644 --- a/lib/test/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 index e4928784792..828f3e513f8 100644 --- 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(); + const moduleLoader = esmLoader.getOrInitializeCascadedLoader(); - register('node:test/mock_loader', { - __proto__: null, - data: { __proto__: null, port: port2 }, - transferList: [port2], - }); + 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);