Fixed a crash when using tag versions of workspace packages (#1047)

* fix: Invalid comparator semver error when using tag versions * test: add testing scenario for versioning with tags * chore: typo * fix: changelog entry dependency information * chore: apply cr suggestion, move semver check earlier in the process * fix: update changelog entry for workspace range, test to cover that case * chore: move test case and adjust assertions * make one of the calls lazy * make the other one lazier too * tweak tests * tweak test title * Apply suggestions from code review --------- Co-authored-by: Mateusz Burzyński <[email protected]>
changesets · Jun 29, 2024 · d108fa6 · d108fa6
1 parent a832b64
commit d108fa6
Show file tree

Hide file tree

Showing 5 changed files with 304 additions and 1 deletion.
diff --git a/.changeset/famous-poems-jog.md b/.changeset/famous-poems-jog.md
@@ -0,0 +1,6 @@
+---
+"@changesets/cli": patch
+"@changesets/apply-release-plan": patch
+---
+
+Fixed a crash that could occur when depending on a tagged version of another workspace package.
diff --git a/packages/apply-release-plan/src/get-changelog-entry.ts b/packages/apply-release-plan/src/get-changelog-entry.ts
@@ -3,6 +3,7 @@ import { ChangelogFunctions, NewChangesetWithCommit } from "@changesets/types";
 import { ModCompWithPackage } from "@changesets/types";
 import startCase from "lodash.startcase";
 import { shouldUpdateDependencyBasedOnConfig } from "./utils";
+import validRange from "semver/ranges/valid";
 
 type ChangelogLines = {
   major: Array<Promise<string>>;
@@ -62,8 +63,10 @@ export default async function getChangelogEntry(
       release.packageJson.peerDependencies?.[rel.name];
 
     const versionRange = dependencyVersionRange || peerDependencyVersionRange;
+    const usesWorkspaceRange = versionRange?.startsWith("workspace:");
     return (
       versionRange &&
+      (usesWorkspaceRange || validRange(versionRange) !== null) &&
       shouldUpdateDependencyBasedOnConfig(
         { type: rel.type, version: rel.newVersion },
         {

diff --git a/packages/apply-release-plan/src/index.test.ts b/packages/apply-release-plan/src/index.test.ts
@@ -1356,6 +1356,106 @@ describe("apply release plan", () => {
             },
           });
         });
+        it("should not update dependant's dependency range when it depends on a tag of a bumped dependency", async () => {
+          let { changedFiles } = await testSetup(
+            {
+              "package.json": JSON.stringify({
+                private: true,
+                workspaces: ["packages/*"],
+              }),
+              "packages/pkg-a/package.json": JSON.stringify({
+                name: "pkg-a",
+                version: "1.0.3",
+                dependencies: {
+                  "pkg-b": "latest",
+                },
+              }),
+              "packages/pkg-b/package.json": JSON.stringify({
+                name: "pkg-b",
+                version: "1.2.0",
+                dependencies: {
+                  "pkg-a": "bulbasaur",
+                },
+              }),
+            },
+            {
+              changesets: [
+                {
+                  id: "quick-lions-devour",
+                  summary: "Hey, let's have fun with testing!",
+                  releases: [
+                    { name: "pkg-a", type: "patch" },
+                    { name: "pkg-b", type: "patch" },
+                  ],
+                },
+              ],
+              releases: [
+                {
+                  name: "pkg-a",
+                  type: "patch",
+                  oldVersion: "1.0.3",
+                  newVersion: "1.0.4",
+                  changesets: ["quick-lions-devour"],
+                },
+                {
+                  name: "pkg-b",
+                  type: "patch",
+                  oldVersion: "1.2.0",
+                  newVersion: "1.2.1",
+                  changesets: ["quick-lions-devour"],
+                },
+              ],
+              preState: undefined,
+            },
+            {
+              changelog: false,
+              commit: false,
+              fixed: [],
+              linked: [],
+              access: "restricted",
+              changedFilePatterns: ["**"],
+              baseBranch: "main",
+              updateInternalDependencies,
+              ignore: [],
+              privatePackages: { version: true, tag: false },
+              ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
+                onlyUpdatePeerDependentsWhenOutOfRange: false,
+                updateInternalDependents: "out-of-range",
+              },
+              snapshot: {
+                useCalculatedVersion: false,
+                prereleaseTemplate: null,
+              },
+            }
+          );
+          let pkgPathA = changedFiles.find((a) =>
+            a.endsWith(`pkg-a${path.sep}package.json`)
+          );
+          let pkgPathB = changedFiles.find((b) =>
+            b.endsWith(`pkg-b${path.sep}package.json`)
+          );
+
+          if (!pkgPathA || !pkgPathB) {
+            throw new Error(`could not find an updated package json`);
+          }
+          let pkgJSONA = await fs.readJSON(pkgPathA);
+          let pkgJSONB = await fs.readJSON(pkgPathB);
+
+          expect(pkgJSONA).toMatchObject({
+            name: "pkg-a",
+            version: "1.0.4",
+            dependencies: {
+              "pkg-b": "latest",
+            },
+          });
+          expect(pkgJSONB).toMatchObject({
+            name: "pkg-b",
+            version: "1.2.1",
+            dependencies: {
+              "pkg-a": "bulbasaur",
+            },
+          });
+        });
       });
       describe("updateInternalDependencies set to minor", () => {
         const updateInternalDependencies = "minor";

diff --git a/packages/apply-release-plan/src/version-package.ts b/packages/apply-release-plan/src/version-package.ts
@@ -6,6 +6,7 @@ import {
 import getVersionRangeType from "@changesets/get-version-range-type";
 import Range from "semver/classes/range";
 import semverPrerelease from "semver/functions/prerelease";
+import validRange from "semver/ranges/valid";
 import { shouldUpdateDependencyBasedOnConfig } from "./utils";
 
 const DEPENDENCY_TYPES = [
@@ -65,7 +66,8 @@ export default function versionPackage(
 
         if (
           !usesWorkspaceRange &&
-          bumpVersionsWithWorkspaceProtocolOnly === true
+          (bumpVersionsWithWorkspaceProtocolOnly ||
+            validRange(depCurrentVersion) === null)
         ) {
           continue;
         }

diff --git a/packages/cli/src/commands/version/version.test.ts b/packages/cli/src/commands/version/version.test.ts
@@ -1081,6 +1081,63 @@ describe("workspace range", () => {
       },
     ]);
   });
+
+  it("should update dependant CHANGELOGs with 'Dependency update' information", async () => {
+    const cwd = await testdir({
+      "package.json": JSON.stringify({
+        private: true,
+        workspaces: ["packages/*"],
+      }),
+      "packages/pkg-a/package.json": JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+      }),
+      "packages/pkg-b/package.json": JSON.stringify({
+        name: "pkg-b",
+        version: "1.0.0",
+        dependencies: { "pkg-a": "workspace:*" },
+      }),
+    });
+
+    const spy = jest.spyOn(fs, "writeFile");
+    await writeChangeset(
+      {
+        summary: "This is a summary",
+        releases: [{ name: "pkg-a", type: "minor" }],
+      },
+      cwd
+    );
+    await version(cwd, defaultOptions, {
+      ...modifiedDefaultConfig,
+      ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
+        ...defaultConfig.___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH,
+        updateInternalDependents: "always",
+      },
+    });
+
+    expect(getChangelog("pkg-a", spy.mock.calls)).toMatchInlineSnapshot(`
+      "# pkg-a
+
+      ## 1.1.0
+
+      ### Minor Changes
+
+      - g1th4sh: This is a summary
+      "
+    `);
+
+    expect(getChangelog("pkg-b", spy.mock.calls)).toMatchInlineSnapshot(`
+      "# pkg-b
+
+      ## 1.0.1
+
+      ### Patch Changes
+
+      - Updated dependencies [g1th4sh]
+        - [email protected]
+      "
+    `);
+  });
 });
 
 describe("same package in different dependency types", () => {
@@ -1839,6 +1896,141 @@ describe("updateInternalDependents: always", () => {
     expect(() => getChangelog("pkg-b", spy.mock.calls)).toThrowError();
     expect(() => getChangelog("pkg-c", spy.mock.calls)).toThrowError();
   });
+
+  it("should not bump dependant when it depends on an npm tag of a bumped dependency", async () => {
+    const cwd = await testdir({
+      "package.json": JSON.stringify({
+        private: true,
+        workspaces: ["packages/*"],
+      }),
+      "packages/pkg-a/package.json": JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+      }),
+      "packages/pkg-b/package.json": JSON.stringify({
+        name: "pkg-b",
+        version: "1.0.0",
+        dependencies: { "pkg-a": "bulbasaur" }, // using tag version from npm
+      }),
+    });
+
+    const spy = jest.spyOn(fs, "writeFile");
+
+    await writeChangeset(
+      {
+        summary: "This is some fix",
+        releases: [{ name: "pkg-b", type: "patch" }],
+      },
+      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)).toThrow();
+
+    expect(getPkgJSON("pkg-b", spy.mock.calls)).toEqual(
+      expect.objectContaining({
+        name: "pkg-b",
+        version: "1.0.1",
+        dependencies: { "pkg-a": "bulbasaur" },
+      })
+    );
+
+    expect(() => getChangelog("pkg-a", spy.mock.calls)).toThrow();
+
+    expect(getChangelog("pkg-b", spy.mock.calls)).toMatchInlineSnapshot(`
+      "# pkg-b
+
+      ## 1.0.1
+
+      ### Patch Changes
+
+      - g1th4sh: This is some fix
+      "
+    `);
+  });
+
+  it("should bump dependant when it depend on an npm tag of a bumped dependency when it has its own changeset", async () => {
+    const cwd = await testdir({
+      "package.json": JSON.stringify({
+        private: true,
+        workspaces: ["packages/*"],
+      }),
+      "packages/pkg-a/package.json": JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+      }),
+      "packages/pkg-b/package.json": JSON.stringify({
+        name: "pkg-b",
+        version: "1.0.0",
+        dependencies: { "pkg-a": "bulbasaur" }, // using tag version from npm
+      }),
+    });
+
+    const spy = jest.spyOn(fs, "writeFile");
+    await writeChangeset(
+      {
+        summary: "This is a summary",
+        releases: [{ name: "pkg-a", type: "minor" }],
+      },
+      cwd
+    );
+    await writeChangeset(
+      {
+        summary: "This is some fix",
+        releases: [{ name: "pkg-b", type: "patch" }],
+      },
+      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.1",
+        dependencies: { "pkg-a": "bulbasaur" },
+      })
+    );
+
+    expect(getChangelog("pkg-a", spy.mock.calls)).toMatchInlineSnapshot(`
+      "# pkg-a
+
+      ## 1.1.0
+
+      ### Minor Changes
+
+      - g1th4sh: This is a summary
+      "
+    `);
+
+    expect(getChangelog("pkg-b", spy.mock.calls)).toMatchInlineSnapshot(`
+      "# pkg-b
+
+      ## 1.0.1
+
+      ### Patch Changes
+
+      - g1th4sh: This is some fix
+      "
+    `);
+  });
 });
 
 describe("pre", () => {