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

Types have been removed from devDependencies #2889

Closed
1 task done
benjdlambert opened this issue Jun 29, 2022 · 6 comments · Fixed by #2942
Closed
1 task done

Types have been removed from devDependencies #2889

benjdlambert opened this issue Jun 29, 2022 · 6 comments · Fixed by #2942

Comments

@benjdlambert
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

Reproduction link

backstage/backstage#12310

Steps to reproduce

Just install recharts in a typescript project and disable skipLibCheck

What is expected?

The typescript compiler should not complain

What is actually happening?

It complains because the types that are depended on in the project are not shipped with the library anymore

Environment Info
Recharts v2.1.6
React 17
System Osx
Browser Chrome

Since #2843, types that are depended on by the published .d.ts files like

https://github.com/recharts/recharts/blob/master/src/util/types.ts#L25

are no longer shipped with the project they are now in devDependencies.

This means that projects that use this library without skipLibCheck: true typescript fails with errors saying to install the types yourself.

This is error prone, as they need to be the same versions of what the package was published with. The types should be moved back to dependencies

@jmfrancois
Copy link
Contributor

This has been discussed on our side (at Talend) before I did the change here.

I am happy to see challenge here. This is a global question indeed.

It is already hard to align runtime libraries provided by deps (still see packages without caret range syntax for example).
I continue to think deps are for runtime, not to develop. and you need the types package to develop with recharts.
So if you toolchain is typescript you have to add types packages.

WDYT @benjdlambert ?

@stianjensen
Copy link
Contributor

I guess that can make sense, although it seems to me to at least be quite common for packages to distribute their types dependencies.
It feels like a lot of work for each consumer of this package to have to manually figure out which version of each types package to install though.

In any case, removing this should probably have been a major bump to 3.x.

@jmfrancois
Copy link
Contributor

I have checked many packages it was not the case. Distribute types is OK but put types of deps is sth else.

@benjdlambert
Copy link
Author

I'm not sure I understand the difference. If they're exported in the public interface of the package, they're arguably not devDependencies at all. Especially to packages consuming this from a Typescript environment.

@benjdlambert
Copy link
Author

We have tooling to prevent exactly this happening in our repository, as mentioned from @stianjensen it's a royal pain to find the type versions that you should have to install. Especially when some @types/x doesn't line up with the same versioning scheme.

I'm feeling that if you have a type from one of your dependencies which is part of your public contract, then you should always distribute that type. If that type changes over a major version, typescript isn't going to do its job properly as the types might not be compatible further down the line?

@jmfrancois
Copy link
Contributor

I am agree to say it should be easy to install types dependencies of a package in the context of typescript which is not an environment but a toolchain.
TS interfaces are published yes but they are useless in the context of a JS project.
This is a bit like react-dom and react-native which is the reason why we have optional peerDependencies support.

May be package.json should add support for a typeDependencies so those deps could be installed when you are in a ts project only.

I will not argue more, if we don't find better option than using dependencies for tooling env ...

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