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
bugfix: ignore ranges when snapshot versioning is performed (use exact version) #857
Conversation
🦋 Changeset detectedLatest commit: 0b15dd4 The changes in this PR will be included in the next version bump. 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 |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0b15dd4:
|
uow nice! I've just crossed trough this problem, good timing for me though :) |
Thanks for the code review @Andarist . I pushed fixes for all notes :) |
{ | ||
changesets: ["some-id"], | ||
name: "pkg-b", | ||
newVersion: "1.1.0", |
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.
While working on #1008 I've noticed that this test case doesn't reflect how the full thing works in practice. Usually the newVersion
should already have the snapshot modifier here. This would be computed here when assembling the release plan:
changesets/packages/assemble-release-plan/src/index.ts
Lines 248 to 255 in 4acaff7
newVersion: snapshotSuffix | |
? getSnapshotVersion( | |
incompleteRelease, | |
preInfo, | |
refinedConfig.snapshot.useCalculatedVersion, | |
snapshotSuffix | |
) | |
: getNewVersion(incompleteRelease, preInfo), |
This test case still tests that the range modifier gets dropped when snapshot
is provided:
changesets/packages/apply-release-plan/src/version-package.ts
Lines 96 to 98 in 4acaff7
let newNewRange = snapshot | |
? version | |
: `${getVersionRangeType(depCurrentVersion)}${version}`; |
However... this should, sort of, be an impossible setup. Should we validate this anyhow? Should the test case be tweaked to reflect the real usage better?
Related: #855