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

Do not require @types/node to be a dependency #44777

Closed
szmarczak opened this issue May 16, 2020 · 10 comments
Closed

Do not require @types/node to be a dependency #44777

szmarczak opened this issue May 16, 2020 · 10 comments

Comments

@szmarczak
Copy link

In Got 10 we switched to TypeScript. This resulted in +600KB overhead in install size, because all your packages define this:

    "dependencies": {
        ...
        "@types/node": "*",
		...
    },

Please note that not all Node.js users use TypeScript, so the increase is really unnecessary.

Currently we use these types:

https://github.com/sindresorhus/got/blob/df333ddbc163e35537a9f49818e722c7aa6803c0/package.json#L48-L49

/cc @BendingBender @paulmelnikow @sindresorhus

@43081j
Copy link
Contributor

43081j commented May 16, 2020

In fact, "all our types" do not depend on @types/node. It just so happens you depend on an individual set of types which specify a reference to node's types:

/// <reference types="node" />

/// <reference types="node" />

There are vast amounts of types packages which do not depend on @types/node, for obvious reasons. I suggest you simply open an issue requesting that these two types packages drop the dependency, or PR it, assuming you are right and they don't in fact need those types.

From a quick look, both of those packages in fact do depend on Node's standard library (both import http), hence the types. So this is working exactly as expected.

@szmarczak
Copy link
Author

There is a fact if you don't use TypeScript then you don't need the types. @types/node should be individually installed by every TypeScript user IMO.

@43081j
Copy link
Contributor

43081j commented May 16, 2020

The node standard library is just like any other, with its own types and used when necessary. A large amount of projects don't make use of it at all.

In your case, you decided to depend on two packages which themselves depend on node's standard libraries. How else would you resolve those types? Just like any dependency, the Node stdlib is one too, so its part of your dependency tree along with its types. It is not a dependency every user should have, it just so happens to be one you chose to depend on.

Just like the source its self, if you didn't want to pull in node's standard library, you simply wouldn't depend on it would you?

@szmarczak
Copy link
Author

How else would you resolve those types? Just like any dependency, the Node stdlib is one too,

Every TypeScript project declares @types/node in their own package.json. Take yarn for an example:

https://github.com/yarnpkg/berry/blob/3c25698b5619cfef1450f6e6ef70fa2df7b0e783/package.json#L23

Just like the source its self, if you didn't want to pull in node's standard library, you simply wouldn't depend on it would you?

How does this relate to the fact that people who don't use TypeScript don't need the types? All TypeScript projects include @types/node in the dependencies if they need it.

@43081j
Copy link
Contributor

43081j commented May 16, 2020

Every typescript package which depends on node declares that dependency. It is incorrect to state all typescript packages do. If it's a dependency of a given package, just like any other dependency, there will be a reason (such as importing http like in this case).

Huge amounts of typescript packages do not use that package, I work on plenty.

Your immediate problem isn't that you are shipping typescript types to consumers, it is that you depend on a large package (node stdlib, @types/node). If not for that, the size difference would be tiny.

it may be worth opening a separate issue specifically about the size of that individual dependency. I imagine it's because it has all the different node versions in it.

@szmarczak
Copy link
Author

It is incorrect to state all typescript packages do.

Okay, but the thing is that deps that reference Node.js have @types/node in the dependencies, and that increases the install size for non-TypeScript projects. That's the problem I'm talking about.

Huge amounts of typescript packages do not use that package, I work on plenty.

Then these projects then are compatible in many environments. I'm talking about Node.js explicitly.

Your immediate problem isn't that you are shipping typescript types to consumers, it is that you depend on a large package (node stdlib, @types/node). If not for that, the size difference would be tiny.

👍 exactly

it may be worth opening a separate issue specifically about the size of that individual dependency. I imagine it's because it has all the different node versions in it.

That's not a solution since all Node.js modules are related with each other. E.g. https needs tls and net, they need stream, url etc.

@43081j
Copy link
Contributor

43081j commented May 16, 2020

More what I meant was that the @types/node package in particular has:

  • Multiple sets of declarations based on which version of node you run
  • Multiple sets of declarations based on which version of typescript you run

I imagine all those end up in the npm package. Maybe that's why it's so big?

@szmarczak
Copy link
Author

I feel like I'm repeating myself :| The problem is that @types/node is large, and I don't want to include @types/node in the Got package because there's a large amount of people who do not use TypeScript. Got itself doesn't include @types/node in the dependencies, it's in the devDependencies. But unfortunately e.g. @types/cacheable-request does.

@43081j
Copy link
Contributor

43081j commented May 16, 2020

Oh I fully understood already. But there are two issues here:

  • @types/node is very large (a few hundred KB, because of fs and inspect modules)
  • non-typescript consumers shouldn't have to download the types

For the second point, it isn't an easy problem to solve. It is a tooling problem (npm, tsc) as typescript would have to be aware of some typescript-specific set of dependencies in the package manifest. Unlikely to be a thing anytime soon, if ever, as that would bring its own problems.

For the first part, you depend on @types/node, your dependencies directly use the node standard library. So one way or another, you must pull them in as part of your package (for typescript consumers) as you literally use those types.

cacheable-request and responselike both directly depend on the node standard library, so their types of course directly depend on @types/node.

your solution here is to either help reduce the size of @types/node (if thats even possible) or to stop depending on the node standard library in your exported types (which of course means responselike and cacheable-request would need to no longer use node types in what they export, or you simply don't export anything which uses those two dependencies).

@orta orta closed this as completed Jun 7, 2021
@orta
Copy link
Collaborator

orta commented Jun 7, 2021

Hi thread, we're moving DefinitelyTyped to use GitHub Discussions for conversations the @types modules in DefinitelyTyped.

To help with the transition, we're closing all issues which haven't had activity in the last 6 months, which includes this issue. If you think closing this issue is a mistake, please pop into the TypeScript Community Discord and mention the issue in the definitely-typed channel.

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