Fix index fallback of CJS package from ESM-mode import when main is present but does not resolve
#49327
Conversation
… present but does not resolve
typescript-bot
added
the
For Uncommitted Bug
|
If @weswigham agrees this is a bug / this or something close to it is a good fix, we should consider it for a 4.7 patch as there could be a bunch of old DT packages like this that are broken with nodenext. |
| && packageInfo.packageJsonContent.exports === undefined | ||
| && packageInfo.packageJsonContent.main === undefined | ||
| // eslint-disable-next-line no-null/no-null | ||
| && (packageInfo.packageJsonContent.exports === undefined || packageInfo.packageJsonContent.exports === null) |
There was a problem hiding this comment.
Double checking on https://nodejs.org/api/esm.html#resolution-algorithm, recognizing both null and undefined here for exports seems good (only one usage makes that explicit, but it should be fine), however ignoring main and looking up an index regardless is not what PACKAGE_RESOLVE describes for an exportless package... it just says Return the URL resolution of packageSubpath in packageURL., which doesn't imply any fallback to index.js lookup if the main fails. If in practice it is, the algorithm in the node docs may be wrong. (Which doesn't surprise me, if I'm honest, I think the people writing the code are more pragmatic than the people reviewing/writing the docs.)
There was a problem hiding this comment.
Indeed, there's a large legacy main resolution code block which isn't included in the documented algorithm.
There was a problem hiding this comment.
I've opened nodejs/node#43269 for the docs issue on node.
|
@typescript-bot cherry-pick this to release-4.7 |
|
Heya @andrewbranch, I've started to run the task to cherry-pick this into |
|
Hey @andrewbranch, I've opened #49329 for you. |
Component commits: 79957c1 Fix index fallback of CJS package from ESM-mode import when `main` is present but does not resolve
Component commits: 79957c1 Fix index fallback of CJS package from ESM-mode import when `main` is present but does not resolve Co-authored-by: Andrew Branch <andrew@wheream.io>
Fixes #49326
To check that this is consistent with Node’s behavior, I tried these package.json modifications in
@types/dedentand added anindex.jsofmodule.exports = "index.js"and ranand ensured both log
index.js. If they did, I assumed that means we should be able to resolve toindex.d.ts.