From 170848bc1831ea9be665b527e2131c81263dfd2c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 14 Oct 2025 18:28:22 +0200 Subject: [PATCH] module: handle null source from async loader hooks in sync hooks This relaxes the validation in sync hooks so that it accepts the quirky nullish source returned by the default step of the async loader when the module being loaded is CommonJS. When there are no customization hooks registered, a saner synchronous default load step is used to use a property instead of a reset nullish source to signify that the module should go through the CJS monkey patching routes and reduce excessive reloading from disk. PR-URL: https://github.com/nodejs/node/pull/59929 Fixes: https://github.com/nodejs/node/issues/59384 Fixes: https://github.com/nodejs/node/issues/57327 Refs: https://github.com/nodejs/node/issues/59666 Refs: https://github.com/dygabo/load_module_test Reviewed-By: Geoffrey Booth Reviewed-By: Jacob Smith --- lib/internal/modules/cjs/loader.js | 8 +- lib/internal/modules/customization_hooks.js | 44 +++++-- lib/internal/modules/esm/hooks.js | 2 +- lib/internal/modules/esm/load.js | 24 +++- lib/internal/modules/esm/loader.js | 61 +++++---- lib/internal/modules/esm/translators.js | 123 +++++++++--------- .../module-hooks/sync-and-async/app.js | 2 + .../sync-and-async/async-customize-loader.js | 10 ++ .../sync-and-async/async-customize.js | 3 + .../sync-and-async/async-forward-loader.js | 3 + .../sync-and-async/async-forward.js | 3 + .../sync-and-async/sync-customize.js | 14 ++ .../sync-and-async/sync-forward.js | 7 + .../test-module-hooks-load-async-and-sync.js | 32 +++++ 14 files changed, 228 insertions(+), 108 deletions(-) create mode 100644 test/fixtures/module-hooks/sync-and-async/app.js create mode 100644 test/fixtures/module-hooks/sync-and-async/async-customize-loader.js create mode 100644 test/fixtures/module-hooks/sync-and-async/async-customize.js create mode 100644 test/fixtures/module-hooks/sync-and-async/async-forward-loader.js create mode 100644 test/fixtures/module-hooks/sync-and-async/async-forward.js create mode 100644 test/fixtures/module-hooks/sync-and-async/sync-customize.js create mode 100644 test/fixtures/module-hooks/sync-and-async/sync-forward.js create mode 100644 test/module-hooks/test-module-hooks-load-async-and-sync.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 57470fb90d..8bd89305e2 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -177,6 +177,7 @@ const { registerHooks, resolveHooks, resolveWithHooks, + validateLoadStrict, } = require('internal/modules/customization_hooks'); const { stripTypeScriptModuleTypes } = require('internal/modules/typescript'); const packageJsonReader = require('internal/modules/package_json_reader'); @@ -1175,7 +1176,7 @@ function loadBuiltinWithHooks(id, url, format) { url ??= `node:${id}`; // TODO(joyeecheung): do we really want to invoke the load hook for the builtins? const loadResult = loadWithHooks(url, format || 'builtin', /* importAttributes */ undefined, - getCjsConditionsArray(), getDefaultLoad(url, id)); + getCjsConditionsArray(), getDefaultLoad(url, id), validateLoadStrict); if (loadResult.format && loadResult.format !== 'builtin') { return undefined; // Format has been overridden, return undefined for the caller to continue loading. } @@ -1791,10 +1792,9 @@ function loadSource(mod, filename, formatFromNode) { mod[kURL] = convertCJSFilenameToURL(filename); } + const defaultLoad = getDefaultLoad(mod[kURL], filename); const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined, - getCjsConditionsArray(), - getDefaultLoad(mod[kURL], filename)); - + getCjsConditionsArray(), defaultLoad, validateLoadStrict); // Reset the module properties with load hook results. if (loadResult.format !== undefined) { mod[kFormat] = loadResult.format; diff --git a/lib/internal/modules/customization_hooks.js b/lib/internal/modules/customization_hooks.js index 580fc05467..c2579269ec 100644 --- a/lib/internal/modules/customization_hooks.js +++ b/lib/internal/modules/customization_hooks.js @@ -262,13 +262,25 @@ function validateResolve(specifier, context, result) { */ /** - * Validate the result returned by a chain of resolve hook. + * Validate the result returned by a chain of load hook. * @param {string} url URL passed into the hooks. * @param {ModuleLoadContext} context Context passed into the hooks. * @param {ModuleLoadResult} result Result produced by load hooks. * @returns {ModuleLoadResult} */ -function validateLoad(url, context, result) { +function validateLoadStrict(url, context, result) { + validateSourceStrict(url, context, result); + validateFormat(url, context, result); + return result; +} + +function validateLoadSloppy(url, context, result) { + validateSourcePermissive(url, context, result); + validateFormat(url, context, result); + return result; +} + +function validateSourceStrict(url, context, result) { const { source, format } = result; // To align with module.register(), the load hooks are still invoked for // the builtins even though the default load step only provides null as source, @@ -276,7 +288,8 @@ function validateLoad(url, context, result) { if (!StringPrototypeStartsWith(url, 'node:') && typeof result.source !== 'string' && !isAnyArrayBuffer(source) && - !isArrayBufferView(source)) { + !isArrayBufferView(source) && + format !== 'addon') { throw new ERR_INVALID_RETURN_PROPERTY_VALUE( 'a string, an ArrayBuffer, or a TypedArray', 'load', @@ -284,7 +297,21 @@ function validateLoad(url, context, result) { source, ); } +} +function validateSourcePermissive(url, context, result) { + const { source, format } = result; + if (format === 'commonjs' && source == null) { + // Accommodate the quirk in defaultLoad used by asynchronous loader hooks + // which sets source to null for commonjs. + // See: https://github.com/nodejs/node/issues/57327#issuecomment-2701382020 + return; + } + validateSourceStrict(url, context, result); +} + +function validateFormat(url, context, result) { + const { format } = result; if (typeof format !== 'string' && format !== undefined) { throw new ERR_INVALID_RETURN_PROPERTY_VALUE( 'a string', @@ -293,12 +320,6 @@ function validateLoad(url, context, result) { format, ); } - - return { - __proto__: null, - format, - source, - }; } class ModuleResolveContext { @@ -338,9 +359,10 @@ let decoder; * @param {ImportAttributes|undefined} importAttributes * @param {string[]} conditions * @param {(url: string, context: ModuleLoadContext) => ModuleLoadResult} defaultLoad + * @param {(url: string, context: ModuleLoadContext, result: ModuleLoadResult) => ModuleLoadResult} validateLoad * @returns {ModuleLoadResult} */ -function loadWithHooks(url, originalFormat, importAttributes, conditions, defaultLoad) { +function loadWithHooks(url, originalFormat, importAttributes, conditions, defaultLoad, validateLoad) { debug('loadWithHooks', url, originalFormat); const context = new ModuleLoadContext(originalFormat, importAttributes, conditions); if (loadHooks.length === 0) { @@ -403,4 +425,6 @@ module.exports = { registerHooks, resolveHooks, resolveWithHooks, + validateLoadStrict, + validateLoadSloppy, }; diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 6e16d75d58..e3ba5fa862 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -61,7 +61,7 @@ const { SHARED_MEMORY_BYTE_LENGTH, WORKER_TO_MAIN_THREAD_NOTIFICATION, } = require('internal/modules/esm/shared_constants'); -let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { +let debug = require('internal/util/debuglog').debuglog('async_loader_worker', (fn) => { debug = fn; }); let importMetaInitializer; diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 8414d303c0..c284163fba 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -141,15 +141,26 @@ function defaultLoadSync(url, context = kEmptyObject) { throwIfUnsupportedURLScheme(urlInstance, false); + let shouldBeReloadedByCJSLoader = false; if (urlInstance.protocol === 'node:') { source = null; - } else if (source == null) { - ({ responseURL, source } = getSourceSync(urlInstance, context)); - context.source = source; + format ??= 'builtin'; + } else if (format === 'addon') { + // Skip loading addon file content. It must be loaded with dlopen from file system. + source = null; + } else { + if (source == null) { + ({ responseURL, source } = getSourceSync(urlInstance, context)); + context = { __proto__: context, source }; + } + + // Now that we have the source for the module, run `defaultGetFormat` to detect its format. + format ??= defaultGetFormat(urlInstance, context); + + // For backward compatibility reasons, we need to let go through Module._load + // again. + shouldBeReloadedByCJSLoader = (format === 'commonjs'); } - - format ??= defaultGetFormat(urlInstance, context); - validateAttributes(url, format, importAttributes); return { @@ -157,6 +168,7 @@ function defaultLoadSync(url, context = kEmptyObject) { format, responseURL, source, + shouldBeReloadedByCJSLoader, }; } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 300da51afe..9cb02f05c8 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -55,8 +55,9 @@ const { resolveWithHooks, loadHooks, loadWithHooks, + validateLoadSloppy, } = require('internal/modules/customization_hooks'); -let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer; +let defaultResolve, defaultLoadSync, importMetaInitializer; const { tracingChannel } = require('diagnostics_channel'); const onImport = tracingChannel('module.import'); @@ -146,6 +147,10 @@ let hooksProxy; * @typedef {ArrayBuffer|TypedArray|string} ModuleSource */ +/** + * @typedef {{ format: ModuleFormat, source: ModuleSource, translatorKey: string }} TranslateContext + */ + /** * This class covers the base machinery of module loading. To add custom * behavior you can pass a customizations object and this object will be @@ -503,18 +508,19 @@ class ModuleLoader { const loadResult = this.#loadSync(url, { format, importAttributes }); + const formatFromLoad = loadResult.format; // Use the synchronous commonjs translator which can deal with cycles. - const finalFormat = - loadResult.format === 'commonjs' || - loadResult.format === 'commonjs-typescript' ? 'commonjs-sync' : loadResult.format; + const translatorKey = (formatFromLoad === 'commonjs' || formatFromLoad === 'commonjs-typescript') ? + 'commonjs-sync' : formatFromLoad; - if (finalFormat === 'wasm') { + if (translatorKey === 'wasm') { assert.fail('WASM is currently unsupported by require(esm)'); } const { source } = loadResult; const isMain = (parentURL === undefined); - const wrap = this.#translate(url, finalFormat, source, parentURL); + const translateContext = { format: formatFromLoad, source, translatorKey, __proto__: null }; + const wrap = this.#translate(url, translateContext, parentURL); assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`); if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) { @@ -523,7 +529,7 @@ class ModuleLoader { const cjsModule = wrap[imported_cjs_symbol]; if (cjsModule) { - assert(finalFormat === 'commonjs-sync'); + assert(translatorKey === 'commonjs-sync'); // Check if the ESM initiating import CJS is being required by the same CJS module. if (cjsModule?.[kIsExecuting]) { const parentFilename = urlToFilename(parentURL); @@ -547,22 +553,22 @@ class ModuleLoader { * Translate a loaded module source into a ModuleWrap. This is run synchronously, * but the translator may return the ModuleWrap in a Promise. * @param {string} url URL of the module to be translated. - * @param {string} format Format of the module to be translated. This is used to find - * matching translators. - * @param {ModuleSource} source Source of the module to be translated. - * @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point. + * @param {TranslateContext} translateContext Context for the translator + * @param {string|undefined} parentURL URL of the module initiating the module loading for the first time. + * Undefined if it's the entry point. * @returns {ModuleWrap} */ - #translate(url, format, source, parentURL) { + #translate(url, translateContext, parentURL) { + const { translatorKey, format } = translateContext; this.validateLoadResult(url, format); - const translator = getTranslators().get(format); + const translator = getTranslators().get(translatorKey); if (!translator) { - throw new ERR_UNKNOWN_MODULE_FORMAT(format, url); + throw new ERR_UNKNOWN_MODULE_FORMAT(translatorKey, url); } - const result = FunctionPrototypeCall(translator, this, url, source, parentURL === undefined); - assert(result instanceof ModuleWrap); + const result = FunctionPrototypeCall(translator, this, url, translateContext, parentURL); + assert(result instanceof ModuleWrap, `The ${format} module returned is not a ModuleWrap`); return result; } @@ -575,7 +581,8 @@ class ModuleLoader { * @returns {ModuleWrap} */ loadAndTranslateForRequireInImportedCJS(url, loadContext, parentURL) { - const { format: formatFromLoad, source } = this.#loadSync(url, loadContext); + const loadResult = this.#loadSync(url, loadContext); + const formatFromLoad = loadResult.format; if (formatFromLoad === 'wasm') { // require(wasm) is not supported. throw new ERR_UNKNOWN_MODULE_FORMAT(formatFromLoad, url); @@ -587,15 +594,16 @@ class ModuleLoader { } } - let finalFormat = formatFromLoad; + let translatorKey = formatFromLoad; if (formatFromLoad === 'commonjs') { - finalFormat = 'require-commonjs'; + translatorKey = 'require-commonjs'; } if (formatFromLoad === 'commonjs-typescript') { - finalFormat = 'require-commonjs-typescript'; + translatorKey = 'require-commonjs-typescript'; } - const wrap = this.#translate(url, finalFormat, source, parentURL); + const translateContext = { ...loadResult, translatorKey, __proto__: null }; + const wrap = this.#translate(url, translateContext, parentURL); assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`); return wrap; } @@ -610,8 +618,9 @@ class ModuleLoader { */ loadAndTranslate(url, loadContext, parentURL) { const maybePromise = this.load(url, loadContext); - const afterLoad = ({ format, source }) => { - return this.#translate(url, format, source, parentURL); + const afterLoad = (loadResult) => { + const translateContext = { ...loadResult, translatorKey: loadResult.format, __proto__: null }; + return this.#translate(url, translateContext, parentURL); }; if (isPromise(maybePromise)) { return maybePromise.then(afterLoad); @@ -837,8 +846,8 @@ class ModuleLoader { return this.#customizations.load(url, context); } - defaultLoad ??= require('internal/modules/esm/load').defaultLoad; - return defaultLoad(url, context); + defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync; + return defaultLoadSync(url, context); } /** @@ -873,7 +882,7 @@ class ModuleLoader { // TODO(joyeecheung): construct the ModuleLoadContext in the loaders directly instead // of converting them from plain objects in the hooks. return loadWithHooks(url, context.format, context.importAttributes, this.#defaultConditions, - this.#loadAndMaybeBlockOnLoaderThread.bind(this)); + this.#loadAndMaybeBlockOnLoaderThread.bind(this), validateLoadSloppy); } return this.#loadAndMaybeBlockOnLoaderThread(url, context); } diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 147d96bda8..446349113e 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -100,10 +100,12 @@ function errPath(url) { } // Strategy for loading a standard JavaScript module. -translators.set('module', function moduleStrategy(url, source, isMain) { +translators.set('module', function moduleStrategy(url, translateContext, parentURL) { + let { source } = translateContext; + const isMain = (parentURL === undefined); assertBufferSource(source, true, 'load'); source = stringify(source); - debug(`Translating StandardModule ${url}`); + debug(`Translating StandardModule ${url}`, translateContext); const { compileSourceTextModule } = require('internal/modules/esm/utils'); const context = isMain ? { isMain } : undefined; const module = compileSourceTextModule(url, source, this, context); @@ -199,20 +201,23 @@ const cjsCache = new SafeMap(); /** * Creates a ModuleWrap object for a CommonJS module. * @param {string} url - The URL of the module. - * @param {string} source - The source code of the module. - * @param {boolean} isMain - Whether the module is the main module. - * @param {string} format - Format of the module. + * @param {{ format: ModuleFormat, source: ModuleSource }} translateContext Context for the translator + * @param {string|undefined} parentURL URL of the module initiating the module loading for the first time. + * Undefined if it's the entry point. * @param {typeof loadCJSModule} [loadCJS] - The function to load the CommonJS module. * @returns {ModuleWrap} The ModuleWrap object for the CommonJS module. */ -function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModule) { - debug(`Translating CJSModule ${url}`); +function createCJSModuleWrap(url, translateContext, parentURL, loadCJS = loadCJSModule) { + debug(`Translating CJSModule ${url}`, translateContext); + const { format: sourceFormat } = translateContext; + let { source } = translateContext; + const isMain = (parentURL === undefined); const filename = urlToFilename(url); // In case the source was not provided by the `load` step, we need fetch it now. source = stringify(source ?? getSource(new URL(url)).source); - const { exportNames, module } = cjsPreparseModuleExports(filename, source, format); + const { exportNames, module } = cjsPreparseModuleExports(filename, source, sourceFormat); cjsCache.set(url, module); const wrapperNames = [...exportNames]; @@ -263,11 +268,12 @@ function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModul /** * Creates a ModuleWrap object for a CommonJS module without source texts. * @param {string} url - The URL of the module. - * @param {boolean} isMain - Whether the module is the main module. + * @param {string|undefined} parentURL - URL of the parent module, if any. * @returns {ModuleWrap} The ModuleWrap object for the CommonJS module. */ -function createCJSNoSourceModuleWrap(url, isMain) { +function createCJSNoSourceModuleWrap(url, parentURL) { debug(`Translating CJSModule without source ${url}`); + const isMain = (parentURL === undefined); const filename = urlToFilename(url); @@ -301,54 +307,60 @@ function createCJSNoSourceModuleWrap(url, isMain) { }, module); } -translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) { +translators.set('commonjs-sync', function requireCommonJS(url, translateContext, parentURL) { initCJSParseSync(); - return createCJSModuleWrap(url, source, isMain, 'commonjs', (module, source, url, filename, isMain) => { - assert(module === CJSModule._cache[filename]); - wrapModuleLoad(filename, null, isMain); - }); + return createCJSModuleWrap(url, translateContext, parentURL, loadCJSModuleWithModuleLoad); }); // Handle CommonJS modules referenced by `require` calls. // This translator function must be sync, as `require` is sync. -translators.set('require-commonjs', (url, source, isMain) => { +translators.set('require-commonjs', (url, translateContext, parentURL) => { initCJSParseSync(); assert(cjsParse); - return createCJSModuleWrap(url, source, isMain, 'commonjs'); + return createCJSModuleWrap(url, translateContext, parentURL); }); // Handle CommonJS modules referenced by `require` calls. // This translator function must be sync, as `require` is sync. -translators.set('require-commonjs-typescript', (url, source, isMain) => { +translators.set('require-commonjs-typescript', (url, translateContext, parentURL) => { assert(cjsParse); - const code = stripTypeScriptModuleTypes(stringify(source), url); - return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript'); + translateContext.source = stripTypeScriptModuleTypes(stringify(translateContext.source), url); + return createCJSModuleWrap(url, translateContext, parentURL); }); +// This goes through Module._load to accommodate monkey-patchers. +function loadCJSModuleWithModuleLoad(module, source, url, filename, isMain) { + assert(module === CJSModule._cache[filename]); + wrapModuleLoad(filename, undefined, isMain); +} + // Handle CommonJS modules referenced by `import` statements or expressions, // or as the initial entry point when the ESM loader handles a CommonJS entry. -translators.set('commonjs', function commonjsStrategy(url, source, isMain) { +translators.set('commonjs', function commonjsStrategy(url, translateContext, parentURL) { if (!cjsParse) { initCJSParseSync(); } // For backward-compatibility, it's possible to return a nullish value for - // CJS source associated with a file: URL. In this case, the source is - // obtained by calling the monkey-patchable CJS loader. - const cjsLoader = source == null ? (module, source, url, filename, isMain) => { - assert(module === CJSModule._cache[filename]); - wrapModuleLoad(filename, undefined, isMain); - } : loadCJSModule; + // CJS source associated with a `file:` URL - that usually means the source is not + // customized (is loaded by default load) or the hook author wants it to be reloaded + // through CJS routine. In this case, the source is obtained by calling the + // monkey-patchable CJS loader. + // TODO(joyeecheung): just use wrapModuleLoad and let the CJS loader + // invoke the off-thread hooks. Use a special parent to avoid invoking in-thread + // hooks twice. + const shouldReloadByCJSLoader = (translateContext.shouldBeReloadedByCJSLoader || translateContext.source == null); + const cjsLoader = shouldReloadByCJSLoader ? loadCJSModuleWithModuleLoad : loadCJSModule; try { // We still need to read the FS to detect the exports. - source ??= readFileSync(new URL(url), 'utf8'); + translateContext.source ??= readFileSync(new URL(url), 'utf8'); } catch { // Continue regardless of error. } - return createCJSModuleWrap(url, source, isMain, 'commonjs', cjsLoader); + return createCJSModuleWrap(url, translateContext, parentURL, cjsLoader); }); /** @@ -373,22 +385,6 @@ function cjsEmplaceModuleCacheEntry(filename, parent) { return cjsMod; } -/** - * Emplace a CJS module cache entry for the given URL. - * @param {string} url The module URL - * @param {CJSModule} parent The parent CJS module - * @returns {CJSModule|undefined} the cached CJS module entry, undefined if url cannot be used to identify a CJS entry. - */ -exports.cjsEmplaceModuleCacheEntryForURL = function cjsEmplaceModuleCacheEntryForURL(url, parent) { - const filename = urlToFilename(url); - if (!filename) { - return; - } - const cjsModule = cjsEmplaceModuleCacheEntry(filename, parent); - cjsCache.set(url, cjsModule); - return cjsModule; -}; - /** * Pre-parses a CommonJS module's exports and re-exports. * @param {string} filename - The filename of the module. @@ -454,8 +450,8 @@ function cjsPreparseModuleExports(filename, source, format) { // Strategy for loading a node builtin CommonJS module that isn't // through normal resolution -translators.set('builtin', function builtinStrategy(url) { - debug(`Translating BuiltinModule ${url}`); +translators.set('builtin', function builtinStrategy(url, translateContext) { + debug(`Translating BuiltinModule ${url}`, translateContext); // Slice 'node:' scheme const id = StringPrototypeSlice(url, 5); const module = loadBuiltinModule(id, url); @@ -468,7 +464,8 @@ translators.set('builtin', function builtinStrategy(url) { }); // Strategy for loading a JSON file -translators.set('json', function jsonStrategy(url, source) { +translators.set('json', function jsonStrategy(url, translateContext) { + let { source } = translateContext; assertBufferSource(source, true, 'load'); debug(`Loading JSONModule ${url}`); const pathname = StringPrototypeStartsWith(url, 'file:') ? @@ -536,10 +533,11 @@ translators.set('json', function jsonStrategy(url, source) { * >} [[Instance]] slot proxy for WebAssembly Module Record */ const wasmInstances = new SafeWeakMap(); -translators.set('wasm', function(url, source) { +translators.set('wasm', function(url, translateContext) { + const { source } = translateContext; assertBufferSource(source, false, 'load'); - debug(`Translating WASMModule ${url}`); + debug(`Translating WASMModule ${url}`, translateContext); let compiled; try { @@ -626,9 +624,10 @@ translators.set('wasm', function(url, source) { }); // Strategy for loading a addon -translators.set('addon', function translateAddon(url, source, isMain) { +translators.set('addon', function translateAddon(url, translateContext, parentURL) { emitExperimentalWarning('Importing addons'); + const { source } = translateContext; // The addon must be loaded from file system with dlopen. Assert // the source is null. if (source !== null) { @@ -639,23 +638,25 @@ translators.set('addon', function translateAddon(url, source, isMain) { source); } - debug(`Translating addon ${url}`); + debug(`Translating addon ${url}`, translateContext); - return createCJSNoSourceModuleWrap(url, isMain); + return createCJSNoSourceModuleWrap(url, parentURL); }); // Strategy for loading a commonjs TypeScript module -translators.set('commonjs-typescript', function(url, source, isMain) { +translators.set('commonjs-typescript', function(url, translateContext, parentURL) { + const { source } = translateContext; assertBufferSource(source, true, 'load'); - const code = stripTypeScriptModuleTypes(stringify(source), url); - debug(`Translating TypeScript ${url}`); - return FunctionPrototypeCall(translators.get('commonjs'), this, url, code, isMain); + debug(`Translating TypeScript ${url}`, translateContext); + translateContext.source = stripTypeScriptModuleTypes(stringify(source), url); + return FunctionPrototypeCall(translators.get('commonjs'), this, url, translateContext, parentURL); }); // Strategy for loading an esm TypeScript module -translators.set('module-typescript', function(url, source, isMain) { +translators.set('module-typescript', function(url, translateContext, parentURL) { + const { source } = translateContext; assertBufferSource(source, true, 'load'); - const code = stripTypeScriptModuleTypes(stringify(source), url); - debug(`Translating TypeScript ${url}`); - return FunctionPrototypeCall(translators.get('module'), this, url, code, isMain); + debug(`Translating TypeScript ${url}`, translateContext); + translateContext.source = stripTypeScriptModuleTypes(stringify(source), url); + return FunctionPrototypeCall(translators.get('module'), this, url, translateContext, parentURL); }); diff --git a/test/fixtures/module-hooks/sync-and-async/app.js b/test/fixtures/module-hooks/sync-and-async/app.js new file mode 100644 index 0000000000..c14477b70f --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/app.js @@ -0,0 +1,2 @@ +console.log('Hello world'); + diff --git a/test/fixtures/module-hooks/sync-and-async/async-customize-loader.js b/test/fixtures/module-hooks/sync-and-async/async-customize-loader.js new file mode 100644 index 0000000000..ac961f0b97 --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/async-customize-loader.js @@ -0,0 +1,10 @@ +export async function load(url, context, nextLoad) { + if (url.endsWith('app.js')) { + return { + shortCircuit: true, + format: 'module', + source: 'console.log("customized by async hook");', + }; + } + return nextLoad(url, context); +} diff --git a/test/fixtures/module-hooks/sync-and-async/async-customize.js b/test/fixtures/module-hooks/sync-and-async/async-customize.js new file mode 100644 index 0000000000..78bf7fc556 --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/async-customize.js @@ -0,0 +1,3 @@ +import { register } from 'node:module'; + +register(new URL('async-customize-loader.js', import.meta.url)); diff --git a/test/fixtures/module-hooks/sync-and-async/async-forward-loader.js b/test/fixtures/module-hooks/sync-and-async/async-forward-loader.js new file mode 100644 index 0000000000..4a1ced7e8d --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/async-forward-loader.js @@ -0,0 +1,3 @@ +export async function load(url, context, nextLoad) { + return nextLoad(url, context); +} diff --git a/test/fixtures/module-hooks/sync-and-async/async-forward.js b/test/fixtures/module-hooks/sync-and-async/async-forward.js new file mode 100644 index 0000000000..fd10aa6df8 --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/async-forward.js @@ -0,0 +1,3 @@ +import { register } from 'node:module'; + +register(new URL('async-forward-loader.js', import.meta.url)); diff --git a/test/fixtures/module-hooks/sync-and-async/sync-customize.js b/test/fixtures/module-hooks/sync-and-async/sync-customize.js new file mode 100644 index 0000000000..90d63db1c7 --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/sync-customize.js @@ -0,0 +1,14 @@ +import { registerHooks } from 'node:module'; + +registerHooks({ + load(url, context, nextLoad) { + if (url.endsWith('app.js')) { + return { + shortCircuit: true, + format: 'module', + source: 'console.log("customized by sync hook")', + }; + } + return nextLoad(url, context); + }, +}); diff --git a/test/fixtures/module-hooks/sync-and-async/sync-forward.js b/test/fixtures/module-hooks/sync-and-async/sync-forward.js new file mode 100644 index 0000000000..2688a240dc --- /dev/null +++ b/test/fixtures/module-hooks/sync-and-async/sync-forward.js @@ -0,0 +1,7 @@ +import { registerHooks } from 'node:module'; + +registerHooks({ + load(url, context, nextLoad) { + return nextLoad(url, context); + }, +}); diff --git a/test/module-hooks/test-module-hooks-load-async-and-sync.js b/test/module-hooks/test-module-hooks-load-async-and-sync.js new file mode 100644 index 0000000000..75f4987942 --- /dev/null +++ b/test/module-hooks/test-module-hooks-load-async-and-sync.js @@ -0,0 +1,32 @@ +'use strict'; +// This tests that sync and async hooks can be mixed. + +require('../common'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); + +const app = fixtures.path('module-hooks', 'sync-and-async', 'app.js'); + +const testCases = [ + // When mixing sync and async hooks, the sync ones always run first. + { preload: ['sync-customize', 'async-customize'], stdout: 'customized by sync hook' }, + { preload: ['async-customize', 'sync-customize'], stdout: 'customized by sync hook' }, + // It should still work when neither hook does any customization. + { preload: ['sync-forward', 'async-forward'], stdout: 'Hello world' }, + { preload: ['async-forward', 'sync-forward'], stdout: 'Hello world' }, + // It should work when only one hook is customizing. + { preload: ['sync-customize', 'async-forward'], stdout: 'customized by sync hook' }, + { preload: ['async-customize', 'sync-forward'], stdout: 'customized by async hook' }, +]; + + +for (const { preload, stdout } of testCases) { + const importArgs = []; + for (const p of preload) { + importArgs.push('--import', fixtures.fileURL(`module-hooks/sync-and-async/${p}.js`)); + } + spawnSyncAndAssert(process.execPath, [...importArgs, app], { + stdout, + trim: true, + }); +}