diff --git a/.changeset/silent-ants-smell.md b/.changeset/silent-ants-smell.md new file mode 100644 index 000000000..f790e76ed --- /dev/null +++ b/.changeset/silent-ants-smell.md @@ -0,0 +1,17 @@ +--- +"@changesets/assemble-release-plan": patch +"@changesets/cli": patch +--- + +author: @Andarist +author: @BPScott + +Fixed the issue that caused transitive dependents of dev dependents to be bumped when a package got bumped and when using `___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH.updateInternalDependents: "always"`. To illustrate this with an example: + +``` +pkg-a - version: 1.0.0 +pkg-b - devDependencies['pkg-a']: 1.0.0 +pkg-c - dependencies['pkg-b']: 1.0.0 +``` + +With a changeset for `pkg-a` the `pkg-c` could have been sometimes incorrectly released. diff --git a/.vscode/launch.json b/.vscode/launch.json index 5a8ad7023..7695f05dc 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,30 +1,12 @@ { "version": "0.2.0", "configurations": [ - { - "type": "node", - "request": "launch", - "name": "Jest All", - "program": "${workspaceFolder}/node_modules/.bin/jest", - "args": ["--runInBand"], - "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen", - "disableOptimisticBPs": true, - "windows": { - "program": "${workspaceFolder}/node_modules/jest/bin/jest" - } - }, { "type": "node", "request": "launch", "name": "Jest Current File", "program": "${workspaceFolder}/node_modules/.bin/jest", - "args": [ - "${fileBasenameNoExtension}", - "--config", - "jest.config.js", - "--no-cache" - ], + "args": ["${relativeFile}", "--config", "jest.config.js", "--no-cache"], "console": "integratedTerminal", "internalConsoleOptions": "neverOpen", "disableOptimisticBPs": true, diff --git a/__fixtures__/simplest-transitive-devdependent/.changeset/README.md b/__fixtures__/simplest-transitive-devdependent/.changeset/README.md new file mode 100644 index 000000000..fd188f505 --- /dev/null +++ b/__fixtures__/simplest-transitive-devdependent/.changeset/README.md @@ -0,0 +1,3 @@ +# We just want a file in here so git collects it + +For this we have deliberately not included a config file, as we want to test the defaults \ No newline at end of file diff --git a/__fixtures__/simplest-transitive-devdependent/package.json b/__fixtures__/simplest-transitive-devdependent/package.json new file mode 100644 index 000000000..2e742d825 --- /dev/null +++ b/__fixtures__/simplest-transitive-devdependent/package.json @@ -0,0 +1,11 @@ +{ + "private": true, + "name": "simple-project", + "description": "three projects, each depending on one other", + "version": "1.0.0", + "bolt": { + "workspaces": [ + "packages/*" + ] + } +} diff --git a/__fixtures__/simplest-transitive-devdependent/packages/pkg-a/package.json b/__fixtures__/simplest-transitive-devdependent/packages/pkg-a/package.json new file mode 100644 index 000000000..ee09b194d --- /dev/null +++ b/__fixtures__/simplest-transitive-devdependent/packages/pkg-a/package.json @@ -0,0 +1,4 @@ +{ + "name": "pkg-a", + "version": "1.0.0" +} diff --git a/__fixtures__/simplest-transitive-devdependent/packages/pkg-b/package.json b/__fixtures__/simplest-transitive-devdependent/packages/pkg-b/package.json new file mode 100644 index 000000000..35ff69741 --- /dev/null +++ b/__fixtures__/simplest-transitive-devdependent/packages/pkg-b/package.json @@ -0,0 +1,5 @@ +{ + "name": "pkg-b", + "version": "1.0.0", + "devDependencies": { "pkg-a": "1.0.0" } +} diff --git a/__fixtures__/simplest-transitive-devdependent/packages/pkg-c/package.json b/__fixtures__/simplest-transitive-devdependent/packages/pkg-c/package.json new file mode 100644 index 000000000..09756a7d5 --- /dev/null +++ b/__fixtures__/simplest-transitive-devdependent/packages/pkg-c/package.json @@ -0,0 +1,5 @@ +{ + "name": "pkg-c", + "version": "1.0.0", + "dependencies": { "pkg-b": "1.0.0" } +} diff --git a/packages/assemble-release-plan/src/determine-dependents.ts b/packages/assemble-release-plan/src/determine-dependents.ts index da355cff8..08b3135e4 100644 --- a/packages/assemble-release-plan/src/determine-dependents.ts +++ b/packages/assemble-release-plan/src/determine-dependents.ts @@ -66,7 +66,9 @@ export default function determineDependents({ ); for (const { depType, versionRange } of dependencyVersionRanges) { - if ( + if (nextRelease.type === "none") { + continue; + } else if ( shouldBumpMajor({ dependent, depType, @@ -80,35 +82,32 @@ export default function determineDependents({ }) ) { type = "major"; - } else { - if ( - // TODO validate this - I don't think it's right anymore - (!releases.has(dependent) || - releases.get(dependent)!.type === "none") && - (config.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH - .updateInternalDependents === "always" || - !semver.satisfies( - incrementVersion(nextRelease, preInfo), - versionRange - )) - ) { - switch (depType) { - case "dependencies": - case "optionalDependencies": - case "peerDependencies": - if (type !== "major" && type !== "minor") { - type = "patch"; - } - break; - case "devDependencies": { - // We don't need a version bump if the package is only in the devDependencies of the dependent package - if ( - type !== "major" && - type !== "minor" && - type !== "patch" - ) { - type = "none"; - } + } else if ( + (!releases.has(dependent) || + releases.get(dependent)!.type === "none") && + (config.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH + .updateInternalDependents === "always" || + !semver.satisfies( + incrementVersion(nextRelease, preInfo), + versionRange + )) + ) { + switch (depType) { + case "dependencies": + case "optionalDependencies": + case "peerDependencies": + if (type !== "major" && type !== "minor") { + type = "patch"; + } + break; + case "devDependencies": { + // We don't need a version bump if the package is only in the devDependencies of the dependent package + if ( + type !== "major" && + type !== "minor" && + type !== "patch" + ) { + type = "none"; } } } diff --git a/packages/assemble-release-plan/src/index.test.ts b/packages/assemble-release-plan/src/index.test.ts index 7fbc6c7b3..24d958379 100644 --- a/packages/assemble-release-plan/src/index.test.ts +++ b/packages/assemble-release-plan/src/index.test.ts @@ -542,6 +542,24 @@ Mixed changesets that contain both ignored and not ignored packages are not allo `); }); + it("should not bump a dev dependent nor its dependent when a package gets bumped", () => { + setup.updateDevDependency("pkg-b", "pkg-a", "1.0.0"); + setup.updateDependency("pkg-c", "pkg-b", "1.0.0"); + + let { releases } = assembleReleasePlan( + setup.changesets, + setup.packages, + defaultConfig, + undefined + ); + + expect(releases.length).toBe(2); + expect(releases[0].name).toEqual("pkg-a"); + expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[1].name).toEqual("pkg-b"); + expect(releases[1].newVersion).toEqual("1.0.0"); + }); + describe("fixed packages", () => { it("should assemble release plan for fixed packages", () => { setup.addChangeset({ @@ -1038,6 +1056,108 @@ Mixed changesets that contain both ignored and not ignored packages are not allo expect(releases[1].newVersion).toEqual("1.0.1"); }); }); + + describe("updateInternalDependents: always", () => { + it("should bump a direct dependent when a dependency package gets bumped", () => { + setup.updateDependency("pkg-b", "pkg-a", "^1.0.0"); + + let { releases } = assembleReleasePlan( + setup.changesets, + setup.packages, + { + ...defaultConfig, + ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { + ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, + updateInternalDependents: "always", + }, + }, + undefined + ); + + expect(releases.length).toBe(2); + expect(releases[0].name).toEqual("pkg-a"); + expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[1].name).toEqual("pkg-b"); + expect(releases[1].newVersion).toEqual("1.0.1"); + }); + + it("should bump a transitive dependent when a dependency package gets bumped", () => { + setup.updateDependency("pkg-b", "pkg-a", "^1.0.0"); + setup.updateDependency("pkg-c", "pkg-b", "^1.0.0"); + + let { releases } = assembleReleasePlan( + setup.changesets, + setup.packages, + { + ...defaultConfig, + ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { + ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, + updateInternalDependents: "always", + }, + }, + undefined + ); + + expect(releases.length).toBe(3); + expect(releases[0].name).toEqual("pkg-a"); + expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[1].name).toEqual("pkg-b"); + expect(releases[1].newVersion).toEqual("1.0.1"); + expect(releases[2].name).toEqual("pkg-c"); + expect(releases[2].newVersion).toEqual("1.0.1"); + }); + + it("not bump a dependent package when a dependency has `none` changeset", () => { + setup.updateDependency("pkg-b", "pkg-c", "^1.0.0"); + setup.addChangeset({ + id: "stuff-and-nonsense", + releases: [{ name: "pkg-c", type: "none" }], + }); + + let { releases } = assembleReleasePlan( + setup.changesets, + setup.packages, + { + ...defaultConfig, + ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { + ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, + updateInternalDependents: "always", + }, + }, + undefined + ); + + expect(releases.length).toBe(2); + expect(releases[0].name).toEqual("pkg-a"); + expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[1].name).toEqual("pkg-c"); + expect(releases[1].newVersion).toEqual("1.0.0"); + }); + + it("should not bump a dev dependent nor its dependent when a package gets bumped", () => { + setup.updateDevDependency("pkg-b", "pkg-a", "^1.0.0"); + setup.updateDependency("pkg-c", "pkg-b", "^1.0.0"); + + let { releases } = assembleReleasePlan( + setup.changesets, + setup.packages, + { + ...defaultConfig, + ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { + ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, + updateInternalDependents: "always", + }, + }, + undefined + ); + + expect(releases.length).toBe(2); + expect(releases[0].name).toEqual("pkg-a"); + expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[1].name).toEqual("pkg-b"); + expect(releases[1].newVersion).toEqual("1.0.0"); + }); + }); }); describe("version update thoroughness", () => { @@ -1335,11 +1455,13 @@ describe("bumping peerDeps", () => { expect(releases[0].name).toEqual("pkg-a"); expect(releases[0].newVersion).toEqual("1.1.0"); }); - }); - describe("updateInternalDependents: always", () => { - it("should bump a direct dependent when a dependency package gets bumped", () => { - setup.updateDependency("pkg-b", "pkg-a", "^1.0.0"); + it("should major bump dependent when leaving range", () => { + setup.updatePeerDependency("pkg-b", "pkg-a", "~1.0.0"); + setup.addChangeset({ + id: "anyway-the-windblows", + releases: [{ name: "pkg-a", type: "minor" }], + }); let { releases } = assembleReleasePlan( setup.changesets, @@ -1348,7 +1470,7 @@ describe("bumping peerDeps", () => { ...defaultConfig, ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, - updateInternalDependents: "always", + onlyUpdatePeerDependentsWhenOutOfRange: true, }, }, undefined @@ -1356,64 +1478,10 @@ describe("bumping peerDeps", () => { expect(releases.length).toBe(2); expect(releases[0].name).toEqual("pkg-a"); - expect(releases[0].newVersion).toEqual("1.0.1"); - expect(releases[1].name).toEqual("pkg-b"); - expect(releases[1].newVersion).toEqual("1.0.1"); - }); - - it("should bump a transitive dependent when a dependency package gets bumped", () => { - setup.addPackage("pkg-c", "1.0.0"); - setup.updateDependency("pkg-b", "pkg-a", "^1.0.0"); - setup.updateDependency("pkg-c", "pkg-b", "^1.0.0"); - - let { releases } = assembleReleasePlan( - setup.changesets, - setup.packages, - { - ...defaultConfig, - ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { - ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, - updateInternalDependents: "always", - }, - }, - undefined - ); - - expect(releases.length).toBe(3); - expect(releases[0].name).toEqual("pkg-a"); - expect(releases[0].newVersion).toEqual("1.0.1"); + expect(releases[0].newVersion).toEqual("1.1.0"); expect(releases[1].name).toEqual("pkg-b"); - expect(releases[1].newVersion).toEqual("1.0.1"); - expect(releases[2].name).toEqual("pkg-c"); - expect(releases[2].newVersion).toEqual("1.0.1"); - }); - }); - - it("should major bump dependent when leaving range", () => { - setup.updatePeerDependency("pkg-b", "pkg-a", "~1.0.0"); - setup.addChangeset({ - id: "anyway-the-windblows", - releases: [{ name: "pkg-a", type: "minor" }], + expect(releases[1].newVersion).toEqual("2.0.0"); }); - - let { releases } = assembleReleasePlan( - setup.changesets, - setup.packages, - { - ...defaultConfig, - ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { - ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, - onlyUpdatePeerDependentsWhenOutOfRange: true, - }, - }, - undefined - ); - - expect(releases.length).toBe(2); - expect(releases[0].name).toEqual("pkg-a"); - expect(releases[0].newVersion).toEqual("1.1.0"); - expect(releases[1].name).toEqual("pkg-b"); - expect(releases[1].newVersion).toEqual("2.0.0"); }); }); diff --git a/packages/cli/src/commands/version/version.test.ts b/packages/cli/src/commands/version/version.test.ts index 21ab085cd..773b1b2a5 100644 --- a/packages/cli/src/commands/version/version.test.ts +++ b/packages/cli/src/commands/version/version.test.ts @@ -1066,6 +1066,53 @@ describe("updateInternalDependents: always", () => { " `); }); + + it("should not bump a dev dependent nor its dependent when a package gets bumped", async () => { + const cwd = await f.copy("simplest-transitive-devdependent"); + const spy = jest.spyOn(fs, "writeFile"); + await writeChangeset(simpleChangeset, cwd); + await version(cwd, defaultOptions, { + ...modifiedDefaultConfig, + ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { + ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH, + updateInternalDependents: "always", + }, + }); + + expect(getPkgJSON("pkg-a", spy.mock.calls)).toEqual( + expect.objectContaining({ + name: "pkg-a", + version: "1.1.0", + }) + ); + expect(getPkgJSON("pkg-b", spy.mock.calls)).toEqual( + expect.objectContaining({ + name: "pkg-b", + version: "1.0.0", + devDependencies: { + "pkg-a": "1.1.0", + }, + }) + ); + // `pkg-c` should not be touched + expect(() => getPkgJSON("pkg-c", spy.mock.calls)).toThrowError(); + + expect(getChangelog("pkg-a", spy.mock.calls)).toMatchInlineSnapshot(` + "# pkg-a + + ## 1.1.0 + + ### Minor Changes + + - g1th4sh: This is a summary + " + `); + + // pkg-b and - pkg-c are not being released so changelogs should not be + // generated for them + expect(() => getChangelog("pkg-b", spy.mock.calls)).toThrowError(); + expect(() => getChangelog("pkg-c", spy.mock.calls)).toThrowError(); + }); }); describe("pre", () => {