bug: CJS exports analysis reads directly from disk

[privatenumber]

I originally filed in nodejs/node#50649 but upon reading the Loaders Roadmap, I think it may be more appropriate to be filed here. This is a bug report that will likely be fixed by the following Milestone 3 item:

• Helper/utility functions to allow access to the CommonJS named exports discovery algorithm (cjs-module-lexer).

Reproduction

https://stackblitz.com/edit/stackblitz-starters-j9tcmc?file=index.mjs

The reproduction calls node --require ./require-hook.cjs index.mjs.

For the sake of the reproduction, require-hook.cjs completely overwrites the code of file.js. (In practice, tsx transforms the ESM export syntax to CJS exports)

But despite that, when importing file.js from index.mjs, the removed export is still present.

This is happening because Node's CJS exports parser bypasses the CJS loader and directly analyzes the disk file. Seems this bug is acknowledged here.

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

Utilizing a CommonJS require hook to compile ESM syntax in a .js file in a CommonJS context, then importing the file from a .mjs file

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

For Node to parse the named exports from the source retrieved by the loader

What do you see instead?

Node should parse the named exports from the source retrieved by the loader.

Additional information

This limitation is acknowledged in the source code as a TODO here:
https://github.com/nodejs/node/blob/a9b2535/lib/internal/modules/esm/translators.js#L421-L424

While it might be considered an edge case, I'm filing this issue to highlight its impact on tsx users:

• Unable to import a synthetic named export in a CJS file from ESM privatenumber/tsx#67
• Does not import correctly: .ts file > .mjs package > type: "module" package privatenumber/tsx#334
• Cannot import named export from a typescript file in a workspace package if entry is using es modules and other package is not. privatenumber/tsx#38

[aduh95]

Well this seems indeed to be a duplicate of nodejs/node#50649, and is hardly related to loaders, right? If you use the loaders API, it might help for this specific issue – but of course it has some limitations and would need some work (PRs welcome).

Closing as duplicate

[privatenumber]

The bug is with the ESM loader not properly detecting CJS exports (so I think it's kinda related to loaders...).

Given the Loaders milestones mentions improving CJS exports discovery, I felt like it may be worth moving the issue here. Didn't mean to open a duplicate issue. Anyway, I trust your judgement in triaging this. Thanks.

[GeoffreyBooth]

The bug is with the ESM loader not properly detecting CJS exports (so I think it's kinda related to loaders...).

All bug reports go to the main Node repo; this repo is for feature discussion and design. And if the bug occurs when no hooks are registered, it's a core Node issue and not something related to this.

[guybedford]

Supporting interception of the exports analysis as part of the CJS hooking sounds to me like a sensible design. Such hooking should use the CJS source load hook to do the exports analysis against that CJS source, and this hooking could also permit modifying the CJS exports detection algorithm.

[GeoffreyBooth]

Maybe the load hook could have an additional return property? So like in addition to source, for format: 'commonjs' it could also return the list of named exports (or whatever data cjs-module-lexer returns).

[aduh95]

Supporting interception of the exports analysis as part of the CJS hooking sounds to me like a sensible design. Such hooking should use the CJS source load hook to do the exports analysis against that CJS source

That’s already what’s happening

[privatenumber]

FYI I confirmed CJS exports analysis is correctly detecting from the load result with the Hooks API (now that it supports loading CJS): nodejs/node#50649 (comment)

It would still nice to see this bug resolved in the ESM loader though. Open issue: nodejs/node#50649