-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add support for peer deps #1072
base: main
Are you sure you want to change the base?
Conversation
It's worth nothing that peer deps are intended to be used in cases where the user is already going to install that peer dep. E.g. publishing a react component, where you peer dep on I am not sure how we can possibly enforce that except via human interaction. Perhaps we should make |
I think maintainer review is a good first state for any confusing systemic change. We can remove the requirement if it's onerous (but I doubt it'll be common, so I bet it's not a problem.) |
Added a mergebot complaint which should put |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far. But I don't have a way to tell from a review whether it's a complete change. I guess the mergebot snapshot test will help with that.
I went ahead and added proper tests for the disallowed dep errors. |
Fixes #433
We've hit multiple cases now where peer deps would have really been beneficial, and now with the new (well, not new anymore) monorepo layout, we can handle these without much code change.
Likely needs more tests, though there really isn't much here besides plumbing.