From 1c8143ef8f11a3116b2e3bd1996bc32239adc7c0 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Mon, 6 Jan 2025 21:06:32 +0100 Subject: [PATCH 1/3] module: clarify cjs global-like error on ModuleJobSync --- lib/internal/modules/esm/module_job.js | 29 ++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 56cb8da5d31b56..688144d526f04f 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -476,8 +476,33 @@ class ModuleJobSync extends ModuleJobBase { throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); } setHasStartedUserESMExecution(); - const namespace = this.module.evaluateSync(filename, parentFilename); - return { __proto__: null, module: this.module, namespace }; + try { + const namespace = this.module.evaluateSync(filename, parentFilename); + return { __proto__: null, module: this.module, namespace }; + } catch (e) { + if (e?.name === 'ReferenceError' && + isCommonJSGlobalLikeNotDefinedError(e.message)) { + e.message += ' in ES module scope'; + + if (StringPrototypeStartsWith(e.message, 'require ')) { + e.message += ', you can use import instead'; + } + + const packageConfig = + StringPrototypeStartsWith(this.module.url, 'file://') && + RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) !== null && + require('internal/modules/package_json_reader') + .getPackageScopeConfig(this.module.url); + if (packageConfig.type === 'module') { + e.message += + '\nThis file is being treated as an ES module because it has a ' + + `'.js' file extension and '${packageConfig.pjsonPath}' contains ` + + '"type": "module". To treat it as a CommonJS script, rename it ' + + 'to use the \'.cjs\' file extension.'; + } + } + throw e; + } } } From 1b9c713353ffe57680162bdac91418568773af05 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Tue, 7 Jan 2025 12:14:29 +0100 Subject: [PATCH 2/3] fixup! module: clarify cjs global-like error on ModuleJobSync --- test/es-module/test-require-module-error-catching.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/es-module/test-require-module-error-catching.js b/test/es-module/test-require-module-error-catching.js index c314513d9bbf04..5400564b3182c6 100644 --- a/test/es-module/test-require-module-error-catching.js +++ b/test/es-module/test-require-module-error-catching.js @@ -17,5 +17,5 @@ assert.throws(() => { require('../fixtures/es-modules/reference-error-esm.js'); }, { name: 'ReferenceError', - message: 'exports is not defined' + message: 'exports is not defined in ES module scope' }); From 7d95b683d445a07a2d1ed8755b4667770bdf1b9f Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 22 Jan 2025 10:52:17 +0100 Subject: [PATCH 3/3] fixup! module: clarify cjs global-like error on ModuleJobSync --- lib/internal/modules/esm/module_job.js | 75 ++++++++++++-------------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 688144d526f04f..51aa863dd615f5 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -65,6 +65,37 @@ const isCommonJSGlobalLikeNotDefinedError = (errorMessage) => (globalLike) => errorMessage === `${globalLike} is not defined`, ); + +/** + * + * @param {Error} e + * @param {string} url + * @returns {void} + */ +const explainCommonJSGlobalLikeNotDefinedError = (e, url) => { + if (e?.name === 'ReferenceError' && + isCommonJSGlobalLikeNotDefinedError(e.message)) { + e.message += ' in ES module scope'; + + if (StringPrototypeStartsWith(e.message, 'require ')) { + e.message += ', you can use import instead'; + } + + const packageConfig = + StringPrototypeStartsWith(url, 'file://') && + RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, url) !== null && + require('internal/modules/package_json_reader') + .getPackageScopeConfig(url); + if (packageConfig.type === 'module') { + e.message += + '\nThis file is being treated as an ES module because it has a ' + + `'.js' file extension and '${packageConfig.pjsonPath}' contains ` + + '"type": "module". To treat it as a CommonJS script, rename it ' + + 'to use the \'.cjs\' file extension.'; + } + } +}; + class ModuleJobBase { constructor(url, importAttributes, phase, isMain, inspectBrk) { assert(typeof phase === 'number'); @@ -326,27 +357,7 @@ class ModuleJob extends ModuleJobBase { try { await this.module.evaluate(timeout, breakOnSigint); } catch (e) { - if (e?.name === 'ReferenceError' && - isCommonJSGlobalLikeNotDefinedError(e.message)) { - e.message += ' in ES module scope'; - - if (StringPrototypeStartsWith(e.message, 'require ')) { - e.message += ', you can use import instead'; - } - - const packageConfig = - StringPrototypeStartsWith(this.module.url, 'file://') && - RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) !== null && - require('internal/modules/package_json_reader') - .getPackageScopeConfig(this.module.url); - if (packageConfig.type === 'module') { - e.message += - '\nThis file is being treated as an ES module because it has a ' + - `'.js' file extension and '${packageConfig.pjsonPath}' contains ` + - '"type": "module". To treat it as a CommonJS script, rename it ' + - 'to use the \'.cjs\' file extension.'; - } - } + explainCommonJSGlobalLikeNotDefinedError(e, this.module.url); throw e; } return { __proto__: null, module: this.module }; @@ -480,27 +491,7 @@ class ModuleJobSync extends ModuleJobBase { const namespace = this.module.evaluateSync(filename, parentFilename); return { __proto__: null, module: this.module, namespace }; } catch (e) { - if (e?.name === 'ReferenceError' && - isCommonJSGlobalLikeNotDefinedError(e.message)) { - e.message += ' in ES module scope'; - - if (StringPrototypeStartsWith(e.message, 'require ')) { - e.message += ', you can use import instead'; - } - - const packageConfig = - StringPrototypeStartsWith(this.module.url, 'file://') && - RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) !== null && - require('internal/modules/package_json_reader') - .getPackageScopeConfig(this.module.url); - if (packageConfig.type === 'module') { - e.message += - '\nThis file is being treated as an ES module because it has a ' + - `'.js' file extension and '${packageConfig.pjsonPath}' contains ` + - '"type": "module". To treat it as a CommonJS script, rename it ' + - 'to use the \'.cjs\' file extension.'; - } - } + explainCommonJSGlobalLikeNotDefinedError(e, this.module.url); throw e; } }