Review requested:

• @nodejs/loaders

cc @JakobJingleheimer @jsumners

There is still one case that I think should be covered by this message highlighted by https://github.com/nodejs/node/blob/main/test/es-module/test-esm-detect-ambiguous.mjs#L212 where an import cjs.js in a package with type: module has the same behaviour but doesn't trigger the message as the error is caught by the module that is importing it, which could be mjs or cjs

For my own clarification: why am I tagged on this one?

For my own clarification: why am I tagged on this one?

Not sure if I tagged wrong. I saw this repro on the nodejs loaders group https://github.com/jsumners-nr/exports-bug

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.23%. Comparing base (b197355) to head (7d95b68).
Report is 13 commits behind head on main.

@@           Coverage Diff           @@
##             main   #56491   +/-   ##
=======================================
  Coverage   90.23%   90.23%           
=======================================
  Files         633      633           
  Lines      186818   186834   +16     
  Branches    36668    36673    +5     
=======================================
+ Hits       168578   168595   +17     
- Misses      11036    11038    +2     
+ Partials     7204     7201    -3     

For my own clarification: why am I tagged on this one?

@jsumners you sort of requested this in slack about a month ago :)

For my own clarification: why am I tagged on this one?

Not sure if I tagged wrong. I saw this repro on the nodejs loaders group https://github.com/jsumners-nr/exports-bug

Thank you. Please tag @jsumners-nr instead. That is my work account.

I presume you're asking so it will give you a reminder (which it won't do when you tag yourself). @jsumners-nr :)

I participate in the loaders group through my work persona. I will review when I am back at work tomorrow. Technically, I wouldn't be tagging myself since it is separate accounts. I'm really just trying to make it clear which account to use for these things.

Can we move this forward? @JakobJingleheimer
Did you have time to check it? @jsumners-nr

Can we move this forward? @JakobJingleheimer Did you have time to check it? @jsumners-nr

I was out all last week due to illness.

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

You can remove the TODO in L395 now because that's talking about...this :)

The other TODO relies on the V8 13.0 upgrade which is blocked on...the macOS upgrade in the CI, sigh.

It seems the two failing test are because --experimental-permission is no longer experimental and test-runner-module-mocking.js doesn't exist (maybe is test-runner-mocking.js?) I tried rebasing locally but didn't work so not sure what to do

Those two came from security release hiccups and should be fixed by 01dd7c4 - not sure about whether the GitHub actions are able to rebase. Can you rebase locally and force push here?

Done

Do we need anything else to move this forward?

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

Landed in b6189c3