esm: refine ERR_REQUIRE_ESM errors #39175

guybedford · 2021-06-27T22:47:11Z

This PR attempts to improve the usability of the ERR_REQUIRE_ESM error messages with two major improvements:

When the module being required contains parseable ESM syntax, constrain the suggested fix to the message "Instead change the require of X.js in Y.js to a dynamic import() which is available in all CommonJS modules." which avoids confusion since we currently suggest all three of that plus changing the type and the extension to cjs, both of which are not appropriate in this case. This is implemented by using the acorn parser on the source at error time.
Ensure that the error mesage frame points to the exact location of the require. This was quite a bit of work to implement and involved a new primitive for exposing the arrow_message_private_symbol to JS so that JS code can inject this frame. The code to reconstruct the frame is more custom than it should be - ideally this could use the standard stack trace API with full source maps integration and be customizable in a way that fits with the stack trace overrides. I hope we can continue to improve this primitives over time to get better error messages more easily in Node.js.

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

guybedford · 2021-06-27T22:48:33Z

@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.

lib/internal/errors.js

lib/internal/modules/cjs/helpers.js

lib/internal/errors.js

aduh95 · 2021-06-28T08:15:00Z

lib/internal/errors.js

+// Hide stack lines before the first user code line.
+function hideLeadingInternalFrames(error, stackFrames) {
+  let frames = stackFrames;
+  if (typeof stackFrames === 'object') {


nit: %Array.isArray% would be more readable (or maybe stackFrames !== undefined would be enough?).

Suggested change

if (typeof stackFrames === 'object') {

if (ArrayIsArray(stackFrames)) {

This came from a line of code in repl written by @devsnek. Wasn't sure if there was some original reason for it.

lib/internal/errors.js

lib/internal/modules/cjs/helpers.js

src/node_errors.cc

devsnek · 2021-07-08T17:18:34Z

now that my pr landed, you can remove deferStack, overrideStackTraceByCode, etc., and just put the error in the overrideStackTrace map.

guybedford · 2021-07-08T17:36:11Z

@devsnek working great - I've pushed that up.

Feedback merged in separate PR.

guybedford · 2021-07-08T18:04:48Z

Ping @nodejs/modules for further reviewers on the error message improvements here.

bmeck

Seems fine to me, there are cheaper top level parsers, but since we are on an error path, this seems fine.

targos · 2021-07-09T13:34:42Z

lib/internal/errors.js

@@ -788,6 +789,34 @@ const fatalExceptionStackEnhancers = {
  }
 };

+// Ensures the printed error line is from user code.
+let _kArrowMessagePrivateSymbol, _setHiddenValue;


do they have to be lazy?

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.

test/es-module/test-cjs-esm-warn.js

lib/internal/modules/cjs/loader.js

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

nodejs-github-bot · 2021-07-15T13:32:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39069/

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>

guybedford · 2021-07-15T15:52:53Z

Landed in e2a6399.

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>

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.

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 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>

github-actions bot added c++ Issues and PRs that require attention from people who are familiar with C++. errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. labels Jun 27, 2021

guybedford force-pushed the require-esm-improvements branch from ff5d5f7 to 1f59c8f Compare June 27, 2021 22:50

aduh95 reviewed Jun 27, 2021

View reviewed changes

lib/internal/errors.js Outdated Show resolved Hide resolved

aduh95 reviewed Jun 27, 2021

View reviewed changes

lib/internal/errors.js Outdated Show resolved Hide resolved

devsnek previously requested changes Jun 28, 2021

View reviewed changes

lib/internal/errors.js Outdated Show resolved Hide resolved

ljharb reviewed Jun 28, 2021

View reviewed changes

lib/internal/errors.js Outdated Show resolved Hide resolved

lib/internal/modules/cjs/helpers.js Outdated Show resolved Hide resolved

aduh95 reviewed Jun 28, 2021

View reviewed changes

addaleax reviewed Jun 28, 2021

View reviewed changes

src/node_errors.cc Outdated Show resolved Hide resolved

aduh95 mentioned this pull request Jun 28, 2021

Improve documentation on Dual Module Packages #34515

Open

guybedford force-pushed the require-esm-improvements branch from d41671c to 8be551d Compare July 8, 2021 17:14

guybedford requested a review from devsnek July 8, 2021 17:15

bmeck approved these changes Jul 8, 2021

View reviewed changes

targos reviewed Jul 9, 2021

View reviewed changes

targos mentioned this pull request Jul 12, 2021

Detect require vs. import based on the entry point, not package.json #39353

Closed

aduh95 reviewed Jul 12, 2021

View reviewed changes

test/es-module/test-cjs-esm-warn.js Outdated Show resolved Hide resolved

test/es-module/test-cjs-esm-warn.js Outdated Show resolved Hide resolved

lib/internal/modules/cjs/loader.js Outdated Show resolved Hide resolved

lib/internal/modules/cjs/loader.js Outdated Show resolved Hide resolved

guybedford and others added 8 commits July 12, 2021 14:58

esm: refine ERR_REQUIRE_ESM errors

15babc4

fixup: space

8b6d578

fixup: arrow fn

0d129ba

fixup: Apply suggestions from code review

7765a14

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

fixup: pr feedback

187c7b9

fixup: errors linting

8fbf23b

fixup: simplify internal stack frame hiding

fdd9b31

fixup: tests

af548b5

This comment has been minimized.

Sign in to view

fixup: lazily load acorn

b156244

This comment has been minimized.

Sign in to view

fixup: bootstrap list

cf5879f

This comment has been minimized.

Sign in to view

guybedford added a commit that referenced this pull request Jul 15, 2021

esm: refine ERR_REQUIRE_ESM errors

e2a6399

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>

guybedford closed this Jul 15, 2021

guybedford deleted the require-esm-improvements branch July 15, 2021 15:52

targos pushed a commit that referenced this pull request Jul 17, 2021

esm: refine ERR_REQUIRE_ESM errors

cb105b2

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>

aduh95 mentioned this pull request Jul 24, 2021

module: refine enrichCJSError #39507

Closed

BethGriggs mentioned this pull request Jul 26, 2021

v16.6.0 proposal #39534

Merged

BethGriggs pushed a commit that referenced this pull request Jul 29, 2021

esm: refine ERR_REQUIRE_ESM errors

93eff3f

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>

olizilla mentioned this pull request Jul 30, 2021

fix: force webpack to load ESM config web3-storage/web3.storage#247

Merged

olizilla mentioned this pull request Jul 31, 2021

TypeError loading webpack.config.js in ESM project with node v16.6 webpack/webpack-cli#2864

Closed

targos added the backport-blocked-v14.x label Sep 4, 2021

danielcompton mentioned this pull request Sep 6, 2023

esm: clarify ERR_REQUIRE_ESM errors #49521

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

esm: refine ERR_REQUIRE_ESM errors #39175

esm: refine ERR_REQUIRE_ESM errors #39175

guybedford commented Jun 27, 2021 •

edited

Loading

guybedford commented Jun 27, 2021

aduh95 Jun 28, 2021

guybedford Jun 28, 2021

devsnek commented Jul 8, 2021

guybedford commented Jul 8, 2021

guybedford commented Jul 8, 2021

bmeck left a comment •

edited

Loading

targos Jul 9, 2021

guybedford Jul 12, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jul 15, 2021

guybedford commented Jul 15, 2021

	if (typeof stackFrames === 'object') {
	if (ArrayIsArray(stackFrames)) {

esm: refine ERR_REQUIRE_ESM errors #39175

esm: refine ERR_REQUIRE_ESM errors #39175

Conversation

guybedford commented Jun 27, 2021 • edited Loading

guybedford commented Jun 27, 2021

aduh95 Jun 28, 2021

Choose a reason for hiding this comment

guybedford Jun 28, 2021

Choose a reason for hiding this comment

devsnek commented Jul 8, 2021

guybedford commented Jul 8, 2021

guybedford commented Jul 8, 2021

bmeck left a comment • edited Loading

Choose a reason for hiding this comment

targos Jul 9, 2021

Choose a reason for hiding this comment

guybedford Jul 12, 2021

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jul 15, 2021

guybedford commented Jul 15, 2021

guybedford commented Jun 27, 2021 •

edited

Loading

bmeck left a comment •

edited

Loading