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

Suggestion: change @types/node dependencies to devDependencies #55519

Open
ImRodry opened this issue Aug 31, 2021 · 7 comments
Open

Suggestion: change @types/node dependencies to devDependencies #55519

ImRodry opened this issue Aug 31, 2021 · 7 comments

Comments

@ImRodry
Copy link
Contributor

ImRodry commented Aug 31, 2021

I'd like to suggest changing the way @types/node is declared under other packages' dependency lists, since many packages have this installed and it's essential for basically all TS projects. If a package has this as a dependency, it's possible you may run into outdated versions of the node package and issues because of that (which has happened to me before).
Just to show how many packages have @types/node as a dependency, this is what I get when doing npm ls @types/node on my project (note that I do not even have it as a dev dependency myself due to this).
image
It just so happens that they're all up to date in this example, but they could not be, and that's where I'm trying to get at. Having the dependency installed means users will not run into errors when trying to access things from node, but will do if their package is outdated and may be confused by this.

My suggestion is to replace all @types/node dependencies with devDependencies so that they are no longer installed when you install the main package in your project.
Hope that explanation made sense and I'd love to hear your feedback on this and will open the PR if maintainers agree with this.

@KapitanOczywisty
Copy link
Contributor

In suggested scenario you'd need to install all devDependencies manually, since devDependencies of (dev)dependiencies are not installed, but are required for properly working typings. For example: instead of yarn add @types/multicast-dns you'd be writing yarn add @types/multicast-dns @types/dns-packet, but this is very simple case, have fun with packages like @types/express.

Also auto-generated dependencies are provided as "@types/node": "*", and this is why you couldn't find multiple versions of the same package. This of course could happen, but it's rare and the same could happen for normal packages.

And lastly If you have something as devDependency and you install packages in production mode, no devDependencies nor any of their dependencies will be installed. Seems like in your case packages which have typings included are installing @types on production and this is out of DT scope.

@ImRodry
Copy link
Contributor Author

ImRodry commented Sep 8, 2021

My main issue here is with the node package because it's one we all need and having it as a dependency in other packages can cause version overlaps like I described.
My issue here is not with production mode, it's simply the version of the node package in dev mode. Maybe applying this to all package dependencies would be too much but I think the packages that depend on @types/node should have it as a dev dependency, since everyone should be installing it anyway.

@KapitanOczywisty
Copy link
Contributor

This seems like solving hypothetical problem in hypothetical conditions. @types/node is in most cases included as * or ^16.7.6, which means there will be only one version installed anyway. In worst case scenario you can force certain version for all packages https://classic.yarnpkg.com/en/docs/selective-version-resolutions/ For npm this feature is on roadmap npm/statusboard#343

And besides manually installed typings there are also for example automatically downloaded by vscode - these may be not happy about @types/node missing.

@ImRodry
Copy link
Contributor Author

ImRodry commented Sep 9, 2021

This seems like solving hypothetical problem in hypothetical conditions. @types/node is in most cases included as * or ^16.7.6, which means there will be only one version installed anyway. In worst case scenario you can force certain version for all packages classic.yarnpkg.com/en/docs/selective-version-resolutions For npm this feature is on roadmap npm/statusboard#343

And besides manually installed typings there are also for example automatically downloaded by vscode - these may be not happy about @types/node missing.

The point I'm trying to make here is that all @types packages would have @types/node as a devDependency so that it isn't installed with the main package to allow for the user to download their version of @types/node themselves. This would mean the package would never be missing and just gives the user more control over it

@demurgos
Copy link
Contributor

For reference, this was discussed a few years ago in this types-publisher issue. This issue was referenced tons of times so I linked to my comment in particular because it should be especially relevant.

The core of the issue is that there are two kinds of type declarations:

  • External module declarations (for regular npm dependencies where you explicitly import their content)
  • Global type declarations (formerly known as ambient declarations)

External declarations are local and should be marked as regular dependencies to allow the resolution of transitive dependencies as mentioned above. I actually believe that your mention of "Most of us install @types packages as dev dependencies, meaning we don't want them to be installed" is actually an anti-pattern because of this.

Then there are packages containing global declarations. They are a pain. And I agree that today these type declarations speficifically should be devDependencies instead of dependencies. I've been writing type declarations since the very early days of TypeScript, before DefinitelyTyped even existed, and I continue to believe that @types/ picked the wrong semantics here. Global declarations MUST be unique in the dependency tree, using "@types/node": "*" in dependencies is a hack relying on the implementation details of the resolution algorithm of some specific versions of some specific package manager.

Full comment (I am still waiting for a satisfying answer since 2016):

@DanielRosenwasser @RyanCavanaugh

Hi,
I am currently reevaluating how to handle @types/node: should it be in dependencies or devDependencies? With which ranges?

When I asked this question a few years ago, I got this reply (#81 (comment)):

My thought is dependencies since you're written as a library with dependents. For any users using Typings with TypeScript 1.8 and below, they shouldn't be affected, but as you noted, 2.0+ users may run into problems (at which point the solution is trivial - just remove node from their typings.json - but still annoying).

Despite this recommendation, there is still a lot of confusion around handling @types/node: read the issues referencing this discussion.

In my experience, adding @types/node to dependencies is the source of many compatibility issues: it is very easy to get incompatible duplicate definitions because it defines globals. Due to npm's dependency installation algorithm, you can get two (or more) different @type/node versions in your dependency tree.

My understanding is that type definitions declaring globals (e.g. @types/node, @types/mocha) should be treated as environment declarations, just like the lib field from tsconfig.json: es5, dom, es2015.promise, etc. This in turn means that they should only be defined by the top-level project.
Thus @type/node or any other global-inducing type definitions should be in devDependencies and never in dependencies. If a user wants to consume a lib depending on @types/node, he must explicitly provide it.
Since I started following this practice (for about 2 5 years now), I never had issues with duplicate global definitions again.

DefinitelyTyped adds @types/node to dependencies, but I am not sure if this is representative of how libraries providing their own types should be handled. The main difference is that DefinitelyTyped also owns @types/node: whenever types/node is updated, all the @types/* depending on it are updated. This means that they always use the exact same version. This is only possible because all these package live in the same repo. For individual projects, it is impossible to ensure that it always depends on the latest version of @types/node.

typings was less convenient but in regard to this issue it was a superior tool to npm+@types/ because it understood the difference between the different kinds of type declarations (ambiant/globals VS external module).

In 2019 2021, how should a standalone Typescript library depend on environment/global declarations such as @types/node?

  • Using dependencies with a range x.y.z / ^x.y.z / >=x.y.z / *?
  • Using devDependencies with a range x.y.z / ^x.y.z / >=x.y.z / *?
  • Using peerDependencies with a range x.y.z / ^x.y.z / >=x.y.z / *?

My opinion is that Node/Mocha/otherLibWithGlobals declarations should be enabled through the lib field in tsconfig, just like browsers types (dom etc.). The browser and Node should be two independent and equally supported platforms, but for some reasons only the browser types are blessed to be configurable in lib and get the right semantics.
The next best thing is to use peerDependencies to push @node/types all the way up as a singleton, but npm deeply messed up by changing the semantics of this field every few versions so there is no way to use it reliably.
We're then left with devDependencies and dependencies, of these two devDependencies is the less harmful one when handling global declarations.

The only notable change in 2021 is that the upcoming 4.5 version supports lib from node_modules. These are the first steps, but we can hope that it would allow to eventually transition to using "@typescript/lib-node": "npm:@types/node". But as with any transition, it's not enough to be a better solution if the transition is too hard...

@ImRodry
Copy link
Contributor Author

ImRodry commented Oct 17, 2021

External declarations are local and should be marked as regular dependencies to allow the resolution of transitive dependencies as mentioned above. I actually believe that your mention of "Most of us install @types packages as dev dependencies, meaning we don't want them to be installed" is actually an anti-pattern because of this.

I'm confused because I've always installed my types dependencies as dev dependencies and they've always installed their own dependencies when I did that. I only do this so that they aren't installed when in production since, well, there's no point in doing that, and it saves some space. I've also always seen other projects do this so I thought this was the pattern as it always seemed logical to me. Also do note that I'm talking about personal projects and not public libraries, although I believe that libraries written in TypeScript should only have the @types packages in their dependencies if they need them for types of their own, otherwise they should go in devDependencies.

As for the rest of your message, I agree that node types should be bundled with TypeScript and we should have a way to opt-in to them through the lib option in the configuration file. Hopefully, this could also solve an issue that I've personally faced which is that I have DOM types installed on my project despite not specifying them under lib and my project running solely on node, meaning that some things are treated as valid when they throw an error in runtime.

@ImRodry ImRodry changed the title Suggestion: change all "dependencies" to "devDependencies" Suggestion: change @types/node dependencies to devDependencies Oct 17, 2021
@ImRodry
Copy link
Contributor Author

ImRodry commented Oct 17, 2021

Changed the original topic and description of the issue to focus more on the @types/node package as I realized that changing all dependencies would be unreasonable and would lead to more problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants