module: move helpers out of cjs loader

PR-URL: #49912 Backport-PR-URL: #50669 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs · Nov 23, 2023 · 8c55f31 · 8c55f31
1 parent 9757989
commit 8c55f31
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 69 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -56,7 +56,6 @@ const {
   StringPrototypeCharAt,
   StringPrototypeCharCodeAt,
   StringPrototypeEndsWith,
-  StringPrototypeLastIndexOf,
   StringPrototypeIndexOf,
   StringPrototypeRepeat,
   StringPrototypeSlice,
@@ -69,7 +68,7 @@ const cjsParseCache = new SafeWeakMap();
 
 // Set first due to cycle with ESM loader functions.
 module.exports = {
-  wrapSafe, Module, toRealPath, readPackageScope, cjsParseCache,
+  wrapSafe, Module, cjsParseCache,
   get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
   initializeCJS,
 };
@@ -89,9 +88,7 @@ const {
 const { internalCompileFunction } = require('internal/vm');
 const assert = require('internal/assert');
 const fs = require('fs');
-const internalFS = require('internal/fs/utils');
 const path = require('path');
-const { sep } = path;
 const { internalModuleStat } = internalBinding('fs');
 const { safeGetenv } = internalBinding('credentials');
 const {
@@ -107,6 +104,7 @@ const {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
+  toRealPath,
 } = require('internal/modules/helpers');
 const packageJsonReader = require('internal/modules/package_json_reader');
 const { getOptionValue, getEmbedderOptions } = require('internal/options');
@@ -402,15 +400,7 @@ function initializeCJS() {
 //   -> a.<ext>
 //   -> a/index.<ext>
 
-/**
- * @param {string} requestPath
- * @return {PackageConfig}
- */
-function readPackage(requestPath) {
-  return packageJsonReader.read(path.resolve(requestPath, 'package.json'));
-}
-
-let _readPackage = readPackage;
+let _readPackage = packageJsonReader.readPackage;
 ObjectDefineProperty(Module, '_readPackage', {
   __proto__: null,
   get() { return _readPackage; },
@@ -422,31 +412,6 @@ ObjectDefineProperty(Module, '_readPackage', {
   configurable: true,
 });
 
-/**
- * Get the nearest parent package.json file from a given path.
- * Return the package.json data and the path to the package.json file, or false.
- * @param {string} checkPath The path to start searching from.
- */
-function readPackageScope(checkPath) {
-  const rootSeparatorIndex = StringPrototypeIndexOf(checkPath, sep);
-  let separatorIndex;
-  do {
-    separatorIndex = StringPrototypeLastIndexOf(checkPath, sep);
-    checkPath = StringPrototypeSlice(checkPath, 0, separatorIndex);
-    if (StringPrototypeEndsWith(checkPath, sep + 'node_modules')) {
-      return false;
-    }
-    const pjson = _readPackage(checkPath + sep);
-    if (pjson.exists) {
-      return {
-        data: pjson,
-        path: checkPath,
-      };
-    }
-  } while (separatorIndex > rootSeparatorIndex);
-  return false;
-}
-
 /**
  * Try to load a specifier as a package.
  * @param {string} requestPath The path to what we are trying to load
@@ -491,14 +456,6 @@ function tryPackage(requestPath, exts, isMain, originalPath) {
   return actual;
 }
 
-/**
- * Cache for storing resolved real paths of modules.
- * In order to minimize unnecessary lstat() calls, this cache is a list of known-real paths.
- * Set to an empty Map to reset.
- * @type {Map<string, string>}
- */
-const realpathCache = new SafeMap();
-
 /**
  * Check if the file exists and is not a directory if using `--preserve-symlinks` and `isMain` is false, keep symlinks
  * intact, otherwise resolve to the absolute realpath.
@@ -514,17 +471,6 @@ function tryFile(requestPath, isMain) {
   return toRealPath(requestPath);
 }
 
-
-/**
- * Resolves the path of a given `require` specifier, following symlinks.
- * @param {string} requestPath The `require` specifier
- */
-function toRealPath(requestPath) {
-  return fs.realpathSync(requestPath, {
-    [internalFS.realpathCacheKey]: realpathCache,
-  });
-}
-
 /**
  * Given a path, check if the file exists with any of the set extensions.
  * @param {string} basePath The path and filename without extension
@@ -586,7 +532,7 @@ function trySelfParentPath(parent) {
 function trySelf(parentPath, request) {
   if (!parentPath) { return false; }
 
-  const { data: pkg, path: pkgPath } = readPackageScope(parentPath);
+  const { data: pkg, path: pkgPath } = packageJsonReader.readPackageScope(parentPath);
   if (!pkg || pkg.exports == null || pkg.name === undefined) {
     return false;
   }
@@ -1143,7 +1089,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
 
   if (request[0] === '#' && (parent?.filename || parent?.id === '<repl>')) {
     const parentPath = parent?.filename ?? process.cwd() + path.sep;
-    const pkg = readPackageScope(parentPath) || { __proto__: null };
+    const pkg = packageJsonReader.readPackageScope(parentPath) || { __proto__: null };
     if (pkg.data?.imports != null) {
       try {
         const { packageImportsResolve } = require('internal/modules/esm/resolve');
@@ -1431,7 +1377,7 @@ Module._extensions['.js'] = function(module, filename) {
     content = fs.readFileSync(filename, 'utf8');
   }
   if (StringPrototypeEndsWith(filename, '.js')) {
-    const pkg = readPackageScope(filename) || { __proto__: null };
+    const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
     // Function require shouldn't be used in ES modules.
     if (pkg.data?.type === 'module') {
       const parent = moduleParentCache.get(module);

diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js
@@ -21,6 +21,8 @@ const {
 const { BuiltinModule } = require('internal/bootstrap/realm');
 
 const { validateString } = require('internal/validators');
+const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
+const internalFS = require('internal/fs/utils');
 const path = require('path');
 const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
 
@@ -39,6 +41,23 @@ let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
 
 /** @typedef {import('internal/modules/cjs/loader.js').Module} Module */
 
+/**
+ * Cache for storing resolved real paths of modules.
+ * In order to minimize unnecessary lstat() calls, this cache is a list of known-real paths.
+ * Set to an empty Map to reset.
+ * @type {Map<string, string>}
+ */
+const realpathCache = new SafeMap();
+/**
+ * Resolves the path of a given `require` specifier, following symlinks.
+ * @param {string} requestPath The `require` specifier
+ */
+function toRealPath(requestPath) {
+  return fs.realpathSync(requestPath, {
+    [internalFS.realpathCacheKey]: realpathCache,
+  });
+}
+
 /** @type {Set<string>} */
 let cjsConditions;
 /**
@@ -310,4 +329,5 @@ module.exports = {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
+  toRealPath,
 };
diff --git a/lib/internal/modules/package_json_reader.js b/lib/internal/modules/package_json_reader.js
@@ -4,12 +4,16 @@ const {
   JSONParse,
   ObjectPrototypeHasOwnProperty,
   SafeMap,
+  StringPrototypeEndsWith,
+  StringPrototypeIndexOf,
+  StringPrototypeLastIndexOf,
+  StringPrototypeSlice,
 } = primordials;
 const {
   ERR_INVALID_PACKAGE_CONFIG,
 } = require('internal/errors').codes;
 const { internalModuleReadJSON } = internalBinding('fs');
-const { toNamespacedPath } = require('path');
+const { resolve, sep, toNamespacedPath } = require('path');
 const { kEmptyObject } = require('internal/util');
 
 const { fileURLToPath, pathToFileURL } = require('internal/url');
@@ -128,4 +132,41 @@ function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
   return result;
 }
 
-module.exports = { read };
+/**
+ * @param {string} requestPath
+ * @return {PackageConfig}
+ */
+function readPackage(requestPath) {
+  return read(resolve(requestPath, 'package.json'));
+}
+
+/**
+ * Get the nearest parent package.json file from a given path.
+ * Return the package.json data and the path to the package.json file, or false.
+ * @param {string} checkPath The path to start searching from.
+ */
+function readPackageScope(checkPath) {
+  const rootSeparatorIndex = StringPrototypeIndexOf(checkPath, sep);
+  let separatorIndex;
+  do {
+    separatorIndex = StringPrototypeLastIndexOf(checkPath, sep);
+    checkPath = StringPrototypeSlice(checkPath, 0, separatorIndex);
+    if (StringPrototypeEndsWith(checkPath, sep + 'node_modules')) {
+      return false;
+    }
+    const pjson = readPackage(checkPath + sep);
+    if (pjson.exists) {
+      return {
+        data: pjson,
+        path: checkPath,
+      };
+    }
+  } while (separatorIndex > rootSeparatorIndex);
+  return false;
+}
+
+module.exports = {
+  read,
+  readPackage,
+  readPackageScope,
+};
diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js
@@ -16,12 +16,13 @@ function resolveMainPath(main) {
   // Note extension resolution for the main entry point can be deprecated in a
   // future major.
   // Module._findPath is monkey-patchable here.
-  const { Module, toRealPath } = require('internal/modules/cjs/loader');
+  const { Module } = require('internal/modules/cjs/loader');
   let mainPath = Module._findPath(path.resolve(main), null, true);
   if (!mainPath) { return; }
 
   const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
   if (!preserveSymlinksMain) {
+    const { toRealPath } = require('internal/modules/helpers');
     mainPath = toRealPath(mainPath);
   }
 
@@ -51,10 +52,11 @@ function shouldUseESMLoader(mainPath) {
   if (esModuleSpecifierResolution === 'node') {
     return true;
   }
-  const { readPackageScope } = require('internal/modules/cjs/loader');
   // Determine the module format of the main
   if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs')) { return true; }
   if (!mainPath || StringPrototypeEndsWith(mainPath, '.cjs')) { return false; }
+
+  const { readPackageScope } = require('internal/modules/package_json_reader');
   const pkg = readPackageScope(mainPath);
   return pkg && pkg.data.type === 'module';
 }

diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot
@@ -4,11 +4,11 @@ throw new Error('Should include grayed stack trace')
 
 Error: Should include grayed stack trace
     at Object.<anonymous> [90m(/[39mtest*force_colors.js:1:7[90m)[39m
-[90m    at Module._compile (node:internal*modules*cjs*loader:1244:14)[39m
-[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1298:10)[39m
-[90m    at Module.load (node:internal*modules*cjs*loader:1101:32)[39m
-[90m    at Module._load (node:internal*modules*cjs*loader:942:12)[39m
-[90m    at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:88:12)[39m
+[90m    at Module._compile (node:internal*modules*cjs*loader:1356:14)[39m
+[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1414:10)[39m
+[90m    at Module.load (node:internal*modules*cjs*loader:1197:32)[39m
+[90m    at Module._load (node:internal*modules*cjs*loader:1013:12)[39m
+[90m    at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:109:12)[39m
 [90m    at node:internal*main*run_main_module:23:47[39m
 
 Node.js *