fix: Duplicate declarations #79
Conversation
hook.js
Outdated
| @@ -180,7 +180,7 @@ async function processModule ({ | |||
| // needs to utilize that new name while being initialized from the | |||
| // corresponding origin namespace. | |||
| const renamedExport = matches[2] | |||
| setters.set(`$${renamedExport}${ns}`, ` | |||
| setters.set(`$${renamedExport}`, ` | |||
There was a problem hiding this comment.
Does this change invalidate the preceding comment, which is saying:
- If
a.mjsexports entities fromb.mjs - And
b.mjsexports a default entity - The default entity exported by
b.mjsshould be namedbunder the namespace associated withb.mjs
There was a problem hiding this comment.
Not sure, need to check that the tests cover this!
There was a problem hiding this comment.
Why does iitm have this namespacing of default exports behaviour? It means that iitm adds additional exports that don't exist without iitm in use. I can see this was added in #53, but why is this desirable?
I assumed we should be exporting exactly the same exports as when the loader is not in use?
> node ./test/hook/v18.19-static-import-gotalike.mjs
file:///Users/tim/Documents/Repositories/import-in-the-middle/test/hook/v18.19-static-import-gotalike.mjs:22
defaultClass as DefaultClass,
^^^^^^^^^^^^
SyntaxError: The requested module '../fixtures/got-alike.mjs' does not provide an export named 'defaultClass'
at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)
Node.js v22.2.0
There was a problem hiding this comment.
Why does iitm have this namespacing of default exports behaviour?
IITM basically has to re-implement ESM loading from scratch because it needs to be able to wrap up any of the entities exported by a module. But transitive modules can export entities with the same identifiers as the module that loaded the transitive dependency (or any ancestor module for that matter). This, combined with star exports, means we have to figure out which one is which in some sort of way. The way chosen is a namespace per module.
- Support
export * from 'module'ESM syntax #43 added support for handling star exports - Fix duplicate default exports #53 attempts to fix the duplicates exports made possible by 1
All of this could be avoided if we had an API where Node does all of the ESM loading and merely gives us back a malleable set of exports.
There was a problem hiding this comment.
All of this could be avoided if we had an API where Node does all of the ESM loading and merely gives us back a malleable set of exports.
Yeah I wish for this too. Is this already being addressed as part of nodejs/node#52219, or do we need to push for a separate proposal?
There was a problem hiding this comment.
@AbhiPrasad yes, that proposal is what we need to participate in to get the API we would like.
timfish
force-pushed
the
fix-duplicate-export
branch
from
43593e0
to
b0e4e4c
Compare
|
Closing favour of #87 |
Closes #60