From 585ac6d7e558261783253bc002f9401e53063a7f Mon Sep 17 00:00:00 2001
From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Date: Tue, 9 Apr 2024 19:32:15 +0200
Subject: [PATCH 1/2] module: tidy code and comments
---
doc/api/errors.md | 2 +-
lib/internal/modules/cjs/loader.js | 2 +-
lib/internal/modules/esm/loader.js | 28 ++++++++++++++++++++--------
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/doc/api/errors.md b/doc/api/errors.md
index e4a6e953a21174..d2a74b230db60d 100644
--- a/doc/api/errors.md
+++ b/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.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index a81f180958aebf..b8263a793578f4 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/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);
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index df7a26c9337c39..5af25d28ea8a40 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/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} 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.
From 3b111056db4624258226570388c6558f4e6cc81d Mon Sep 17 00:00:00 2001
From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Date: Tue, 9 Apr 2024 19:54:49 +0200
Subject: [PATCH 2/2] fixup!: `urlToFilename`
---
lib/internal/modules/esm/translators.js | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
index 4bdee1b843fbd5..8f4b6b25d88896 100644
--- a/lib/internal/modules/esm/translators.js
+++ b/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);