| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@GeoffreyBooth I know we went through this many times before, but since it keeps coming up I'm happy to go through the same discussions again on the exact wording. |
Sorry, something went wrong.
There was a problem hiding this comment.
nit: %Array.isArray% would be more readable (or maybe stackFrames !== undefined would be enough?).
| if (typeof stackFrames === 'object') { | |
| if (ArrayIsArray(stackFrames)) { |
Sorry, something went wrong.
There was a problem hiding this comment.
This came from a line of code in repl written by @devsnek. Wasn't sure if there was some original reason for it.
Sorry, something went wrong.
|
now that my pr landed, you can remove deferStack, overrideStackTraceByCode, etc., and just put the error in the overrideStackTrace map. |
Sorry, something went wrong.
|
@devsnek working great - I've pushed that up. |
Sorry, something went wrong.
|
Ping @nodejs/modules for further reviewers on the error message improvements here. |
Sorry, something went wrong.
There was a problem hiding this comment.
Seems fine to me, there are cheaper top level parsers, but since we are on an error path, this seems fine.
Sorry, something went wrong.
There was a problem hiding this comment.
do they have to be lazy?
Sorry, something went wrong.
There was a problem hiding this comment.
They don't have to be, but we only hit this path on ERR_REQUIRE_ESM errors. I was following the pattern of other lazy requires used in this module.
Sorry, something went wrong.
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sorry, something went wrong.
PR-URL: #39175 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #39175 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #39175 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
since node 16.6 webpack-cli fails to load esm config unless WEBPACK_CLI_FORCE_LOAD_ESM_CONFIG=true see: nodejs/node#39175 see: https://github.com/webpack/webpack-cli/blob/a660ffcbeb2807bce1554f787297e697464abd59/packages/webpack-cli/lib/webpack-cli.js#L50-L51 License: (Apache-2.0 AND MIT) Signed-off-by: Oli Evans <oli@tableflip.io>
Since node v16.6 webpack-cli fails to load esm config unless WEBPACK_CLI_FORCE_LOAD_ESM_CONFIG=true see: nodejs/node#39175 see: https://github.com/webpack/webpack-cli/blob/a660ffcbeb2807bce1554f787297e697464abd59/packages/webpack-cli/lib/webpack-cli.js#L50-L51 License: (Apache-2.0 AND MIT) Signed-off-by: Oli Evans <oli@tableflip.io>
Since node v16.6 webpack-cli fails to load esm config unless WEBPACK_CLI_FORCE_LOAD_ESM_CONFIG=true see: nodejs/node#39175 see: https://github.com/webpack/webpack-cli/blob/a660ffcbeb2807bce1554f787297e697464abd59/packages/webpack-cli/lib/webpack-cli.js#L50-L51 License: (Apache-2.0 AND MIT) Signed-off-by: Oli Evans <oli@tableflip.io>
In nodejs#39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: nodejs#39175
In #39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: #39175 PR-URL: #49521 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
In #39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: #39175 PR-URL: #49521 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
In nodejs#39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: nodejs#39175 PR-URL: nodejs#49521 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
In #39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: #39175 PR-URL: #49521 Backport-PR-URL: #50669 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
In #39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: nodejs/node#39175 PR-URL: nodejs/node#49521 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
In #39175, better ESM errors were introduced. This commit tweaks the language in the error slightly to make it clear that there are three different options to resolve the error. Refs: nodejs/node#39175 PR-URL: nodejs/node#49521 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
| Back | FazBrowse Home | New Git URL |
This PR attempts to improve the usability of the ERR_REQUIRE_ESM error messages with two major improvements:
In addition the internal frames are stripped from the error stack to remove the redundant internal stack lines to ensure that the output is as simple as possible for this error message.
I've provided two examples below. One for requiring CJS that Node.js thinks is ESM, and one requiring ESM that actually contains import / export syntax.
Requiring ESM with ESM syntax Before this PR:
internal/modules/cjs/loader.js:1080 throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath); ^ Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/guybedford/Projects/node/test.js require() of ES modules is not supported. require() of /home/guybedford/Projects/node/test.js from /home/guybedford/Projects/node/test.cjs is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules. Instead rename test.js to end in .cjs, change the requiring code to use import(), or change "type": "module" to "type": "commonjs" in /home/guybedford/Projects/node/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead). at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) at Module.require (internal/modules/cjs/loader.js:952:19) at require (internal/modules/cjs/helpers.js:88:18) at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) at Module._compile (internal/modules/cjs/loader.js:1063:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) { code: 'ERR_REQUIRE_ESM' }Requiring ESM with ESM syntax after this PR:
/home/guybedford/Projects/node/test.cjs:1 require('./test.js'); ^ Error [ERR_REQUIRE_ESM]: require() of ES Module /home/guybedford/Projects/node/test.js from /home/guybedford/Projects/node/test.cjs not supported. Instead change the require of test.js in /home/guybedford/Projects/node/test.cjs to a dynamic import() which is available in all CommonJS modules. at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) { code: 'ERR_REQUIRE_ESM' }Requiring a .js file with no import or export syntax before this PR:
internal/modules/cjs/loader.js:1080 throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath); ^ Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/guybedford/Projects/node/test.js require() of ES modules is not supported. require() of /home/guybedford/Projects/node/test.js from /home/guybedford/Projects/node/test.cjs is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules. Instead rename test.js to end in .cjs, change the requiring code to use import(), or change "type": "module" to "type": "commonjs" in /home/guybedford/Projects/node/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead). at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) at Module.require (internal/modules/cjs/loader.js:952:19) at require (internal/modules/cjs/helpers.js:88:18) at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) at Module._compile (internal/modules/cjs/loader.js:1063:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) { code: 'ERR_REQUIRE_ESM' }Requiring a .js file with no import or export syntax after this PR:
/home/guybedford/Projects/node/test.cjs:1 require('./test.js'); ^ Error [ERR_REQUIRE_ESM]: require() of ES Module /home/guybedford/Projects/node/test.js from /home/guybedford/Projects/node/test.cjs not supported. test.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules. Instead rename test.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /home/guybedford/Projects/node/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead). at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) { code: 'ERR_REQUIRE_ESM' }Requiring a .mjs file before this PR:
internal/modules/cjs/loader.js:926 throw new ERR_REQUIRE_ESM(filename); ^ Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/guybedford/Projects/node/test.mjs at Module.load (internal/modules/cjs/loader.js:926:11) at Function.Module._load (internal/modules/cjs/loader.js:769:14) at Module.require (internal/modules/cjs/loader.js:952:19) at require (internal/modules/cjs/helpers.js:88:18) at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) at Module._compile (internal/modules/cjs/loader.js:1063:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10) at Module.load (internal/modules/cjs/loader.js:928:32) at Function.Module._load (internal/modules/cjs/loader.js:769:14) at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) { code: 'ERR_REQUIRE_ESM' }Requiring a .mjs file after this PR:
node:internal/modules/cjs/loader:978 throw new ERR_REQUIRE_ESM(filename, true); ^ Error [ERR_REQUIRE_ESM]: require() of ES Module /home/guybedford/Projects/node/test.mjs not supported. Instead change the require of /home/guybedford/Projects/node/test.mjs to a dynamic import() which is available in all CommonJS modules. at Object.<anonymous> (/home/guybedford/Projects/node/test.cjs:1:1) { code: 'ERR_REQUIRE_ESM' }@nodejs/modules