module: use synchronous hooks for preparsing in import(cjs)

PR-URL: #55698 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
nodejs · Dec 10, 2024 · 8e780bc · 8e780bc
1 parent e5bb6c2
commit 8e780bc
Showing 1 changed file with 34 additions and 31 deletions.
diff --git 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);
         }