From 3deeccfe12cebfe1ddbbdab1b757608f2055ef76 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 11 May 2023 22:47:32 +0200 Subject: [PATCH 01/32] module: make CJS load from ESM loader --- lib/internal/modules/esm/load.js | 68 ++++++++++++- lib/internal/modules/esm/loader.js | 47 ++++++++- lib/internal/modules/esm/module_job.js | 13 +++ lib/internal/modules/esm/translators.js | 129 +++++++++++++----------- 4 files changed, 190 insertions(+), 67 deletions(-) diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index fbd86e2881c0f0..39b2cfc2aebb88 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -69,6 +69,33 @@ async function getSource(url, context) { return { __proto__: null, responseURL, source }; } +function getSourceSync(url, context) { + const parsed = new URL(url); + const responseURL = url; + let source; + if (parsed.protocol === 'file:') { + const { readFileSync } = require('fs'); + source = readFileSync(parsed); + } else if (parsed.protocol === 'data:') { + const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname); + if (!match) { + throw new ERR_INVALID_URL(url); + } + const { 1: base64, 2: body } = match; + source = BufferFrom(decodeURIComponent(body), base64 ? 'base64' : 'utf8'); + } else { + const supportedSchemes = ['file', 'data']; + if (experimentalNetworkImports) { + ArrayPrototypePush(supportedSchemes, 'http', 'https'); + } + throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, supportedSchemes); + } + if (policy?.manifest) { + policy.manifest.assertIntegrity(parsed, source); + } + return { __proto__: null, responseURL, source }; +} + /** * Node.js default load hook. @@ -92,10 +119,7 @@ async function defaultLoad(url, context = kEmptyObject) { validateAssertions(url, format, importAssertions); - if ( - format === 'builtin' || - format === 'commonjs' - ) { + if (format === 'builtin') { source = null; } else if (source == null) { ({ responseURL, source } = await getSource(urlInstance, context)); @@ -109,6 +133,41 @@ async function defaultLoad(url, context = kEmptyObject) { }; } +/** + * Node.js default load hook. + * @param {string} url + * @param {object} context + * @returns {object} + */ +function defaultLoadSync(url, context = kEmptyObject) { + let responseURL = url; + const { importAssertions } = context; + let { + format, + source, + } = context; + + if (format == null) { + format = defaultGetFormat(url, context); + } + + validateAssertions(url, format, importAssertions); + + if (format === 'builtin') { + source = null; + } else if (source == null) { + ({ responseURL, source } = getSourceSync(url, context)); + } + + return { + __proto__: null, + format, + responseURL, + source, + }; +} + + /** * throws an error if the protocol is not one of the protocols * that can be loaded in the default loader @@ -160,5 +219,6 @@ function throwUnknownModuleFormat(url, format) { module.exports = { defaultLoad, + defaultLoadSync, throwUnknownModuleFormat, }; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index f5ee63ddee5d4f..e7ea1affff0a38 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -18,7 +18,7 @@ const { emitExperimentalWarning } = require('internal/util'); const { getDefaultConditions, } = require('internal/modules/esm/utils'); -let defaultResolve, defaultLoad, importMetaInitializer; +let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer; function newResolveCache() { const { ResolveCache } = require('internal/modules/esm/module_map'); @@ -220,7 +220,12 @@ class ModuleLoader { return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions); } - getJobFromResolveResult(resolveResult, parentURL, importAssertions) { + getModuleJobSync(specifier, parentURL, importAssertions) { + const resolveResult = this.resolveSync(specifier, parentURL, importAssertions); + return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions, true); + } + + getJobFromResolveResult(resolveResult, parentURL, importAssertions, sync) { const { url, format } = resolveResult; const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions; let job = this.loadCache.get(url, resolvedImportAssertions.type); @@ -231,7 +236,7 @@ class ModuleLoader { } if (job === undefined) { - job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format); + job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format, sync); } return job; @@ -248,8 +253,25 @@ class ModuleLoader { * `resolve` hook * @returns {Promise} The (possibly pending) module job */ - #createModuleJob(url, importAssertions, parentURL, format) { - const moduleProvider = async (url, isMain) => { + #createModuleJob(url, importAssertions, parentURL, format, sync) { + const moduleProvider = sync ? (url, isMain) => { + const { + format: finalFormat, + responseURL, + source, + } = this.loadSync(url, { + format, + importAssertions, + }); + + const translator = getTranslators().get(finalFormat); + + if (!translator) { + throw new ERR_UNKNOWN_MODULE_FORMAT(finalFormat, responseURL); + } + + return FunctionPrototypeCall(translator, this, responseURL, source, isMain); + } : async (url, isMain) => { const { format: finalFormat, responseURL, @@ -388,6 +410,15 @@ class ModuleLoader { return result; } + loadSync(url, context) { + defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync; + + const result = defaultLoadSync(url, context); + result.format = `require-${result?.format}`; + this.validateLoadResult(url, result?.format); + return result; + } + validateLoadResult(url, format) { if (format == null) { require('internal/modules/esm/load').throwUnknownModuleFormat(url, format); @@ -473,6 +504,12 @@ class CustomizedModuleLoader { forceLoadHooks() { hooksProxy.waitForWorker(); } + loadSync(url, context) { + const result = hooksProxy.makeSyncRequest('load', url, context); + this.validateLoadResult(url, result?.format); + + return result; + } } let emittedExperimentalWarning = false; diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 35185c64a7d08f..b6089030a51bb8 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -57,6 +57,8 @@ class ModuleJob { this.isMain = isMain; this.inspectBrk = inspectBrk; + this.url = url; + this.module = undefined; // Expose the promise to the ModuleWrap directly for linking below. // `this.module` is also filled in below. @@ -186,6 +188,17 @@ class ModuleJob { } } + runSync() { + assert(this.modulePromise instanceof ModuleWrap); + + // TODO: better handle errors + this.modulePromise.instantiate(); + const timeout = -1; + const breakOnSigint = false; + this.modulePromise.evaluate(timeout, breakOnSigint); + return { __proto__: null, module: this.modulePromise }; + } + async run() { await this.instantiate(); const timeout = -1; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 267d89f1d44730..7a7c45e550d12e 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -1,13 +1,13 @@ 'use strict'; const { - ArrayPrototypeForEach, ArrayPrototypeMap, - Boolean, + Function, JSONParse, ObjectGetPrototypeOf, ObjectPrototypeHasOwnProperty, ObjectKeys, + ReflectApply, SafeArrayIterator, SafeMap, SafeSet, @@ -25,7 +25,7 @@ function lazyTypes() { } const { readFileSync } = require('fs'); -const { extname, isAbsolute } = require('path'); +const { dirname, join } = require('path'); const { hasEsmSyntax, loadBuiltinModule, @@ -33,13 +33,12 @@ const { } = require('internal/modules/helpers'); const { Module: CJSModule, - cjsParseCache, } = require('internal/modules/cjs/loader'); -const { fileURLToPath, URL } = require('internal/url'); +const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { debug = fn; }); -const { emitExperimentalWarning } = require('internal/util'); +const { emitExperimentalWarning, kEmptyObject } = require('internal/util'); const { ERR_UNKNOWN_BUILTIN_MODULE, ERR_INVALID_RETURN_PROPERTY_VALUE, @@ -140,6 +139,45 @@ function enrichCJSError(err, content, filename) { } } +const cjsCache = new Map(); +function loadCJS(url, source) { + if (cjsCache.has(url)) return cjsCache.get(url).exports; + + debug(`Loading CJSModule ${url}`); + + const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); + + // TODO: make module object compatible with CJS Module class + const module = { exports: {} }; + const filename = fileURLToPath(new URL(url)); + const __dirname = dirname(filename); + // TODO: add missing properties on the require function + // eslint-disable-next-line func-name-matching,func-style + const requireFn = function require(spec) { + // TODO: add support for absolute paths + if (spec[0] === '.') { + spec = pathToFileURL(join(__dirname, spec)); + } + // TODO: add support for JSON imports + const job = asyncESM.esmLoader.getModuleJobSync(spec, url, kEmptyObject); + job.runSync(); + return cjsCache.get(job.url).exports; + }; + + cjsCache.set(url, module); + ReflectApply(compiledWrapper, module.exports, + [module.exports, requireFn, module, filename, __dirname]); + + return module.exports; +} + +translators.set('require-commonjs', (url, source, isMain) => { + return new ModuleWrap(url, undefined, ['default'], function() { + const exports = loadCJS(url, source); + this.setExport('default', exports); + }); +}); + // Strategy for loading a node-style CommonJS module const isWindows = process.platform === 'win32'; translators.set('commonjs', async function commonjsStrategy(url, source, @@ -149,25 +187,12 @@ translators.set('commonjs', async function commonjsStrategy(url, source, const filename = fileURLToPath(new URL(url)); if (!cjsParse) await initCJSParse(); - const { module, exportNames } = cjsPreparseModuleExports(filename); + const { exportNames } = cjsPreparseModuleExports(filename); const namesWithDefault = exportNames.has('default') ? [...exportNames] : ['default', ...exportNames]; return new ModuleWrap(url, undefined, namesWithDefault, function() { - debug(`Loading CJSModule ${url}`); - - let exports; - if (asyncESM.esmLoader.cjsCache.has(module)) { - exports = asyncESM.esmLoader.cjsCache.get(module); - asyncESM.esmLoader.cjsCache.delete(module); - } else { - try { - exports = CJSModule._load(filename, undefined, isMain); - } catch (err) { - enrichCJSError(err, undefined, filename); - throw err; - } - } + const exports = loadCJS(url, source); for (const exportName of exportNames) { if (!ObjectPrototypeHasOwnProperty(exports, exportName) || @@ -187,20 +212,6 @@ translators.set('commonjs', async function commonjsStrategy(url, source, }); function cjsPreparseModuleExports(filename) { - let module = CJSModule._cache[filename]; - if (module) { - const cached = cjsParseCache.get(module); - if (cached) - return { module, exportNames: cached.exportNames }; - } - const loaded = Boolean(module); - if (!loaded) { - module = new CJSModule(filename); - module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); - CJSModule._cache[filename] = module; - } - let source; try { source = readFileSync(filename, 'utf8'); @@ -219,29 +230,30 @@ function cjsPreparseModuleExports(filename) { const exportNames = new SafeSet(new SafeArrayIterator(exports)); // Set first for cycles. - cjsParseCache.set(module, { source, exportNames, loaded }); - - if (reexports.length) { - module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); - } - ArrayPrototypeForEach(reexports, (reexport) => { - let resolved; - try { - resolved = CJSModule._resolveFilename(reexport, module); - } catch { - return; - } - const ext = extname(resolved); - if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) && - isAbsolute(resolved)) { - const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved); - for (const name of reexportNames) - exportNames.add(name); - } - }); - - return { module, exportNames }; + // cjsParseCache.set(module, { source, exportNames }); + + // TODO: add back support for reexports + // if (reexports.length) { + // module.filename = filename; + // module.paths = CJSModule._nodeModulePaths(module.path); + // } + // ArrayPrototypeForEach(reexports, (reexport) => { + // let resolved; + // try { + // resolved = CJSModule._resolveFilename(reexport, module); + // } catch { + // return; + // } + // const ext = extname(resolved); + // if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) && + // isAbsolute(resolved)) { + // const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved); + // for (const name of reexportNames) + // exportNames.add(name); + // } + // }); + + return { exportNames }; } // Strategy for loading a node builtin CommonJS module that isn't @@ -283,6 +295,7 @@ translators.set('json', async function jsonStrategy(url, source) { // A require call could have been called on the same file during loading and // that resolves synchronously. To make sure we always return the identical // export, we have to check again if the module already exists or not. + // TODO: remove CJS loader from here as well. module = CJSModule._cache[modulePath]; if (module && module.loaded) { const exports = module.exports; From c1b346c5950c69ad2b8e45f809a34af0c6d16541 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 14 May 2023 19:18:16 +0200 Subject: [PATCH 02/32] fixup! module: make CJS load from ESM loader --- lib/internal/modules/esm/loader.js | 1 + lib/internal/modules/esm/module_job.js | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index e7ea1affff0a38..b2296b970e0b47 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -307,6 +307,7 @@ class ModuleLoader { moduleProvider, parentURL === undefined, inspectBrk, + sync, ); this.loadCache.set(url, importAssertions.type, job); diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index b6089030a51bb8..a269d31cc79074 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -51,7 +51,7 @@ class ModuleJob { // `loader` is the Loader instance used for loading dependencies. // `moduleProvider` is a function constructor(loader, url, importAssertions = { __proto__: null }, - moduleProvider, isMain, inspectBrk) { + moduleProvider, isMain, inspectBrk, sync = false) { this.loader = loader; this.importAssertions = importAssertions; this.isMain = isMain; @@ -64,6 +64,11 @@ class ModuleJob { // `this.module` is also filled in below. this.modulePromise = ReflectApply(moduleProvider, loader, [url, isMain]); + if (sync) { + this.module = this.modulePromise; + this.modulePromise = PromiseResolve(this.module); + } + // Wait for the ModuleWrap instance being linked with all dependencies. const link = async () => { this.module = await this.modulePromise; @@ -189,14 +194,18 @@ class ModuleJob { } runSync() { - assert(this.modulePromise instanceof ModuleWrap); + if (this.instantiated !== undefined) { + return this.instantiated; + } + assert(this.module instanceof ModuleWrap); // TODO: better handle errors - this.modulePromise.instantiate(); + this.module.instantiate(); + this.instantiated = PromiseResolve(); const timeout = -1; const breakOnSigint = false; - this.modulePromise.evaluate(timeout, breakOnSigint); - return { __proto__: null, module: this.modulePromise }; + this.module.evaluate(timeout, breakOnSigint); + return { __proto__: null, module: this.module }; } async run() { From df14ab841fec9ea3023b11bb70a034c55dcde50c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 14 May 2023 19:25:41 +0200 Subject: [PATCH 03/32] fixup! module: make CJS load from ESM loader --- lib/internal/modules/esm/module_job.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index a269d31cc79074..a37512d37566cb 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -194,10 +194,10 @@ class ModuleJob { } runSync() { + assert(this.module instanceof ModuleWrap); if (this.instantiated !== undefined) { - return this.instantiated; + return { __proto__: null, module: this.module }; } - assert(this.module instanceof ModuleWrap); // TODO: better handle errors this.module.instantiate(); From f5fdea1edcc1e6fe4121429dc37400faa8032661 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 23 May 2023 16:59:08 +0200 Subject: [PATCH 04/32] make CJS ModuleWrap instanciation consistent --- lib/internal/modules/esm/translators.js | 153 ++++++++++++------------ 1 file changed, 75 insertions(+), 78 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 7a7c45e550d12e..3b2fc6a0efa33e 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -24,8 +24,9 @@ function lazyTypes() { return _TYPES = require('internal/util/types'); } +const assert = require('internal/assert'); const { readFileSync } = require('fs'); -const { dirname, join } = require('path'); +const { dirname, extname, isAbsolute, join } = require('path'); const { hasEsmSyntax, loadBuiltinModule, @@ -33,6 +34,7 @@ const { } = require('internal/modules/helpers'); const { Module: CJSModule, + cjsParseCache, } = require('internal/modules/cjs/loader'); const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { @@ -139,61 +141,45 @@ function enrichCJSError(err, content, filename) { } } -const cjsCache = new Map(); -function loadCJS(url, source) { - if (cjsCache.has(url)) return cjsCache.get(url).exports; - - debug(`Loading CJSModule ${url}`); - - const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); - - // TODO: make module object compatible with CJS Module class - const module = { exports: {} }; - const filename = fileURLToPath(new URL(url)); - const __dirname = dirname(filename); - // TODO: add missing properties on the require function - // eslint-disable-next-line func-name-matching,func-style - const requireFn = function require(spec) { - // TODO: add support for absolute paths - if (spec[0] === '.') { - spec = pathToFileURL(join(__dirname, spec)); - } - // TODO: add support for JSON imports - const job = asyncESM.esmLoader.getModuleJobSync(spec, url, kEmptyObject); - job.runSync(); - return cjsCache.get(job.url).exports; - }; - - cjsCache.set(url, module); - ReflectApply(compiledWrapper, module.exports, - [module.exports, requireFn, module, filename, __dirname]); - - return module.exports; -} - -translators.set('require-commonjs', (url, source, isMain) => { - return new ModuleWrap(url, undefined, ['default'], function() { - const exports = loadCJS(url, source); - this.setExport('default', exports); - }); -}); - -// Strategy for loading a node-style CommonJS module -const isWindows = process.platform === 'win32'; -translators.set('commonjs', async function commonjsStrategy(url, source, - isMain) { +// TODO: can we use a weak map instead? +const cjsCache = new SafeMap(); +function createCJSModuleWrap(url, source) { debug(`Translating CJSModule ${url}`); const filename = fileURLToPath(new URL(url)); - if (!cjsParse) await initCJSParse(); - const { exportNames } = cjsPreparseModuleExports(filename); + const { exportNames } = cjsPreparseModuleExports(filename, source); const namesWithDefault = exportNames.has('default') ? [...exportNames] : ['default', ...exportNames]; return new ModuleWrap(url, undefined, namesWithDefault, function() { - const exports = loadCJS(url, source); + if (cjsCache.has(url)) return cjsCache.get(url).exports; + + debug(`Loading CJSModule ${url}`); + const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); + + // TODO: make module object compatible with CJS Module class + const module = { exports: {} }; + const __dirname = dirname(filename); + // TODO: add missing properties on the require function + // eslint-disable-next-line func-name-matching,func-style + const requireFn = function require(spec) { + // TODO: add support for absolute paths + if (spec[0] === '.') { + spec = pathToFileURL(join(__dirname, spec)); + } + // TODO: add support for JSON imports + const job = asyncESM.esmLoader.getModuleJob(spec, url, kEmptyObject, true); + job.runSync(); + return cjsCache.get(job.url).exports; + }; + + cjsCache.set(url, module); + ReflectApply(compiledWrapper, module.exports, + [module.exports, requireFn, module, filename, __dirname]); + + const { exports } = module; for (const exportName of exportNames) { if (!ObjectPrototypeHasOwnProperty(exports, exportName) || exportName === 'default') @@ -209,16 +195,23 @@ translators.set('commonjs', async function commonjsStrategy(url, source, } this.setExport('default', exports); }); + +} + +translators.set('require-commonjs', (url, source, isMain) => { + assert(cjsParse); + + return createCJSModuleWrap(url, source); }); -function cjsPreparseModuleExports(filename) { - let source; - try { - source = readFileSync(filename, 'utf8'); - } catch { - // Continue regardless of error. - } +// Strategy for loading a node-style CommonJS module +translators.set('commonjs', async function commonjsStrategy(url, source, + isMain) { + if (!cjsParse) await initCJSParse(); + return createCJSModuleWrap(url, source); +}); +function cjsPreparseModuleExports(filename, source) { let exports, reexports; try { ({ exports, reexports } = cjsParse(source || '')); @@ -230,30 +223,33 @@ function cjsPreparseModuleExports(filename) { const exportNames = new SafeSet(new SafeArrayIterator(exports)); // Set first for cycles. - // cjsParseCache.set(module, { source, exportNames }); - - // TODO: add back support for reexports - // if (reexports.length) { - // module.filename = filename; - // module.paths = CJSModule._nodeModulePaths(module.path); - // } - // ArrayPrototypeForEach(reexports, (reexport) => { - // let resolved; - // try { - // resolved = CJSModule._resolveFilename(reexport, module); - // } catch { - // return; - // } - // const ext = extname(resolved); - // if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) && - // isAbsolute(resolved)) { - // const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved); - // for (const name of reexportNames) - // exportNames.add(name); - // } - // }); - - return { exportNames }; + cjsParseCache.set(module, { source, exportNames }); + + if (reexports.length) { + module.filename = filename; + module.paths = CJSModule._nodeModulePaths(module.path); + for (let i = 0; i < reexports.length; i++) { + const reexport = reexports[i]; + let resolved; + try { + // TODO: this should be calling the `resolve` hook chain instead. + resolved = CJSModule._resolveFilename(reexport, module); + } catch { + return; + } + // 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)) { + const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved, readFileSync(resolved, 'utf-8')); + for (const name of reexportNames) + exportNames.add(name); + } + } + } + + return { exportNames, reexports }; } // Strategy for loading a node builtin CommonJS module that isn't @@ -271,6 +267,7 @@ translators.set('builtin', async function builtinStrategy(url) { }); // Strategy for loading a JSON file +const isWindows = process.platform === 'win32'; translators.set('json', async function jsonStrategy(url, source) { emitExperimentalWarning('Importing JSON modules'); assertBufferSource(source, true, 'load'); From 598d7893b52892729cc67f3144f3bb048b25eac0 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 23 May 2023 17:30:18 +0200 Subject: [PATCH 05/32] fixup! make CJS ModuleWrap instanciation consistent --- lib/internal/modules/esm/translators.js | 28 +++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 3b2fc6a0efa33e..9810c7b10b3253 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -2,6 +2,7 @@ const { ArrayPrototypeMap, + Boolean, Function, JSONParse, ObjectGetPrototypeOf, @@ -148,7 +149,7 @@ function createCJSModuleWrap(url, source) { const filename = fileURLToPath(new URL(url)); - const { exportNames } = cjsPreparseModuleExports(filename, source); + const { exportNames, module } = cjsPreparseModuleExports(filename, source); const namesWithDefault = exportNames.has('default') ? [...exportNames] : ['default', ...exportNames]; @@ -159,8 +160,6 @@ function createCJSModuleWrap(url, source) { const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); - // TODO: make module object compatible with CJS Module class - const module = { exports: {} }; const __dirname = dirname(filename); // TODO: add missing properties on the require function // eslint-disable-next-line func-name-matching,func-style @@ -212,9 +211,26 @@ translators.set('commonjs', async function commonjsStrategy(url, source, }); function cjsPreparseModuleExports(filename, source) { + let module = CJSModule._cache[filename]; + if (module) { + const cached = cjsParseCache.get(module); + if (cached) + return { module, exportNames: cached.exportNames }; + } + const loaded = Boolean(module); + debug(`IsLoaded CJSModule ${filename}? ${loaded}`); + if (!loaded) { + module = new CJSModule(filename); + module.filename = filename; + module.paths = CJSModule._nodeModulePaths(module.path); + CJSModule._cache[filename] = module; + } + let exports, reexports; try { - ({ exports, reexports } = cjsParse(source || '')); + // TODO: use a TextDecoder, or add support for BufferView in `cjsParse` + const sourceString = typeof source === 'string' ? source : `${source}`; + ({ exports, reexports } = cjsParse(sourceString || '')); } catch { exports = []; reexports = []; @@ -235,7 +251,7 @@ function cjsPreparseModuleExports(filename, source) { // TODO: this should be calling the `resolve` hook chain instead. resolved = CJSModule._resolveFilename(reexport, module); } catch { - return; + continue; } // TODO: this should be calling the `load` hook chain and check if it returns // `format: 'commonjs'` instead of relying on file extensions. @@ -249,7 +265,7 @@ function cjsPreparseModuleExports(filename, source) { } } - return { exportNames, reexports }; + return { module, exportNames }; } // Strategy for loading a node builtin CommonJS module that isn't From afd882a8cbaaa91e2a1596d493de80e5c0bcc73e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 23 May 2023 17:36:23 +0200 Subject: [PATCH 06/32] add some properties to `require` --- lib/internal/modules/esm/translators.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 9810c7b10b3253..0fe2963dff81d5 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -41,7 +41,7 @@ const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { debug = fn; }); -const { emitExperimentalWarning, kEmptyObject } = require('internal/util'); +const { emitExperimentalWarning, kEmptyObject, setOwnProperty } = require('internal/util'); const { ERR_UNKNOWN_BUILTIN_MODULE, ERR_INVALID_RETURN_PROPERTY_VALUE, @@ -147,7 +147,7 @@ const cjsCache = new SafeMap(); function createCJSModuleWrap(url, source) { debug(`Translating CJSModule ${url}`); - const filename = fileURLToPath(new URL(url)); + const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url; const { exportNames, module } = cjsPreparseModuleExports(filename, source); const namesWithDefault = exportNames.has('default') ? @@ -173,6 +173,16 @@ function createCJSModuleWrap(url, source) { job.runSync(); return cjsCache.get(job.url).exports; }; + setOwnProperty(require, 'resolve', function resolve(spec) { + // TODO: add support for absolute paths + if (spec[0] === '.') { + spec = pathToFileURL(join(__dirname, spec)); + } + const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); + return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; + }); + setOwnProperty(require, 'cache', CJSModule._cache); + setOwnProperty(require, 'main', process.mainModule); cjsCache.set(url, module); ReflectApply(compiledWrapper, module.exports, @@ -211,6 +221,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source, }); function cjsPreparseModuleExports(filename, source) { + // TODO: Do we want to keep hitting the user mutable CJS loader here? let module = CJSModule._cache[filename]; if (module) { const cached = cjsParseCache.get(module); From 9958f1f3b27bf250f5df844a05c8d9319e2f529a Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 23 May 2023 17:42:32 +0200 Subject: [PATCH 07/32] fixup! add some properties to `require` --- lib/internal/modules/esm/translators.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 0fe2963dff81d5..6e42ae48f38fa3 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -173,16 +173,17 @@ function createCJSModuleWrap(url, source) { job.runSync(); return cjsCache.get(job.url).exports; }; - setOwnProperty(require, 'resolve', function resolve(spec) { + setOwnProperty(requireFn, 'resolve', function resolve(spec) { // TODO: add support for absolute paths if (spec[0] === '.') { spec = pathToFileURL(join(__dirname, spec)); } + // TODO: make this work for CJSism (e.g. dir import, extensionless imports, etc.) const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; }); - setOwnProperty(require, 'cache', CJSModule._cache); - setOwnProperty(require, 'main', process.mainModule); + setOwnProperty(requireFn, 'cache', CJSModule._cache); + setOwnProperty(requireFn, 'main', process.mainModule); cjsCache.set(url, module); ReflectApply(compiledWrapper, module.exports, From 6d57350be22b43c631510971602c392302c43198 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 25 May 2023 16:40:30 +0200 Subject: [PATCH 08/32] fixes --- lib/internal/modules/esm/loader.js | 10 +++++++--- lib/internal/modules/esm/module_job.js | 2 ++ lib/internal/modules/esm/translators.js | 18 +++++++++--------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index b2296b970e0b47..b41dc77f58af82 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -414,9 +414,13 @@ class ModuleLoader { loadSync(url, context) { defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync; - const result = defaultLoadSync(url, context); - result.format = `require-${result?.format}`; - this.validateLoadResult(url, result?.format); + let result = defaultLoadSync(url, context); + let format = result?.format; + if (format === 'commonjs') { + format = 'require-commonjs'; + result = { __proto__: result, format }; + } + this.validateLoadResult(url, format); return result; } diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index a37512d37566cb..9e15ef666e269d 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -67,6 +67,8 @@ class ModuleJob { if (sync) { this.module = this.modulePromise; this.modulePromise = PromiseResolve(this.module); + } else { + this.modulePromise = PromiseResolve(this.modulePromise); } // Wait for the ModuleWrap instance being linked with all dependencies. diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 6e42ae48f38fa3..9d31b6ae69e8e1 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -161,14 +161,12 @@ function createCJSModuleWrap(url, source) { const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); const __dirname = dirname(filename); - // TODO: add missing properties on the require function // eslint-disable-next-line func-name-matching,func-style const requireFn = function require(spec) { // TODO: add support for absolute paths if (spec[0] === '.') { - spec = pathToFileURL(join(__dirname, spec)); + spec = pathToFileURL(CJSModule._findPath(join(__dirname, spec))); } - // TODO: add support for JSON imports const job = asyncESM.esmLoader.getModuleJob(spec, url, kEmptyObject, true); job.runSync(); return cjsCache.get(job.url).exports; @@ -176,7 +174,7 @@ function createCJSModuleWrap(url, source) { setOwnProperty(requireFn, 'resolve', function resolve(spec) { // TODO: add support for absolute paths if (spec[0] === '.') { - spec = pathToFileURL(join(__dirname, spec)); + spec = pathToFileURL(CJSModule._findPath(join(__dirname, spec))); } // TODO: make this work for CJSism (e.g. dir import, extensionless imports, etc.) const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); @@ -238,11 +236,11 @@ function cjsPreparseModuleExports(filename, source) { CJSModule._cache[filename] = module; } + source = stringify(source); + let exports, reexports; try { - // TODO: use a TextDecoder, or add support for BufferView in `cjsParse` - const sourceString = typeof source === 'string' ? source : `${source}`; - ({ exports, reexports } = cjsParse(sourceString || '')); + ({ exports, reexports } = cjsParse(source || '')); } catch { exports = []; reexports = []; @@ -282,11 +280,12 @@ function cjsPreparseModuleExports(filename, source) { // Strategy for loading a node builtin CommonJS module that isn't // through normal resolution -translators.set('builtin', async function builtinStrategy(url) { +translators.set('builtin', function builtinStrategy(url) { debug(`Translating BuiltinModule ${url}`); // Slice 'node:' scheme const id = StringPrototypeSlice(url, 5); const module = loadBuiltinModule(id, url); + cjsCache.set(url, module); if (!StringPrototypeStartsWith(url, 'node:') || !module) { throw new ERR_UNKNOWN_BUILTIN_MODULE(url); } @@ -296,7 +295,7 @@ translators.set('builtin', async function builtinStrategy(url) { // Strategy for loading a JSON file const isWindows = process.platform === 'win32'; -translators.set('json', async function jsonStrategy(url, source) { +translators.set('json', function jsonStrategy(url, source) { emitExperimentalWarning('Importing JSON modules'); assertBufferSource(source, true, 'load'); debug(`Loading JSONModule ${url}`); @@ -346,6 +345,7 @@ translators.set('json', async function jsonStrategy(url, source) { if (pathname) { CJSModule._cache[modulePath] = module; } + cjsCache.set(url, module); return new ModuleWrap(url, undefined, ['default'], function() { debug(`Parsing JSONModule ${url}`); this.setExport('default', module.exports); From ad7a8ed0bc7c05596bdc58dfc4ad6d61f122a3ed Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 25 May 2023 19:05:59 +0200 Subject: [PATCH 09/32] add support for requiring absolute URLs, JSON files, and `.node` --- lib/internal/modules/esm/translators.js | 30 +++++++++++++++++-------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 9d31b6ae69e8e1..18da09137d5693 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -27,7 +27,7 @@ function lazyTypes() { const assert = require('internal/assert'); const { readFileSync } = require('fs'); -const { dirname, extname, isAbsolute, join } = require('path'); +const { dirname, extname, isAbsolute } = require('path'); const { hasEsmSyntax, loadBuiltinModule, @@ -163,20 +163,32 @@ function createCJSModuleWrap(url, source) { const __dirname = dirname(filename); // eslint-disable-next-line func-name-matching,func-style const requireFn = function require(spec) { - // TODO: add support for absolute paths - if (spec[0] === '.') { - spec = pathToFileURL(CJSModule._findPath(join(__dirname, spec))); + let importAttributes = kEmptyObject; + if (!StringPrototypeStartsWith(spec, 'node:')) { + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + switch (extname(path)) { + case '.json': + importAttributes = { __proto__: null, type: 'json' }; + break; + case '.node': + return CJSModule._load(spec, module); + default: + } + spec = pathToFileURL(path); + } } - const job = asyncESM.esmLoader.getModuleJob(spec, url, kEmptyObject, true); + const job = asyncESM.esmLoader.getModuleJob(spec, url, importAttributes, true); job.runSync(); return cjsCache.get(job.url).exports; }; setOwnProperty(requireFn, 'resolve', function resolve(spec) { - // TODO: add support for absolute paths - if (spec[0] === '.') { - spec = pathToFileURL(CJSModule._findPath(join(__dirname, spec))); + if (!StringPrototypeStartsWith(spec, 'node:')) { + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + spec = pathToFileURL(path); + } } - // TODO: make this work for CJSism (e.g. dir import, extensionless imports, etc.) const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; }); From d0e4abfdd640c78736589e720f9c4d185d4d1a74 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 25 May 2023 19:13:51 +0200 Subject: [PATCH 10/32] fix `CustomizedModuleLoader` --- lib/internal/modules/esm/loader.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index b41dc77f58af82..ca7f7ac3073b94 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -510,8 +510,13 @@ class CustomizedModuleLoader { hooksProxy.waitForWorker(); } loadSync(url, context) { - const result = hooksProxy.makeSyncRequest('load', url, context); - this.validateLoadResult(url, result?.format); + let result = hooksProxy.makeSyncRequest('load', url, context); + let format = result?.format; + if (format === 'commonjs') { + format = 'require-commonjs'; + result = { __proto__: result, format }; + } + this.validateLoadResult(url, format); return result; } From 39219be375eb1fc42d4c8bd8f3c7be6706e7f9c9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 26 May 2023 01:21:38 +0200 Subject: [PATCH 11/32] stringify at the right places, `process.mainModule` --- lib/internal/modules/esm/translators.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 18da09137d5693..2682296e83fb08 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -144,15 +144,20 @@ function enrichCJSError(err, content, filename) { // TODO: can we use a weak map instead? const cjsCache = new SafeMap(); -function createCJSModuleWrap(url, source) { +function createCJSModuleWrap(url, source, isMain) { debug(`Translating CJSModule ${url}`); const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url; + source = stringify(source); const { exportNames, module } = cjsPreparseModuleExports(filename, source); const namesWithDefault = exportNames.has('default') ? [...exportNames] : ['default', ...exportNames]; + if (isMain) { + setOwnProperty(process, 'mainModule', module); + } + return new ModuleWrap(url, undefined, namesWithDefault, function() { if (cjsCache.has(url)) return cjsCache.get(url).exports; @@ -175,7 +180,7 @@ function createCJSModuleWrap(url, source) { return CJSModule._load(spec, module); default: } - spec = pathToFileURL(path); + spec = `${pathToFileURL(path)}`; } } const job = asyncESM.esmLoader.getModuleJob(spec, url, importAttributes, true); @@ -186,7 +191,7 @@ function createCJSModuleWrap(url, source) { if (!StringPrototypeStartsWith(spec, 'node:')) { const path = CJSModule._resolveFilename(spec, module); if (spec !== path) { - spec = pathToFileURL(path); + spec = `${pathToFileURL(path)}`; } } const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); @@ -228,7 +233,7 @@ translators.set('require-commonjs', (url, source, isMain) => { translators.set('commonjs', async function commonjsStrategy(url, source, isMain) { if (!cjsParse) await initCJSParse(); - return createCJSModuleWrap(url, source); + return createCJSModuleWrap(url, source, isMain); }); function cjsPreparseModuleExports(filename, source) { @@ -248,8 +253,6 @@ function cjsPreparseModuleExports(filename, source) { CJSModule._cache[filename] = module; } - source = stringify(source); - let exports, reexports; try { ({ exports, reexports } = cjsParse(source || '')); From 739dc9e868d6775cf71b33e2b9e51547006c2aa7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 31 May 2023 16:44:06 +0200 Subject: [PATCH 12/32] add dynamic import in CJS + enriched errors --- lib/internal/modules/esm/translators.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 2682296e83fb08..32fb9dac4bc4ce 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -51,6 +51,7 @@ const moduleWrap = internalBinding('module_wrap'); const { ModuleWrap } = moduleWrap; const asyncESM = require('internal/process/esm_loader'); const { emitWarningSync } = require('internal/process/warning'); +const { internalCompileFunction } = require('internal/vm'); let cjsParse; async function initCJSParse() { @@ -163,7 +164,24 @@ function createCJSModuleWrap(url, source, isMain) { debug(`Loading CJSModule ${url}`); - const compiledWrapper = Function('exports', 'require', 'module', '__filename', '__dirname', source); + let compiledWrapper; + try { + compiledWrapper = internalCompileFunction(source, [ + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ], { + filename, + importModuleDynamically(specifier, _, importAssertions) { + return asyncESM.esmLoader.import(specifier, url, importAssertions); + }, + }).function; + } catch (err) { + enrichCJSError(err, source, url); + throw err; + } const __dirname = dirname(filename); // eslint-disable-next-line func-name-matching,func-style From 297a63bcdbfee40384ee998ecd68f86be1292e20 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 29 Jul 2023 12:04:06 +0200 Subject: [PATCH 13/32] fix rebase conflict --- lib/internal/modules/esm/load.js | 2 +- lib/internal/modules/esm/translators.js | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 39b2cfc2aebb88..80a4bbf8eb23c4 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -148,7 +148,7 @@ function defaultLoadSync(url, context = kEmptyObject) { } = context; if (format == null) { - format = defaultGetFormat(url, context); + format = defaultGetFormat(new URL(url), context); } validateAssertions(url, format, importAssertions); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 32fb9dac4bc4ce..5794bd07e5ea0a 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -3,7 +3,6 @@ const { ArrayPrototypeMap, Boolean, - Function, JSONParse, ObjectGetPrototypeOf, ObjectPrototypeHasOwnProperty, @@ -201,7 +200,7 @@ function createCJSModuleWrap(url, source, isMain) { spec = `${pathToFileURL(path)}`; } } - const job = asyncESM.esmLoader.getModuleJob(spec, url, importAttributes, true); + const job = asyncESM.esmLoader.getModuleJobSync(spec, url, importAttributes); job.runSync(); return cjsCache.get(job.url).exports; }; @@ -212,7 +211,7 @@ function createCJSModuleWrap(url, source, isMain) { spec = `${pathToFileURL(path)}`; } } - const { url: resolvedURL } = asyncESM.esmLoader.resolve(spec, url); + const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(spec, url, kEmptyObject); return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; }); setOwnProperty(requireFn, 'cache', CJSModule._cache); From 32bf8784a724999fb55368531a08427a40bff859 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 29 Jul 2023 15:05:37 +0200 Subject: [PATCH 14/32] fix cache duplication issues --- lib/internal/modules/esm/translators.js | 111 ++++++++++++------------ 1 file changed, 56 insertions(+), 55 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 5794bd07e5ea0a..1a1aa0e85bbb79 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -151,6 +151,7 @@ function createCJSModuleWrap(url, source, isMain) { source = stringify(source); const { exportNames, module } = cjsPreparseModuleExports(filename, source); + cjsCache.set(url, module); const namesWithDefault = exportNames.has('default') ? [...exportNames] : ['default', ...exportNames]; @@ -159,67 +160,67 @@ function createCJSModuleWrap(url, source, isMain) { } return new ModuleWrap(url, undefined, namesWithDefault, function() { - if (cjsCache.has(url)) return cjsCache.get(url).exports; - debug(`Loading CJSModule ${url}`); - let compiledWrapper; - try { - compiledWrapper = internalCompileFunction(source, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - importModuleDynamically(specifier, _, importAssertions) { - return asyncESM.esmLoader.import(specifier, url, importAssertions); - }, - }).function; - } catch (err) { - enrichCJSError(err, source, url); - throw err; - } + if (!module.loaded) { + let compiledWrapper; + try { + compiledWrapper = internalCompileFunction(source, [ + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ], { + filename, + importModuleDynamically(specifier, _, importAssertions) { + return asyncESM.esmLoader.import(specifier, url, importAssertions); + }, + }).function; + } catch (err) { + enrichCJSError(err, source, url); + throw err; + } - const __dirname = dirname(filename); - // eslint-disable-next-line func-name-matching,func-style - const requireFn = function require(spec) { - let importAttributes = kEmptyObject; - if (!StringPrototypeStartsWith(spec, 'node:')) { - const path = CJSModule._resolveFilename(spec, module); - if (spec !== path) { - switch (extname(path)) { - case '.json': - importAttributes = { __proto__: null, type: 'json' }; - break; - case '.node': - return CJSModule._load(spec, module); - default: + const __dirname = dirname(filename); + // eslint-disable-next-line func-name-matching,func-style + const requireFn = function require(spec) { + let importAttributes = kEmptyObject; + if (!StringPrototypeStartsWith(spec, 'node:')) { + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + switch (extname(path)) { + case '.json': + importAttributes = { __proto__: null, type: 'json' }; + break; + case '.node': + return CJSModule._load(spec, module); + default: + } + spec = `${pathToFileURL(path)}`; } - spec = `${pathToFileURL(path)}`; } - } - const job = asyncESM.esmLoader.getModuleJobSync(spec, url, importAttributes); - job.runSync(); - return cjsCache.get(job.url).exports; - }; - setOwnProperty(requireFn, 'resolve', function resolve(spec) { - if (!StringPrototypeStartsWith(spec, 'node:')) { - const path = CJSModule._resolveFilename(spec, module); - if (spec !== path) { - spec = `${pathToFileURL(path)}`; + const job = asyncESM.esmLoader.getModuleJobSync(spec, url, importAttributes); + job.runSync(); + return cjsCache.get(job.url).exports; + }; + setOwnProperty(requireFn, 'resolve', function resolve(spec) { + if (!StringPrototypeStartsWith(spec, 'node:')) { + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + spec = `${pathToFileURL(path)}`; + } } - } - const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(spec, url, kEmptyObject); - return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; - }); - setOwnProperty(requireFn, 'cache', CJSModule._cache); - setOwnProperty(requireFn, 'main', process.mainModule); - - cjsCache.set(url, module); - ReflectApply(compiledWrapper, module.exports, - [module.exports, requireFn, module, filename, __dirname]); + const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(spec, url, kEmptyObject); + return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; + }); + setOwnProperty(requireFn, 'cache', CJSModule._cache); + setOwnProperty(requireFn, 'main', process.mainModule); + + ReflectApply(compiledWrapper, module.exports, + [module.exports, requireFn, module, filename, __dirname]); + setOwnProperty(module, 'loaded', true); + } const { exports } = module; for (const exportName of exportNames) { From 74c93d1d664f8969fe0ad3bcc2d384f083b1a8cf Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 31 Jul 2023 01:18:03 +0200 Subject: [PATCH 15/32] make the change opt-in --- lib/internal/modules/esm/load.js | 5 +- lib/internal/modules/esm/translators.js | 156 +++++++++++++++--------- 2 files changed, 100 insertions(+), 61 deletions(-) diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 80a4bbf8eb23c4..f06cdeec7d9f15 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -119,7 +119,10 @@ async function defaultLoad(url, context = kEmptyObject) { validateAssertions(url, format, importAssertions); - if (format === 'builtin') { + if ( + format === 'builtin' || + format === 'commonjs' + ) { source = null; } else if (source == null) { ({ responseURL, source } = await getSource(urlInstance, context)); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 1a1aa0e85bbb79..ac888b0ba15495 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -142,9 +142,75 @@ function enrichCJSError(err, content, filename) { } } +/** + * Loads a CJS module without using the monkey-patchable CJS loader. Modules + * loaded with this method have a special `require` function that calls the ESM + * loader. + */ +function loadCJSModule(module, source, url, filename) { + let compiledWrapper; + try { + compiledWrapper = internalCompileFunction(source, [ + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ], { + filename, + importModuleDynamically(specifier, _, importAssertions) { + return asyncESM.esmLoader.import(specifier, url, importAssertions); + }, + }).function; + } catch (err) { + enrichCJSError(err, source, url); + throw err; + } + + const __dirname = dirname(filename); + // eslint-disable-next-line func-name-matching,func-style + const requireFn = function require(spec) { + let importAttributes = kEmptyObject; + if (!StringPrototypeStartsWith(spec, 'node:')) { + // TODO: do not depend on the monkey-patchable CJS loader here. + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + switch (extname(path)) { + case '.json': + importAttributes = { __proto__: null, type: 'json' }; + break; + case '.node': + return CJSModule._load(spec, module); + default: + } + spec = `${pathToFileURL(path)}`; + } + } + const job = asyncESM.esmLoader.getModuleJobSync(spec, url, importAttributes); + job.runSync(); + return cjsCache.get(job.url).exports; + }; + setOwnProperty(requireFn, 'resolve', function resolve(spec) { + if (!StringPrototypeStartsWith(spec, 'node:')) { + const path = CJSModule._resolveFilename(spec, module); + if (spec !== path) { + spec = `${pathToFileURL(path)}`; + } + } + const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(spec, url, kEmptyObject); + return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; + }); + setOwnProperty(requireFn, 'cache', CJSModule._cache); + setOwnProperty(requireFn, 'main', process.mainModule); + + ReflectApply(compiledWrapper, module.exports, + [module.exports, requireFn, module, filename, __dirname]); + setOwnProperty(module, 'loaded', true); +} + // TODO: can we use a weak map instead? const cjsCache = new SafeMap(); -function createCJSModuleWrap(url, source, isMain) { +function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { debug(`Translating CJSModule ${url}`); const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url; @@ -163,66 +229,16 @@ function createCJSModuleWrap(url, source, isMain) { debug(`Loading CJSModule ${url}`); if (!module.loaded) { - let compiledWrapper; - try { - compiledWrapper = internalCompileFunction(source, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - importModuleDynamically(specifier, _, importAssertions) { - return asyncESM.esmLoader.import(specifier, url, importAssertions); - }, - }).function; - } catch (err) { - enrichCJSError(err, source, url); - throw err; - } - - const __dirname = dirname(filename); - // eslint-disable-next-line func-name-matching,func-style - const requireFn = function require(spec) { - let importAttributes = kEmptyObject; - if (!StringPrototypeStartsWith(spec, 'node:')) { - const path = CJSModule._resolveFilename(spec, module); - if (spec !== path) { - switch (extname(path)) { - case '.json': - importAttributes = { __proto__: null, type: 'json' }; - break; - case '.node': - return CJSModule._load(spec, module); - default: - } - spec = `${pathToFileURL(path)}`; - } - } - const job = asyncESM.esmLoader.getModuleJobSync(spec, url, importAttributes); - job.runSync(); - return cjsCache.get(job.url).exports; - }; - setOwnProperty(requireFn, 'resolve', function resolve(spec) { - if (!StringPrototypeStartsWith(spec, 'node:')) { - const path = CJSModule._resolveFilename(spec, module); - if (spec !== path) { - spec = `${pathToFileURL(path)}`; - } - } - const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(spec, url, kEmptyObject); - return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL; - }); - setOwnProperty(requireFn, 'cache', CJSModule._cache); - setOwnProperty(requireFn, 'main', process.mainModule); - - ReflectApply(compiledWrapper, module.exports, - [module.exports, requireFn, module, filename, __dirname]); - setOwnProperty(module, 'loaded', true); + loadCJS(module, source, url, filename); } - const { exports } = module; + let exports; + if (asyncESM.esmLoader.cjsCache.has(module)) { + exports = asyncESM.esmLoader.cjsCache.get(module); + asyncESM.esmLoader.cjsCache.delete(module); + } else { + ({ exports } = module); + } for (const exportName of exportNames) { if (!ObjectPrototypeHasOwnProperty(exports, exportName) || exportName === 'default') @@ -251,7 +267,27 @@ translators.set('require-commonjs', (url, source, isMain) => { translators.set('commonjs', async function commonjsStrategy(url, source, isMain) { if (!cjsParse) await initCJSParse(); - return createCJSModuleWrap(url, source, isMain); + + // For backward-compatibility, it's possible to return a nullish value for + // commonjs source associated with a file: URL. In this case, the source is + // obtained by calling the monkey-patchable CJS loader. + const cjsLoader = source == null ? (module, source, url, filename) => { + try { + module.load(filename); + } catch (err) { + enrichCJSError(err, source, url); + throw err; + } + } : loadCJSModule; + + try { + // We still need to read the FS to detect the exports. + source ??= readFileSync(new URL(url), 'utf8'); + } catch { + // Continue regardless of error. + } + return createCJSModuleWrap(url, source, isMain, cjsLoader); + }); function cjsPreparseModuleExports(filename, source) { From 0a6310156d1ef1d2c077c92c2db67b892f539d88 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 31 Jul 2023 01:53:05 +0200 Subject: [PATCH 16/32] fix failing tests --- lib/internal/modules/esm/loader.js | 21 +++++++--------- .../builtin-named-exports-loader.mjs | 9 ++++--- .../not-found-assert-loader.mjs | 24 +++++++------------ 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index ca7f7ac3073b94..3a52aa65349f5e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -10,6 +10,7 @@ const { } = primordials; const { + ERR_REQUIRE_ESM, ERR_UNKNOWN_MODULE_FORMAT, } = require('internal/errors').codes; const { getOptionValue } = require('internal/options'); @@ -414,11 +415,15 @@ class ModuleLoader { loadSync(url, context) { defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync; - let result = defaultLoadSync(url, context); + let result = this.#customizations ? + this.#customizations.loadSync(url, context) : + defaultLoadSync(url, context); let format = result?.format; if (format === 'commonjs') { format = 'require-commonjs'; result = { __proto__: result, format }; + } else if (format === 'module') { + throw new ERR_REQUIRE_ESM(url, true); } this.validateLoadResult(url, format); return result; @@ -501,6 +506,9 @@ class CustomizedModuleLoader { load(url, context) { return hooksProxy.makeAsyncRequest('load', undefined, url, context); } + loadSync(url, context) { + return hooksProxy.makeSyncRequest('load', url, context); + } importMetaInitialize(meta, context, loader) { hooksProxy.importMetaInitialize(meta, context, loader); @@ -509,17 +517,6 @@ class CustomizedModuleLoader { forceLoadHooks() { hooksProxy.waitForWorker(); } - loadSync(url, context) { - let result = hooksProxy.makeSyncRequest('load', url, context); - let format = result?.format; - if (format === 'commonjs') { - format = 'require-commonjs'; - result = { __proto__: result, format }; - } - this.validateLoadResult(url, format); - - return result; - } } let emittedExperimentalWarning = false; diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs index 8c317c1b7ce31e..43975b155e569f 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -32,7 +32,7 @@ export function load(url, context, next) { return { shortCircuit: true, source: generateBuiltinModule(urlObj.pathname), - format: 'module', + format: 'commonjs', }; } return next(url); @@ -46,13 +46,12 @@ function generateBuiltinModule(builtinName) { return `\ const $builtinInstance = ${GET_BUILTIN}(${JSON.stringify(builtinName)}); -export const __fromLoader = true; - -export default $builtinInstance; +module.exports = $builtinInstance; +module.exports.__fromLoader = true; ${ builtinExports - .map(name => `export const ${name} = $builtinInstance.${name};`) + .map(name => `exports.${name} = $builtinInstance.${name};`) .join('\n') } `; diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index ea4c73724298db..45e71f8a314bbf 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -1,22 +1,16 @@ import assert from 'node:assert'; -// a loader that asserts that the defaultResolve will throw "not found" -// (skipping the top-level main of course) +// A loader that asserts that the defaultResolve will throw "not found" +// (skipping the top-level main of course, and the built-in ones needed for run-worker). let mainLoad = true; export async function resolve(specifier, { importAssertions }, next) { - if (mainLoad) { + if (mainLoad || specifier === 'path' || specifier === 'worker_threads') { mainLoad = false; return next(specifier); } - try { - await next(specifier); - } - catch (e) { - assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND'); - return { - url: 'node:fs', - importAssertions, - }; - } - assert.fail(`Module resolution for ${specifier} should be throw ERR_MODULE_NOT_FOUND`); -} + await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); + return { + url: 'node:fs', + importAssertions, + }; +} \ No newline at end of file From a6f01a30cf12e0b0d1205ea25b248db016ff2029 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 31 Jul 2023 02:13:21 +0200 Subject: [PATCH 17/32] update docs --- doc/api/esm.md | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 6d510d0d0b00b5..c6b6030e5cd4dc 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -908,6 +908,9 @@ export function resolve(specifier, context, nextResolve) {