Skip to content

Commit fe793a6

Browse files
aduh95targos
authored andcommitted
test_runner: do not expose internal loader
PR-URL: #54106 Fixes: #54071 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 2be78b0 commit fe793a6

File tree

4 files changed

+25
-25
lines changed

4 files changed

+25
-25
lines changed

Diff for: lib/internal/modules/esm/hooks.js

+8-6
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,15 @@ class Hooks {
145145
* @param {any} [data] Arbitrary data to be passed from the custom
146146
* loader (user-land) to the worker.
147147
*/
148-
async register(urlOrSpecifier, parentURL, data) {
148+
async register(urlOrSpecifier, parentURL, data, isInternal) {
149149
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
150-
const keyedExports = await cascadedLoader.import(
151-
urlOrSpecifier,
152-
parentURL,
153-
kEmptyObject,
154-
);
150+
const keyedExports = isInternal ?
151+
require(urlOrSpecifier) :
152+
await cascadedLoader.import(
153+
urlOrSpecifier,
154+
parentURL,
155+
kEmptyObject,
156+
);
155157
await this.addCustomLoader(urlOrSpecifier, keyedExports, data);
156158
}
157159

Diff for: lib/internal/modules/esm/loader.js

+5-4
Original file line numberDiff line numberDiff line change
@@ -491,15 +491,15 @@ class ModuleLoader {
491491
/**
492492
* @see {@link CustomizedModuleLoader.register}
493493
*/
494-
register(specifier, parentURL, data, transferList) {
494+
register(specifier, parentURL, data, transferList, isInternal) {
495495
if (!this.#customizations) {
496496
// `CustomizedModuleLoader` is defined at the bottom of this file and
497497
// available well before this line is ever invoked. This is here in
498498
// order to preserve the git diff instead of moving the class.
499499
// eslint-disable-next-line no-use-before-define
500500
this.setCustomizations(new CustomizedModuleLoader());
501501
}
502-
return this.#customizations.register(`${specifier}`, `${parentURL}`, data, transferList);
502+
return this.#customizations.register(`${specifier}`, `${parentURL}`, data, transferList, isInternal);
503503
}
504504

505505
/**
@@ -636,10 +636,11 @@ class CustomizedModuleLoader {
636636
* @param {any} [data] Arbitrary data to be passed from the custom loader
637637
* (user-land) to the worker.
638638
* @param {any[]} [transferList] Objects in `data` that are changing ownership
639+
* @param {boolean} [isInternal] For internal loaders that should not be publicly exposed.
639640
* @returns {{ format: string, url: URL['href'] }}
640641
*/
641-
register(originalSpecifier, parentURL, data, transferList) {
642-
return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data);
642+
register(originalSpecifier, parentURL, data, transferList, isInternal) {
643+
return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data, isInternal);
643644
}
644645

645646
/**

Diff for: lib/test/mock_loader.js renamed to lib/internal/test_runner/mock/loader.js

-6
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,6 @@ const { createRequire, isBuiltin } = require('module');
2626
const { defaultGetFormatWithoutErrors } = require('internal/modules/esm/get_format');
2727
const { defaultResolve } = require('internal/modules/esm/resolve');
2828

29-
// TODO(cjihrig): This file should not be exposed publicly, but register() does
30-
// not handle internal loaders. Before marking this API as stable, one of the
31-
// following issues needs to be implemented:
32-
// https://github.com/nodejs/node/issues/49473
33-
// or https://github.com/nodejs/node/issues/52219
34-
3529
// TODO(cjihrig): The mocks need to be thread aware because the exports are
3630
// evaluated on the thread that creates the mock. Before marking this API as
3731
// stable, one of the following issues needs to be implemented:

Diff for: lib/internal/test_runner/mock/mock.js

+12-9
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ const {
5353
} = require('internal/validators');
5454
const { MockTimers } = require('internal/test_runner/mock/mock_timers');
5555
const { strictEqual, notStrictEqual } = require('assert');
56-
const { isBuiltin, Module, register } = require('module');
56+
const { Module } = require('internal/modules/cjs/loader');
5757
const { MessageChannel } = require('worker_threads');
58-
const { _load, _nodeModulePaths, _resolveFilename } = Module;
58+
const { _load, _nodeModulePaths, _resolveFilename, isBuiltin } = Module;
5959
function kDefaultFunction() {}
6060
const enableModuleMocking = getOptionValue('--experimental-test-module-mocks');
6161
const kMockSearchParam = 'node-test-mock';
@@ -650,19 +650,22 @@ function setupSharedModuleState() {
650650
const { mock } = require('test');
651651
const mockExports = new SafeMap();
652652
const { port1, port2 } = new MessageChannel();
653-
654-
register('node:test/mock_loader', {
655-
__proto__: null,
656-
data: { __proto__: null, port: port2 },
657-
transferList: [port2],
658-
});
653+
const moduleLoader = esmLoader.getOrInitializeCascadedLoader();
654+
655+
moduleLoader.register(
656+
'internal/test_runner/mock/loader',
657+
'node:',
658+
{ __proto__: null, port: port2 },
659+
[port2],
660+
true,
661+
);
659662

660663
sharedModuleState = {
661664
__proto__: null,
662665
loaderPort: port1,
663666
mockExports,
664667
mockMap: new SafeMap(),
665-
moduleLoader: esmLoader.getOrInitializeCascadedLoader(),
668+
moduleLoader,
666669
};
667670
mock._mockExports = mockExports;
668671
Module._load = FunctionPrototypeBind(cjsMockModuleLoad, sharedModuleState);

0 commit comments

Comments
 (0)