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

Should shared React Components use peerDependencies or dependencies? #345

Closed
magicmark opened this issue Dec 19, 2017 · 2 comments
Closed

Comments

@magicmark
Copy link

Hi @kentcdodds!

I see that many shared react component libs (e.g. downshift) use peerDependencies to depend on react/react-dom (vs regular dependencies).

I'm interested as to what are the tradeoffs/differences here for doing so? Why not just use regular dependencies?

Thanks so much!!

@magicmark
Copy link
Author

So I did some digging, and I think I've found some answers!

  • peerDeps were introduced in npm@1.2.10:
    https://nodejs.org/en/blog/npm/peer-dependencies/#the-problem-plugins

    Even for plugins that do have such direct dependencies, probably due to the host package supplying utility APIs, specifying the dependency in the plugin's package.json would result in a dependency tree with multiple copies of the host package—not what you want.

    Sounds good, this sums up what we're after :)

  • It seems like benefit here is less nesting of node_modules, which is important, because of the issues raised in the above blog post.

  • We might still get nesting, even with wildcard dependencies - Yarn dependency resolution could be improved by adding deduping ([feature] improve resolution deduping yarnpkg/yarn#3778). peerDependencies helps negate this issue.

However, I still have a question about how we enforce peerDependencies, which was my original concern. I'll share my current understanding:

  • npm@3 stopped giving errors if the peerDependency was not correctly installed, and just gives a quiet warning at install time. [1] npm@5 and yarn have the same behavior.

  • So a danger here is that in the case of human error, since npm & yarn do not exit 1 if there's an incompatibility, applications could silently be broken. The build would succeed, and everything would install fine, but then something might be broken on the page.

  • What happens when a bundled library component is imported and loaded onto the page, where the component was built with react@15, but the application is at react@16?

    This appears to work for me - as long as it doesn't use any deprecated APIs. However when it does, such as a ref string attribute, the component will break, and React will loudly complain in the console.

    (There's some more discussion on this here Warn when two versions of React are used alongside facebook/react#2402)

    So the potential for something to appear to work fine, and then suddenly break when an API changes, without any bumps to the package furthers my concern. (Upper bounding to the major version would help solve some problems here)

  • npm ls/yarn check will exit 1 if there's a version incompatibility. Requires running this manually, or including it as part of the build.

    To challenge some design decisions though, maybe ideally:

    • npm/yarn would exit non-0 on install (I presume there might be a good reason why it currently doesn't though?), or
    • React would loudly complain 100% of the time (may be be unwanted, could be overly noisy?)
  • To sum up, I'm not totally sure if my paranoia about silently broken applications as a result of using peerDependencies is valid/reasonable?

    (Or if it is, should we just assume consumers are always running yarn check? Or should this be by default enforced/checked as described above?)

Sorry for the essay, and thanks so much for taking the time for this!

[1] http://blog.npmjs.org/post/110924823920/npm-weekly-5

@kentcdodds
Copy link
Owner

Hey @magicmark!

Thanks for the question and the follow-up. I'm pretty sure you've worked out things. I actually have answered this question before as well: #279

To answer your specific question:

What happens when a bundled library component is imported and loaded onto the page, where the component was built with react@15, but the application is at react@16?

Generally you should probably not let your peerDependencies range include versions you haven't tested your component with. That's part of why the warning is there. It says: "Hey, the creator of this library hasn't tested their code with the version of react you have installed. Just be aware of that!"

I wouldn't worry too much about it though. peerDependencies serve a very important role in the world of plugins and interdependencies. So definitely use them, and make the version as wide as you can :)

Good luck!

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

No branches or pull requests

2 participants