-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't import a hybrid ESM + CommonJS package from CommonJS using Node16 resolution #54620
Comments
This is almost always a misconfiguration on the part of the package you're importing (usually using the same Also see https://github.com/arethetypeswrong/arethetypeswrong.github.io |
In this case the package that I'm importing is defined in the same sandbox, and definitely has a separate .d.cts file |
For putative module resolution bugs, we'll need an exact breakdown of why you think something is provably wrong. We've ironed out basically everything here and the configuration space is extremely large (thus the odds of a misconfiguration are very high). See also (WIP) module docs https://gist.github.com/andrewbranch/79f872a8b9f0507c9c5f2641cfb3efa6 |
@RyanCavanaugh happy to provide any more info that's required. I actually just read those docs as well. Basically what I'm seeing is an inability to import a hybrid module with a package.json specified (as far as I can tell) identically to what's specified in the current docs: // package.json
{
"name": "my-package",
"type": "module",
"exports": {
".": {
// Entry-point for `import "my-package"` in ESM
"import": {
// Where TypeScript will look.
"types": "./types/esm/index.d.ts",
// Where Node.js will look.
"default": "./esm/index.js"
},
// Entry-point for `require("my-package")` in CJS
"require": {
// Where TypeScript will look.
"types": "./types/commonjs/index.d.cts",
// Where Node.js will look.
"default": "./commonjs/index.cjs"
},
}
},
// Fall-back for older versions of TypeScript
"types": "./types/index.d.ts",
// CJS fall-back for older versions of Node.js
"main": "./commonjs/index.cjs"
} When consuming this package from CommonJS code using |
🐽 |
Update: I think I've figured out some more details of what's going on here. In my original example, the package Locally if I take my example and move the Edit: just to clarify the above a bit more, this works:
with // app.ts
import { test } from 'hybrid' but this doesn't:
// app.ts
import { test } from './hybrid' However, a transpiled commonjs version of |
If you have
then the correct corresponding entry in package.json is
which works as expected. |
@RyanCavanaugh gotcha, in that case does this example in the docs need to be updated? |
Indeed all of the docs need updating 😅 |
@RyanCavanaugh I opened a PR here to make that change: however I realized in doing so, that if the purpose of this field is for "old" versions of Typescript, wouldn't it fail if that version of Typescript does not recognize |
I am now having issues with many many packages that seem to have this configured wrong. When using
and
It seems like the impact could be significantly breaking many (probably misconfigured) npm packages. So far I have run into this with
|
Indeed, the example on the website is correct/intentional for this reason. The website example assumes that the package described by the package.json will be loaded from inside somebody’s node_modules by a non-relative import, e.g. The point of the example on the website is that |
@andrewbranch that all makes sense, though I find it unfortunate in Node's case that these two cases should be different. Thanks for writing those new docs by the way, it's the clearest explanation I've found about this whole situation. If it isn't already called out in there, I think this difference in how It may also be helpful to describe that this is the case being hit in the error message. The real issue I ran into here is when attempting to use newer versions of Typescript in an Nx monorepo. However, each package is meant to be published to NPM, so when compiled and published that path which was previously an aliased "path" now becomes a literal import for the same-named package. Eg. there's a path alias for Even as I'm writing this I realize it's a gigantic edge case, so I'll probably need to go take it up with Nx. |
Yeah, |
Bug Report
🔎 Search Terms
esm commonjs node16 hybrid resolution
🕗 Version & Regression Information
⏯ Playground Link
CodeSandbox
I'm seeing this behaviour locally with Typescript 5.1, but Codesandbox doesn't seem to have that version as an option. Might need to download locally to see the problem.
💻 Code
See the Codesandbox
🙁 Actual behavior
The line in test.js that imports from the
hybrid
module gives this compiler error:(behaviour on the Codesandbox is different, I can't set the version to Typescript 5.1, might need to download locally to see the problem)
🙂 Expected behavior
In this setup I have a module
hybrid
which declares itself as an ESModule (type: module
) in its package.json. However, it also exports commonjs javascript and types, using the approach described in this documention:https://www.typescriptlang.org/docs/handbook/esm-node.html#commonjs-interop
The module and moduleResolution settings of the outer
.ts
file which imports this package isnode16
. I expected based on that documentation that Typescript would be able to resolvehybrid
asCommonJS
because of the package.json setup, but it seems to insist that it's an ESModule. If I change the "types" field of the package.json to the.d.cts
file, then resolution works, but I imagine it now wouldn't work when importing from an ESModule. The documentation also specifies that it should be.d.ts
, and also that it's only there for the benefit of older versions of Typescript. With that in mind, I also tried removing the line entirely and that broke resolution again.Not sure if this is a bug, a documentation issue, or something I'm doing wrong with the setup.
The text was updated successfully, but these errors were encountered: