Dynamic import() in a CJS file can execute a module twice

[qraynaud]

Acknowledgements

• I searched existing issues to avoid duplicates
• I understand this is a place to report a confirmed bug (not seek debugging help)
• I understand this is a collaborative open source project, and relies on community contributions
• I have read and understood the Contribution guide

Minimal reproduction URL

https://stackblitz.com/edit/stackblitz-starters-hfn7ts

Version

4.7.1

Node.js version

18.18.0 and 20.11.0

Package manager

npm

Operating system

Linux

Problem & expected behavior (under 200 words)

• What happened

I’m in the process of migrating an app from CJS to ESM. This app contains some dynamic imports. The files dynamically imported are in part all the mongoose models to collect data on them and to basic checks. As soon as I started changing the extension of some models from .ts to .mts I had issues with mongoose reporting that I'm trying to register the same model twice. I made a simple repro in the provided stackblitz.

• What I expected

You have 2 files in my minimun repro. One works.mts and one bugs.ts. They both basically contain the same code. Only the extension changes. It should execute the module.mts file alongside them only once. But in one case it is executed twice.

Contributions

• I will open a pull request for this issue
• I will support the work financially (any amount helps)

[privatenumber]

This is definitely unexpected.

What's happening is the first import statement gets converted to require(), and since require can't load Module (.mjs) files, it transforms them to CommonJS so it can be. The second dynamic import() remains untouched.

So the first and second files are technically different modules, where the first one is dynamically derived on demand.

This is a technical limitation but it will be resolved once nodejs/node#51977 lands

[qraynaud]

I understand that you keep the dynamic import, but since it’s using mts behind, it means that at some point tsx get to transpile it "again" right? So in this instance you do transpile it as an ESM module instead of as a CJS modules as the first time right? Could you not detect that it was previously loaded as a CJS and return the CJS version instead (since node do support import() on CJS files)? Or return to node a fake module that does export the required CJS module?

Even waiting for nodejs/node#51977 would not really resolve the issue since this patch does not support requiring modules with top level async in them.

Replacing the import() with a require() implicitely would end badly for a lot of files.
And if you replace it with a try/catch to fallback to import(), then the bug I'm reporting would still be there for those.

[privatenumber]

I'm not sure, but feel free to give it a tackle. I'd be happy to review the PR!