From 1215a8bf124603a906c7bf748dee4f7f22cc6dd7 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 2 Nov 2024 09:42:29 +0100 Subject: [PATCH] module: use synchronous hooks for preparsing in import(cjs) PR-URL: https://github.com/nodejs/node/pull/55698 Reviewed-By: Geoffrey Booth Reviewed-By: Chengzhong Wu Reviewed-By: Guy Bedford --- lib/internal/modules/esm/translators.js | 65 +++++++++++++------------ 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index a9a3234befe10f..5b2a865582e5cd 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -26,7 +26,7 @@ const { const { BuiltinModule } = require('internal/bootstrap/realm'); const assert = require('internal/assert'); const { readFileSync } = require('fs'); -const { dirname, extname, isAbsolute } = require('path'); +const { dirname, extname } = require('path'); const { assertBufferSource, loadBuiltinModule, @@ -42,6 +42,9 @@ const { kModuleSource, kModuleExport, kModuleExportNames, + findLongestRegisteredExtension, + resolveForCJSWithHooks, + loadSourceForCJSWithHooks, } = require('internal/modules/cjs/loader'); const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { @@ -171,17 +174,18 @@ const cjsCache = new SafeMap(); * @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 {typeof loadCJSModule} [loadCJS=loadCJSModule] - The function to load the CommonJS module. * @returns {ModuleWrap} The ModuleWrap object for the CommonJS module. */ -function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { +function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModule) { debug(`Translating CJSModule ${url}`); 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); + const { exportNames, module } = cjsPreparseModuleExports(filename, source, isMain, format); cjsCache.set(url, module); const wrapperNames = [...exportNames, 'module.exports']; @@ -228,7 +232,7 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) { initCJSParseSync(); - return createCJSModuleWrap(url, source, isMain, (module, source, url, filename, isMain) => { + return createCJSModuleWrap(url, source, isMain, 'commonjs', (module, source, url, filename, isMain) => { assert(module === CJSModule._cache[filename]); wrapModuleLoad(filename, null, isMain); }); @@ -240,7 +244,7 @@ translators.set('require-commonjs', (url, source, isMain) => { initCJSParseSync(); assert(cjsParse); - return createCJSModuleWrap(url, source); + return createCJSModuleWrap(url, source, isMain, 'commonjs'); }); // Handle CommonJS modules referenced by `require` calls. @@ -249,7 +253,7 @@ translators.set('require-commonjs-typescript', (url, source, isMain) => { emitExperimentalWarning('Type Stripping'); assert(cjsParse); const code = stripTypeScriptModuleTypes(stringify(source), url); - return createCJSModuleWrap(url, code); + return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript'); }); // Handle CommonJS modules referenced by `import` statements or expressions, @@ -273,16 +277,17 @@ translators.set('commonjs', function commonjsStrategy(url, source, isMain) { } catch { // Continue regardless of error. } - return createCJSModuleWrap(url, source, isMain, cjsLoader); + return createCJSModuleWrap(url, source, isMain, 'commonjs', cjsLoader); }); /** * Pre-parses a CommonJS module's exports and re-exports. * @param {string} filename - The filename of the module. * @param {string} [source] - The source code of the module. + * @param {boolean} isMain - Whether it is pre-parsing for the entry point. + * @param {string} format */ -function cjsPreparseModuleExports(filename, source) { - // TODO: Do we want to keep hitting the user mutable CJS loader here? +function cjsPreparseModuleExports(filename, source, isMain, format) { let module = CJSModule._cache[filename]; if (module && module[kModuleExportNames] !== undefined) { return { module, exportNames: module[kModuleExportNames] }; @@ -293,10 +298,15 @@ function cjsPreparseModuleExports(filename, source) { module.filename = filename; module.paths = CJSModule._nodeModulePaths(module.path); module[kIsCachedByESMLoader] = true; - module[kModuleSource] = source; CJSModule._cache[filename] = module; } + if (source === undefined) { + ({ source } = loadSourceForCJSWithHooks(module, filename, format)); + } + module[kModuleSource] = source; + + debug(`Preparsing exports of ${filename}`); let exports, reexports; try { ({ exports, reexports } = cjsParse(source || '')); @@ -310,34 +320,27 @@ function cjsPreparseModuleExports(filename, source) { // Set first for cycles. module[kModuleExportNames] = exportNames; + // If there are any re-exports e.g. `module.exports = { ...require(...) }`, + // pre-parse the dependencies to find transitively exported names. if (reexports.length) { - module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); + module.filename ??= filename; + module.paths ??= CJSModule._nodeModulePaths(dirname(filename)); + for (let i = 0; i < reexports.length; i++) { + debug(`Preparsing re-exports of '${filename}'`); const reexport = reexports[i]; let resolved; + let format; try { - // TODO: this should be calling the `resolve` hook chain instead. - // Doing so would mean dropping support for CJS in the loader thread, as - // this call needs to be sync from the perspective of the main thread, - // which we can do via HooksProxy and Atomics, but we can't do within - // the loaders thread. Until this is done, the lexer will use the - // monkey-patchable CJS loader to get the path to the module file to - // load (which may or may not be aligned with the URL that the `resolve` - // hook have returned). - resolved = CJSModule._resolveFilename(reexport, module); - } catch { + ({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false)); + } catch (e) { + debug(`Failed to resolve '${reexport}', skipping`, e); continue; } - // TODO: this should be calling the `load` hook chain and check if it returns - // `format: 'commonjs'` instead of relying on file extensions. - const ext = extname(resolved); - if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) && - isAbsolute(resolved)) { - // TODO: this should be calling the `load` hook chain to get the source - // (and fallback to reading the FS only if the source is nullish). - const source = readFileSync(resolved, 'utf-8'); - const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved, source); + + if (format === 'commonjs' || + (!BuiltinModule.normalizeRequirableId(resolved) && findLongestRegisteredExtension(resolved) === '.js')) { + const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved, undefined, false, format); for (const name of reexportNames) { exportNames.add(name); }