module: tidy code and comments

PR-URL: nodejs#52437
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Feng Yu <F3n67u@outlook.com>

doc/api/errors.md

@@ -2501,7 +2501,7 @@ This is not allowed because ES Modules cannot be evaluated while they are
 already being evaluated.
 
 To avoid the cycle, the `require()` call involved in a cycle should not happen
-at the top-level of either a ES Module (via `createRequire()`) or a CommonJS
+at the top-level of either an ES Module (via `createRequire()`) or a CommonJS
 module, and should be done lazily in an inner function.
 
 <a id="ERR_REQUIRE_ASYNC_MODULE"></a>

lib/internal/modules/cjs/loader.js

@@ -1063,7 +1063,7 @@ Module._load = function(request, parent, isMain) {
     }
     // If it's cached by the ESM loader as a way to indirectly pass
     // the module in to avoid creating it twice, the loading request
-    // come from imported CJS. In that case use the kModuleCircularVisited
+    // came from imported CJS. In that case use the kModuleCircularVisited
     // to determine if it's loading or not.
     if (cachedModule[kModuleCircularVisited]) {
       return getExportsForCircularRequire(cachedModule);

lib/internal/modules/esm/loader.js

@@ -42,6 +42,10 @@ const {
 } = require('internal/modules/helpers');
 let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer;
 
+/**
+ * @typedef {import('url').URL} URL
+ */
+
 /**
  * Lazy loads the module_map module and returns a new instance of ResolveCache.
  * @returns {import('./module_map.js').ResolveCache')}
@@ -77,6 +81,10 @@ function getTranslators() {
  */
 let hooksProxy;
 
+/**
+ * @typedef {import('../cjs/loader.js').Module} CJSModule
+ */
+
 /**
  * @typedef {Record<string, any>} ModuleExports
  */
@@ -257,11 +265,11 @@ class ModuleLoader {
   /**
    * This constructs (creates, instantiates and evaluates) a module graph that
    * is require()'d.
-   * @param {import('../cjs/loader.js').Module} mod CJS module wrapper of the ESM.
+   * @param {CJSModule} mod CJS module wrapper of the ESM.
    * @param {string} filename Resolved filename of the module being require()'d
    * @param {string} source Source code. TODO(joyeecheung): pass the raw buffer.
    * @param {string} isMain Whether this module is a main module.
-   * @param {import('../cjs/loader.js').Module|undefined} parent Parent module, if any.
+   * @param {CJSModule|undefined} parent Parent module, if any.
    * @returns {{ModuleWrap}}
    */
   importSyncForRequire(mod, filename, source, isMain, parent) {
@@ -343,7 +351,7 @@ class ModuleLoader {
         }
         throw new ERR_REQUIRE_CYCLE_MODULE(message);
       }
-      // Othersie the module could be imported before but the evaluation may be already
+      // Otherwise the module could be imported before but the evaluation may be already
       // completed (e.g. the require call is lazy) so it's okay. We will return the
       // module now and check asynchronicity of the entire graph later, after the
       // graph is instantiated.
@@ -352,8 +360,12 @@ class ModuleLoader {
 
     defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
     const loadResult = defaultLoadSync(url, { format, importAttributes });
-    const { responseURL, source } = loadResult;
-    const { format: finalFormat } = loadResult;
+    const {
+      format: finalFormat,
+      responseURL,
+      source,
+    } = loadResult;
+
     this.validateLoadResult(url, finalFormat);
     if (finalFormat === 'wasm') {
       assert.fail('WASM is currently unsupported by require(esm)');
@@ -725,11 +737,11 @@ function getOrInitializeCascadedLoader() {
 
 /**
  * Register a single loader programmatically.
- * @param {string|import('url').URL} specifier
- * @param {string|import('url').URL} [parentURL] Base to use when resolving `specifier`; optional if
+ * @param {string|URL} specifier
+ * @param {string|URL} [parentURL] Base to use when resolving `specifier`; optional if
  * `specifier` is absolute. Same as `options.parentUrl`, just inline
  * @param {object} [options] Additional options to apply, described below.
- * @param {string|import('url').URL} [options.parentURL] Base to use when resolving `specifier`
+ * @param {string|URL} [options.parentURL] Base to use when resolving `specifier`
  * @param {any} [options.data] Arbitrary data passed to the loader's `initialize` hook
  * @param {any[]} [options.transferList] Objects in `data` that are changing ownership
  * @returns {void} We want to reserve the return value for potential future extension of the API.

lib/internal/modules/esm/translators.js

@@ -41,6 +41,7 @@ const { dirname, extname, isAbsolute } = require('path');
 const {
   loadBuiltinModule,
   stripBOM,
+  urlToFilename,
 } = require('internal/modules/helpers');
 const {
   kIsCachedByESMLoader,
@@ -243,7 +244,7 @@ function loadCJSModule(module, source, url, filename) {
       }
     }
     const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
-    return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL;
+    return urlToFilename(resolvedURL);
   });
   setOwnProperty(requireFn, 'main', process.mainModule);
 
@@ -265,7 +266,7 @@ const cjsCache = new SafeMap();
 function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
   debug(`Translating CJSModule ${url}`);
 
-  const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : 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);