Wrong error annotation when commonjs `require`s an ES module

targos commented 3 weeks ago

Version

^22.4.0
main with the --no-experimental-require-module flag.

Platform

macOS arm64

Subsystem

esm,module

What steps will reproduce the bug?

mkdir undefined && cd undefined
echo '{"type":"module"}' > package.json
echo "import nothing from 'somewhere'" > app.js
echo "require('./app.js')" > test.cjs
node test.cjs

How often does it reproduce? Is there a required condition?

It starts to happen with v22.4.0. v20.x and <=22.3.0 are not affected.

What is the expected behavior? Why is that the expected behavior?

$ node test.cjs
/Users/mzasso/git/test/undefined/test.cjs:1
require('./app.js')
^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/mzasso/git/test/undefined/app.js from /Users/mzasso/git/test/undefined/test.cjs not supported.
Instead change the require of app.js in /Users/mzasso/git/test/undefined/test.cjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/mzasso/git/test/undefined/test.cjs:1:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v22.3.0

What do you see instead?

$ node test.cjs
/Users/mzasso/git/test/undefined/test.cjs:315
undefined
             ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/mzasso/git/test/undefined/app.js from /Users/mzasso/git/test/undefined/test.cjs not supported.
Instead change the require of app.js in /Users/mzasso/git/test/undefined/test.cjs to a dynamic import() which is available in all CommonJS modules.
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at Object.<anonymous> (/Users/mzasso/git/test/undefined/test.cjs:1:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v22.4.0

Additional information

As you can see, this seems to be caused by the presence of TracingChannel.traceSync in the stack trace.

targos commented 3 weeks ago

Probably caused by https://github.com/nodejs/node/pull/44340 @Qard

Qard commented 3 weeks ago

@RafaelGSS is the one that finished and landed that PR, so they would probably know better what is going on here.

RafaelGSS commented 1 week ago

Sorry, I've just seen it. I'll check when I get some time (currently traveling for 2 weeks)

geeksilva97 commented 5 days ago

Would anybody guide me to where this part of the message,

/Users/mzasso/git/test/undefined/test.cjs:1
require('./app.js')
^

, is generated?

I thought it was https://github.com/nodejs/node/blob/51eb4c0cdaf6257af4302e321dec2dd9cf28c5cb/src/node_errors.cc#L137 but I'm wrong

geeksilva97 commented 5 days ago

Would anybody guide me to where this part of the message,
/Users/mzasso/git/test/undefined/test.cjs:1
require('./app.js')
^
, is generated?

I thought it was

https://github.com/nodejs/node/blob/51eb4c0cdaf6257af4302e321dec2dd9cf28c5cb/src/node_errors.cc#L137

but I'm wrong

Found it. https://github.com/nodejs/node/blob/main/lib/internal/modules/cjs/loader.js#L1656

nodejs / node