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

Should Glamorous be a dependency rather than peerDependency #177

Closed
penx opened this issue Mar 26, 2018 · 4 comments
Closed

Should Glamorous be a dependency rather than peerDependency #177

penx opened this issue Mar 26, 2018 · 4 comments

Comments

@penx
Copy link
Member

penx commented Mar 26, 2018

This ticket is to discuss potentially moving Glamorous from peerDependencies to dependencies.

This was initially raised on Slack by @lennym:

You could achieve something similar though (I think) by moving the peer dependency to a main dependency, but with the same leniency for versions. npm would dedupe to the parent project’s version if it satisfies (which it’d need to anyway to satisfy the peer dependency) but would install latest if no parent version were present.

@penx
Copy link
Member Author

penx commented Mar 26, 2018

I think it's appropriate to use peer dependencies when you are stating:

  1. no more than 1 of this dependency should ever be included in an application
  2. this package is solely designed to be used as a dependency

As far as I'm aware, glamor does not work if you have multiple instances loaded in the browser, so I think it's safe to assume multiple instances of Glamorous could also cause issues. As such, I think it's preferable to keep it in peer dependencies.

On the other hand, if we were to switch to another CSSinJS solution such as styled-components, I understand the desire to have peer projects automatically update, however if a core change like this was made I would expect to update the major version number and would want peer projects to be aware of the change.

@penx
Copy link
Member Author

penx commented Mar 26, 2018

Both Atlassian AtlasKit and Auth0 Cosmos have styled-components as a dependency rather than peerDependency

https://bitbucket.org/atlassian/atlaskit-mk-2/src/71efe2fbeb26866ef6f29d81d43e31978184460c/packages/core/avatar/package.json?at=master

https://www.npmjs.com/package/auth0-cosmos?activeTab=dependencies

Although the styled-components FAQ specifically says to use peerDependencies:

If you are a library author, we recommend that you should not bundle and ship styled-components module with your library. There are two steps that you need to do to achieve this:

https://www.styled-components.com/docs/faqs

@penx
Copy link
Member Author

penx commented Mar 26, 2018

@stevesims
Copy link
Member

Closing - we no longer use glamorous, and have had no reported problems using peerDependencies

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