Only bump peer dependents when leaving range using an experimental flag

[Feiyang1]

Replaces #185
Introduces an experimental flag onlyUpdatePeerDependentsWhenOutOfRange. When set to true, peer dependents are bumped only if the peerDep version is leaving range.

[changeset-bot]

🦋 Changeset is good to go

Latest commit: 2778497

We got this.

This PR includes changesets to release 6 packages

Name	Type
@changesets/assemble-release-plan	Major
@changesets/apply-release-plan	Major
@changesets/get-release-plan	Major
@changesets/config	Patch
@changesets/types	Patch
@changesets/cli	Patch

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

[emmatown]

This is a major to assemble-release-plan, apply-release-plan and any other package that exposes a function that accepts a Config object

[Feiyang1]

Marked the following packages major - assemble-release-plan, apply-release-plan and @changesets/get-release-plan.

Out of curiosity, only @changesets/assemble-release-plan consumes the experimental flag for now, why do we want to bump major for apply-release-plan and get-release-plan?

Second question - Why do we want to use major for them at all? It seems to me a minor version bump suffices because it's not a breaking change. You kinda touched on it in #185 (comment), but I don't think I understand

We'll likely do lots of majors to internal packages(as in the packages that aren't the cli package) when changing these options because otherwise we would have to use exact ranges everywhere

Can you please elaborate?

[Andarist]

Hm, I'm also not sure about this - future changes should be majors (if we are worried about breaking those unsafe flags - which we don't for the top-level packages, we don't want to major bump cli, so Im also unsure why we should care about breaking them for internal ones, the only thing that comes to my mind are fresh reinstalls), but adding the first "unsafe" flag shouldn't require major bump based on my understanding.

[emmatown]

If we did not do a major for assemble-release-plan, people would get broken installs for anything below the latest version of @changesets/cli because they could have the new version of @changesets/assemble-release-plan where we depend on config.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH.onlyUpdatePeerDependentsWhenOutOfRange existing and an old version of @changesets/config.

The reason that it's also important to bump other packages which accept a Config object is that we've added a new required property to the Config object so therefore they should have major bumps. This will manifest concretely in the form of type errors(for consumers of the internal packages) but it's also a conceptual thing that would still exist if we did not have types, types are just making it visible.

This is similar to something often seen in the React community where React adds a new feature like forwardRef or hooks, adding that feature isn't a major for React but it if a consumer starts using that new feature, that consumer has to do a major bump.

[emmatown]

Could you add a test to https://github.com/atlassian/changesets/blob/master/packages/cli/src/commands/version/version.test.ts for something like this:

A project that looks like this:

• pkg-a@1.0.0
  • peerDependencies
    • pkg-b@^1.0.0
• pkg-b@1.0.0

Then you add the following changeset:

• pkg-a@patch
• pkg-b@patch

The project should look like this:

• pkg-a@1.0.1
  • peerDependencies
    • pkg-b@^1.0.0 (this is the important bit, this must be ^1.0.0, NOT ^1.0.1)
• pkg-b@1.0.1

[Feiyang1]

Could you add a test to https://github.com/atlassian/changesets/blob/master/packages/cli/src/commands/version/version.test.ts for something like this:

A project that looks like this:

• pkg-a@1.0.0

  • peerDependencies

    • pkg-b@^1.0.0

• pkg-b@1.0.0

Then you add the following changeset:

• pkg-a@patch
• pkg-b@patch

The project should look like this:

• pkg-a@1.0.1

  • peerDependencies

    • pkg-b@^1.0.0 (this is the important bit, this must be ^1.0.0, NOT ^1.0.1)

• pkg-b@1.0.1

Done. I actually didn't handle this case before, so it required some code changes as well.

Would you mind explain the rationale behind keeping ^1.0.0 instead of bumping it to ^1.0.1? Is it because bumping peerDepedencies signals the change of minimal required version for peerDependencies, and thus consumers need to upgrade them and it should be a major for the dependent? Thanks!

[emmatown]

Is it because bumping peerDepedencies signals the change of minimal required version for peerDependencies, and thus consumers need to upgrade them and it should be a major for the dependent

Yep, that.

[emmatown]

Could you merge this function with shouldUpdateInternalDependency? I find it understand to understand what's going on here with the way the logic is working in versionPackage and this particular part of Changesets is something that's very important that we get right and it's hard to check if it's hard to understand

[Feiyang1]

Done. Also moved the test from cli package to apply-release-plan because that's where we test updateInternalDependencies.

[Feiyang1]

@mitchellhamilton Do you think we are ready to merge it? I'd like to create a PR (code is ready) for #381 (comment) which depends on this one.

[emmatown]

Thanks!!

[atlassian-cla-bot]

Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution.
If your email is listed below, please ensure that you sign the CLA with the same email address.

The following users still need to sign our CLA:
❌mitchellhamilton

_{Already signed the CLA? To re-check, try refreshing the page.}