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

Add support for peer deps #1072

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

jakebailey
Copy link
Member

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.

@jakebailey
Copy link
Member Author

jakebailey commented Sep 30, 2024

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 @types/react because the user will have already installed it and can control the version. Anything more than that increases the annoyance levels for those with package managers which do not auto install peers.

I am not sure how we can possibly enforce that except via human interaction. Perhaps we should make mergebot flag any PR which modifies peerDeps as needing a maintainer review?

@sandersn
Copy link
Member

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.)

@jakebailey
Copy link
Member Author

Added a mergebot complaint which should put Check Config on those PRs, though I don't have a PR to test it on quite yet. Will make one at the risk of annoying someone...

Copy link
Member

@sandersn sandersn left a 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.

@jakebailey
Copy link
Member Author

I went ahead and added proper tests for the disallowed dep errors.

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

Successfully merging this pull request may close these issues.

Allow peer dependencies with stricter version ranges
2 participants