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

Feature Request: React-transition-group be part of dependencies and not peer dependencies #118

Closed
benjie91 opened this issue Jun 26, 2019 · 8 comments

Comments

@benjie91
Copy link

Is there any specific reason as to why react-transition-group is listed as peer dependencies? I am using a different version of the react-transition-group lib for my project and this will cause conflict with the lib.

@schiehll
Copy link
Owner

Hey @benjie91

You should update your react-transition-group, why would you want two different versions of the same dependency in your bundle?

That's why it's a peer dependency, so you can have only one version of it, same reason why react is a peer dependency.

@benjie91
Copy link
Author

In that case, your documentation state that it is v2.5.3 and up. So are you limiting users of your lib to this particular version range.

For newer projects who are using v4.1.1 of the react-transition-group, do you suggest them to include both version of dependencies which I am not in favor of as you mentioned.

React is a different story because you are building your lib/app with it. It is expected that everyone who use react for the view component to include it as part of their dependencies.

Likewise for prop-types, I am also strongly in favor it being in the dependencies. You can see facebook also recommending it that way.
https://github.com/facebook/prop-types#how-to-depend-on-this-package

@benjie91
Copy link
Author

Another link for you to consider.
https://github.com/cssinjs/react-jss/issues/164

You can also see Gearon's explanation on why it should be dependencies instead.

@schiehll
Copy link
Owner

No, if there's a newer version of react-transition-group that doesn't work with this lib, we should update it to make it work with the newer version.

But no, I will not make it a dependency because if every lib does that you would end up with a lot of duplicated code in your bundle, and I don't think that this is the right thing to do.

@benjie91
Copy link
Author

benjie91 commented Jun 26, 2019

I think you can let yarn and npm to be the dependency manager as you had already use the carrot sign for the packages and thus there is minimal impact on duplicated code.

In the client side land, an old project especially enterprise application will not likely be able to use your lib because the risks of upgrading these dependencies such as react-transition-group might cause more harm than good. Likewise for new projects, if they are using a later version of react-transition-group, then they have to wait for the maintainer of this lib to upgrade before they can use it. Of course, there are other ways to resolve it (e.g. forking your project, maintaining two separate version in the package.json) but it is not elegant at all.

For example, react-bootstrap lib is already packaging react-transition-group as v4 and up (dependency) and your lib is forcing users to use ^2.5.3 and up (peerDependencies). Here there are already duplicated code but your lib is imposing more restriction on users by dictating what version they need to use. If I also need to use the react-transition-group but I specify it to be v4.1.1 (Yarn and npm would resolve it to the same dependency as the one used by reasct-bootstrap), I would be able to use react-bootstrap but not your lib due to breaking changes between v2 and v4.

@schiehll
Copy link
Owner

schiehll commented Jun 26, 2019

My concern is not about what you install in your node_modules, but what ends up in your final bundle, the one your users will have to download before using your app.

Again, I think this type of dependencies should be peer, because I don't want the final user to have to download multiple versions of a 4.4kb (gziped) dep, just because the developer didn't wanted to manage its dependencies. If the people building react-bootstrap and other libs think different, its ok, but as I've said before, if everyone do it like that your final user will have to download a ton of dup code.

By the way, I've just tested with the latest version of react-transition-group (4.1.1) and it worked, so feel free to use the latest one.

@schiehll
Copy link
Owner

But maybe I'm getting it wrong 🤔

If I tell rollup that it's external, even as a dependency it wouldn't be included in the bundle of the lib, so maybe there's a way to make it work even for UMD bundles.

@schiehll
Copy link
Owner

It was included in the v6 release, thanks for the suggestion!

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

2 participants