-
Notifications
You must be signed in to change notification settings - Fork 520
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
update a dependency correctly when it's declared multiple times with different range #412
Conversation
🦋 Changeset is good to goLatest commit: 0e8a6bb We got this. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
…esets into fei-multi-dep-fix
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.
Addressed comments. PTAL.
"pkg-b": "1.0.1" | ||
}, | ||
peerDependencies: { | ||
"pkg-b": "^1.0.1" |
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.
I'm always a little bit when it comes to how changesets handle peer dependencies, but I have thought that updating a peer dependency with a patch release shouldn't bump this? Or it just bumps the range but doesn't bump the major of the dependent package (pkg-a here)?
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.
Correct. Patch bump to a peerDependency doesn't bump the major of the dependent package. See here.
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.
An offtopic observation - the linked line looks suspicious. What if nextRelease.type === 'none'
? This might be worth investigating.
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.
hmm, that seems like a possibility. I might look into it when I get time and create an issue if I can reproduce. I will leave it in this PR though.
@Andarist gentle ping |
__fixtures__/simple-project-same-dep-diff-range/packages/pkg-a/package.json
Show resolved
Hide resolved
@Andarist Sorry for the extremely slow response. Can you please take another look? |
Fixes #411