Skip to content
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

TypeScript module "Node16" does not resolve types of CJS module #49160

Closed
LaurensRietveld opened this issue May 18, 2022 · 13 comments
Closed

TypeScript module "Node16" does not resolve types of CJS module #49160

LaurensRietveld opened this issue May 18, 2022 · 13 comments
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@LaurensRietveld
Copy link

LaurensRietveld commented May 18, 2022

Bug Report

🔎 Search Terms

NodeNext, esm, CJS, colors

🕗 Version & Regression Information

  • This only occurs in 4.7 and 4.8 builds
  • I was unable to test this on prior versions because node16/nodenext is only supported in the nightly versions.

⏯ Playground Link

I didn't include a playground link, as I can't reference dependencies there.
See here for an MWE.
To test:

yarn install
yarn tsc

💻 Code

import colors from "colors";
console.log(colors)

🙁 Actual behavior

TypeScript cannot find the typings for some CommonJS packages such as colors and form-data-encoder

As a result, I'm getting this error:

src/index.ts:1:20 - error TS7016: Could not find a declaration file for module 'colors'. '/home/lrd900/triply/playground/cjs/node_modules/colors/lib/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/colors` if it exists or add a new declaration (.d.ts) file containing `declare module 'colors';`

1 import colors from "colors";
                     ~~~~~~~~


Found 1 error in src/index.ts:1

🙂 Expected behavior

ESM module behaviour to import CJS as it used to.

Related

#47848

@scott-lc
Copy link

scott-lc commented May 18, 2022

To expand on this, TypeScript 4.7.1-rc with nodenext module resolution is unable to resolve various NPM dependencies. See this example repo.

https://github.com/scott-lc/tsc-nodenext

npm install
npm run build

With module and moduleResolution set to nodenext, it won't compile:

src/test.ts:2:31 - error TS7016: Could not find a declaration file for module 'is-plain-object'. '/Users/ssadler/Code/livecontrol.io/tsc-nodenext/node_modules/is-plain-object/dist/is-plain-object.mjs' implicitly has an 'any' type.
  Try `npm i --save-dev @types/is-plain-object` if it exists or add a new declaration (.d.ts) file containing `declare module 'is-plain-object';`

2 import { isPlainObject } from "is-plain-object";
                                ~~~~~~~~~~~~~~~~~

src/test.ts:7:17 - error TS2351: This expression is not constructable.
  Type 'typeof import("/Users/ssadler/Code/livecontrol.io/tsc-nodenext/node_modules/decimal.js/decimal")' has no construct signatures.

7 console.log(new Decimal(123.4567));
                  ~~~~~~~

Setting module to esnext and moduleResolution to node (as in pre-4.7), works fine.

@scott-lc
Copy link

colors does not have types defined in its package.json file. I believe the compiler will fallback to looking for a .d.ts file in the same directory as the main file, which in this case is lib/index.d.js:

{
    "main": "lib/index.js",
}

Unfortunately in the colors package, the corresponding "index.d.ts" is at the root of the repository, not at ./lib/index.d.js. Either 1) adding an explicit types property to the package.json or 2) moving the types to ./lib/index.d.js "fixes" the problem.

Seeing that colors has not been updated in 3 years but still gets 21M downloads a week means that a lot of developers are going to run into this issue. Should the TypeScript compiler support this use case or is it going to be up to the Node/NPM community to update existing packages that don't exactly conform to the nodenext type definition specification?

@ulrichb
Copy link

ulrichb commented May 25, 2022

I've also declaration reference issues for external CommonJS packages after switching to module: node16 (with TS 4.7.2).

Namely telegraf and form-data-encoder, but these packages have explicit "types" references to .d.ts in their package.json files.

Can also reproduce @LaurensRietveld's issue with import colors from "colors".

Other CommonJS packages with inline declarations (e.g. import del from "del") work on the other hand...

@LaurensRietveld LaurensRietveld changed the title TypeScript module "Node16" does not resolve index.d.ts in root of CJS module TypeScript module "Node16" does not resolve types of CJS module May 25, 2022
@LaurensRietveld
Copy link
Author

Thanks @ulrichb

  • I've updated the issue description and made it a bit less opinionated (as you showed that this isn't just about a missing types field). I also added the form-data-encoder package in the MWE
  • I also updated the MWE so that we're using the latest 4.7 release

@andrewbranch
Copy link
Member

Every one of the packages mentioned here is misconfigured. You already figured out the issue with colors. (Actually, I’m a little surprised that colors even works with --moduleResolution node.) Both form-data-encoder and telegraf have exports pointing to lib/index.js but no corresponding types for that entrypoint. Note that types corresponds to main only. main/types will be completely ignored by Node/TypeScript if exports is defined.

While node16/nodenext was in preview over the last two releases, we made an effort to track down some of the most popular npm packages that have TypeScript typings and package.json exports and audit them for correctness, but it was inevitable that many more were going to go through a (hopefully brief) period of pain as more people start to use this mode. This is a big part of the reason we held node16/nodenext in preview—we were hoping people would start to try it, ask why their dependencies weren’t working, and we could help get a lot of this straightened out before we shipped the stable release. Please do file issues/PRs against these libraries; as some of the earliest adopters of this mode, you can really help make this smoother for the folks who start trying it out in the coming weeks and months. Thanks!

@andrewbranch andrewbranch added the External Relates to another program, environment, or user action which we cannot control. label May 25, 2022
@LaurensRietveld
Copy link
Author

Thanks Andrew, I get the rationale and the choice not to support incorrect metadata. The effect is a pity though, considering some packages (such as the colors package for known reasons) are not expected to be actively maintained.
If I understand correctly then we can consider this a "won't fix" (on typescript's side) and close this issue

@LaurensRietveld
Copy link
Author

A note for those using the colors package with yarn2+: the yarn patch functionality also works on package.json files. Depending on your usecase, this may be a way out when using dependencies like colors

@sorgloomer
Copy link

@LaurensRietveld Shouldn't we reopen this issue until the universe (including colors) gets migrated?

@kwasimensah
Copy link

kwasimensah commented Aug 16, 2022

I just ran into this with errlop@4.5.0 and commander@9.1.0

I'd actually disagree with @andrewbranch's statement at #49160 (comment) . I think projects like errlop and commander were properly specified at the time they released those specific versions. Without the ability in open-source to migrate all packages that specify types, this is probably going to continue to be painful until a fallback is added that works with the current state of the world

@shadowspawn
Copy link

(Commander was updated in v9.2.0 with support for the new resolution approach.)

@andrewbranch
Copy link
Member

I didn’t say they were always misconfigured, but they’re misconfigured now 🤷. They opted to use package.json exports before TypeScript supported it, so they were kinda broken as soon as they did that, but with no real way to remedy it yet. Now that we do support it, it’s not surprising that some updates will be needed. Nobody is at fault for the misconfiguration, but there’s simply nothing we can do on our end that won’t break other stuff. If we add additional guesses/fallbacks, those will absolutely produce incorrect resolutions some of the time, and people will get incorrect type info and missing errors on stuff that will crash at runtime.

Also, this is not known widely enough: all any package author has to do is put their .d.ts files in the same directories as their .js files, just like tsc emits them by default. The only packages that are dealing with issues are ones that put their types in a separate folder for some reason. .d.ts and .js files are best friends. If you don’t separate them, you literally don’t have to put anything special at all in the package.json.

@kwasimensah
Copy link

But I also want to make sure that we have empathy for why people were using exports already. It seems to be the only way to support commonjs and esm in the same package. And I can see how it super desirable to make it easier for your package to be used in both node and the browser.

And I also think relying on the layout of where the .d.ts file is relative to the .js would lead to bloat/repetitiveness if you you’re trying to support both in the same package.

@Qix-
Copy link

Qix- commented Oct 5, 2022

The concept of "misconfigured" is preposterous given that there is no standard for package.json (the community has been asking for one for years and NPM refuses, and now there are too many hands in the cookie jar and none of them seem to want to work together).

So the idea that something in package.json is "misconfigured" because it's old is simply wrong. If it worked before, then it's not misconfigured. An ecosystem tool has just chosen to break them.

After 10+ years in the Node scene the package.json nightmare has only gotten worse. I'm not sure what the solution is, but the fact is that node16 breaks imports in most non-trivial cases I've tried it.

e1himself added a commit to prezly/slate that referenced this issue May 7, 2023
To keep `.d.ts` files next to their JS code versions.
This will make the packages play well with TS 5.0.
See microsoft/TypeScript#49160 (comment)
wolfy1339 added a commit to octokit/webhooks that referenced this issue Sep 16, 2023
This is not a bug with the TypeScript compiler but a "feature"...
See microsoft/TypeScript#49160
See ajv-validator/ajv#2047
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

8 participants