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

Unmet peer dependency "prop-type" "classnames" #72

Closed
VincentLanglet opened this issue Feb 13, 2019 · 5 comments
Closed

Unmet peer dependency "prop-type" "classnames" #72

VincentLanglet opened this issue Feb 13, 2019 · 5 comments

Comments

@VincentLanglet
Copy link
Contributor

Expected Behavior

yarn gives no warning

Current Behavior

yarn gives two unmet peer dependency warning

Steps to Reproduce

yarn add notistack

Context

I have a typescript project. I have no reason to add prop-types in mine.
I think prop-types and classnames should be dependencies and not peerDependencies.

Your Environment

Tech Version
Notistack v0.4.2
@VincentLanglet VincentLanglet changed the title Unmet peer dependency "prop-type" "ckassnames" Unmet peer dependency "prop-type" "classnames" Feb 13, 2019
@oliviertassinari
Copy link
Contributor

oliviertassinari commented Feb 18, 2019

@iamhosseindhv What do you think of making classnames a dependency instead of a peer dependency? You can benchmark the other libraries, it's what most maintainers do. If your concern is with the potential bundle duplication, you can make the accepted scope wide.

Alternatively, you can replace the package with clsx. We have replaced classnames usage with clsx in Material-UI v4-alpha.

Related to https://github.com/mui-org/material-ui/pull/14572/files#r257685108

@oliviertassinari
Copy link
Contributor

The React team encourages the usage of prop-type as a dependency.

@VincentLanglet
Copy link
Contributor Author

@oliviertassinari Thanks, let me buy you a 🍔for your interest !

@oliviertassinari
Copy link
Contributor

@VincentLanglet Il faut essayer Cojean maintenant !

@iamhosseindhv
Copy link
Owner

Thanks @oliviertassinari and @VincentLanglet. I'll move them both to dependencies. And it'll be published in the next version. CHANGELOG

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

3 participants