Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

remove @types/react from dependencies #239

Closed
1pete opened this issue Nov 6, 2018 · 9 comments · Fixed by #245
Closed

remove @types/react from dependencies #239

1pete opened this issue Nov 6, 2018 · 9 comments · Fixed by #245

Comments

@1pete
Copy link

1pete commented Nov 6, 2018

From #234, I think library shouldn't include types of other dependencies. If types has to be in dependencies, it has to be in react project itself, so other dependencies those depend on it, don't have to make the redundancy.

I have tons of examples
react-dropzone: https://github.com/react-dropzone/react-dropzone/blob/master/package.json
react-datepicker: https://github.com/Hacker0x01/react-datepicker/blob/master/package.json
reactstrap: https://github.com/reactstrap/reactstrap/blob/master/package.json

But if it's @type/react-popper, then yeah, it'd make a bit sense.

@FezVrasta
Copy link
Member

react-popper types require React types, so it's correct to have them in the dependencies list.

@Strnadj
Copy link

Strnadj commented Nov 14, 2018

@FezVrasta what is the solution? I always have to manually remove @types/react from node_modules/react-popper otherwise it conflicts with @types/react installed in project dependencies. It causes very ugly errors like:

ERROR in [at-loader] ./node_modules/@types/react/index.d.ts:2305:19 
    TS2320: Interface 'Element' cannot simultaneously extend types 'ReactElement<any>' and 'ReactElement<any>'.
  Named property 'type' of types 'ReactElement<any>' and 'ReactElement<any>' are not identical.

as @1pete mentions, it can be solved (as examples).

Next thing is types (@types/react) are in dependencies and devDependencies ... twice

@FezVrasta
Copy link
Member

I'm sorry guys, I don't know, I just can't stand TypeScript problems anymore. It's always a problem with this thing. Why don't you guys use Flow? It just works 😩

Please feel free to discuss here with each others and decide about a solution, I don't know anything about TS and the more issues I see about it the less I want to know.

I'll reopen this issue until you guys find a solution. Feel free to send PRs to show what you'd like to change but please include extensive tests to make sure it really works, because every time someone merges a PR about TypeScript a bunch of people comes here complaining that everything is broken.

Sorry for my tone, I don't want to be rude, but this story has going on for so long that I got sick of it, next step will be to get rid of the types all together and just ship Flow.

@Strnadj
Copy link

Strnadj commented Nov 14, 2018

What about moving TypeScript definitions to https://github.com/DefinitelyTyped/DefinitelyTyped and that is all... For those who want it, install @types/react-popper....

@FezVrasta
Copy link
Member

Yes that'd be a good idea but then I will not even try to maintain them anymore.

Honestly I thought it would be easy to ship those types with the library, with Popper.js (which is not typed) I just added a popper.js.flow file with the types and it just works, I thought it would have been the same with TS here...

@rogierschouten
Copy link

@FezVrasta the real solution is to make @types/react both a peerDependency and a devDependency. The peerDependency should preferably have a "*" version or a version range.

@1pete
Copy link
Author

1pete commented Nov 15, 2018

@rogierschouten no, that is an absurd move because every application has to declare @types/react as dependencies regardless it uses typescript or not

I have 2 suggestions

  1. (Recommended) In this project, just declare @types/react in devDependencies (or completely remove from everywhere)
    so applications those use typescript and this library, they has to add @types/react to their own package.json
  2. In this project, declare in dependencies with "*" as version.

@JLarky
Copy link
Contributor

JLarky commented Nov 15, 2018

I'm sorry guys, I don't know, I just can't stand TypeScript problems anymore. It's always a problem with this thing. Why don't you guys use Flow? It just works 😩

my thoughts on that exactly! If only people could see that Clojure just works and used it instead of JavaScript ;(

@sseppola
Copy link

I think @1pete's suggestion about installing it as a devDependency is spot on, this seems to be what other libs are doing without causing problems.

For those of us using yarn you can use selective dependency resolutions to get around this, just add the following to your package.json:

  "resolutions": {
    "react-popper/@types/react": "16.4.1"
  },

Note: Use the version of @types/react you're using otherwise

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

Successfully merging a pull request may close this issue.

6 participants