Accept tag dependencies as valid (#704)

.changeset/chilled-turkeys-poke.md

@@ -0,0 +1,5 @@
+---
+"@changesets/get-dependents-graph": minor
+---
+
+Dependencies specified using a tag will no longer mark the graph as invalid. With such dependencies the user's intent is to fetch those from the registry even if otherwise they could be linked locally.

.changeset/pink-jokes-shave.md

@@ -0,0 +1,5 @@
+---
+"@changesets/cli": minor
+---
+
+Dependencies specified using a tag will no longer result in printing incorrect errors in the console.

__fixtures__/dependant-with-tag-range/.changeset/README.md

@@ -0,0 +1 @@
+# We just want a file in here so git collects it

__fixtures__/dependant-with-tag-range/.changeset/config.json

@@ -0,0 +1 @@
+{}

__fixtures__/dependant-with-tag-range/examples/example-a/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "example-a",
+  "version": "1.0.0",
+  "dependencies": {
+    "pkg-a": "latest"
+  }
+}

__fixtures__/dependant-with-tag-range/package.json

@@ -0,0 +1,10 @@
+{
+  "private": true,
+  "name": "dependant-with-tag-range",
+  "description": "example depending on latest tag",
+  "version": "1.0.0",
+  "workspaces": [
+    "examples/*",
+    "packages/*"
+  ]
+}

__fixtures__/dependant-with-tag-range/packages/pkg-a/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "pkg-a",
+  "version": "1.0.0"
+}

packages/cli/src/commands/version/version.test.ts

@@ -267,6 +267,37 @@ describe("running version in a simple project", () => {
     expect(changesetDir).toContain(".ignored-temporarily.md");
   });
 
+  it("should not update a dependant that uses a tag as a dependency rage for a package that could otherwise be local", async () => {
+    const cwd = await f.copy("dependant-with-tag-range");
+
+    await writeChangeset(
+      {
+        releases: [{ name: "pkg-a", type: "major" }],
+        summary: "a very useful summary for the change"
+      },
+      cwd
+    );
+
+    await version(cwd, defaultOptions, modifiedDefaultConfig);
+
+    expect((await getPackages(cwd)).packages.map(x => x.packageJson))
+      .toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "dependencies": Object {
+            "pkg-a": "latest",
+          },
+          "name": "example-a",
+          "version": "1.0.0",
+        },
+        Object {
+          "name": "pkg-a",
+          "version": "2.0.0",
+        },
+      ]
+    `);
+  });
+
   describe("when there are multiple changeset commits", () => {
     it("should bump releasedPackages", async () => {
       await writeChangesets([simpleChangeset, simpleChangeset2], cwd);

packages/get-dependents-graph/src/get-dependency-graph.test.ts

@@ -1,5 +1,15 @@
 import getDependencyGraph from "./get-dependency-graph";
 
+const consoleError = console.error;
+
+beforeEach(async () => {
+  console.error = jest.fn();
+});
+
+afterEach(async () => {
+  console.error = consoleError;
+});
+
 describe("getting the dependency graph", function() {
   it("should skip dependencies specified through the link protocol", function() {
     const { graph, valid } = getDependencyGraph({
@@ -30,7 +40,41 @@ describe("getting the dependency graph", function() {
     });
     expect(graph.get("foo")!.dependencies).toStrictEqual([]);
     expect(valid).toBeTruthy();
+    expect((console.error as any).mock.calls).toMatchInlineSnapshot(`Array []`);
   });
+
+  it("should skip dependencies specified using a tag", function() {
+    const { graph, valid } = getDependencyGraph({
+      root: {
+        dir: ".",
+        packageJson: { name: "root", version: "1.0.0" }
+      },
+      packages: [
+        {
+          dir: "examples/foo",
+          packageJson: {
+            name: "foo-example",
+            version: "1.0.0",
+            dependencies: {
+              bar: "latest"
+            }
+          }
+        },
+        {
+          dir: "packages/bar",
+          packageJson: {
+            name: "bar",
+            version: "1.0.0"
+          }
+        }
+      ],
+      tool: "pnpm"
+    });
+    expect(graph.get("foo-example")!.dependencies).toStrictEqual([]);
+    expect(valid).toBeTruthy();
+    expect((console.error as any).mock.calls).toMatchInlineSnapshot(`Array []`);
+  });
+
   it("should set valid to false if the link protocol is used in a non-dev dep", function() {
     const { valid } = getDependencyGraph({
       root: {
@@ -59,5 +103,12 @@ describe("getting the dependency graph", function() {
       tool: "pnpm"
     });
     expect(valid).toBeFalsy();
+    expect((console.error as any).mock.calls).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "Package \\"foo\\" must depend on the current version of \\"bar\\": \\"1.0.0\\" vs \\"link:../bar\\"",
+        ],
+      ]
+    `);
   });
 });

packages/get-dependents-graph/src/get-dependency-graph.ts

@@ -19,21 +19,35 @@ const getAllDependencies = (config: PackageJSON) => {
     if (!deps) continue;
 
     for (const name of Object.keys(deps)) {
-      const depVersion = deps[name];
+      const depRange = deps[name];
       if (
-        (depVersion.startsWith("link:") || depVersion.startsWith("file:")) &&
+        (depRange.startsWith("link:") || depRange.startsWith("file:")) &&
         type === "devDependencies"
       ) {
         continue;
       }
 
-      allDependencies.set(name, depVersion);
+      allDependencies.set(name, depRange);
     }
   }
 
   return allDependencies;
 };
 
+const isProtocolRange = (range: string) => range.indexOf(":") !== -1;
+
+const getValidRange = (potentialRange: string) => {
+  if (isProtocolRange(potentialRange)) {
+    return null;
+  }
+
+  try {
+    return new semver.Range(potentialRange);
+  } catch {
+    return null;
+  }
+};
+
 export default function getDependencyGraph(
   packages: Packages,
   opts?: {
@@ -65,39 +79,44 @@ export default function getDependencyGraph(
     const dependencies = [];
     const allDependencies = getAllDependencies(pkg.packageJson);
 
-    for (let [depName, depVersion] of allDependencies) {
+    for (let [depName, depRange] of allDependencies) {
       const match = packagesByName[depName];
       if (!match) continue;
 
       const expected = match.packageJson.version;
-      const usesWorkspaceRange = depVersion.startsWith("workspace:");
+      const usesWorkspaceRange = depRange.startsWith("workspace:");
 
       if (usesWorkspaceRange) {
-        depVersion = depVersion.replace(/^workspace:/, "");
+        depRange = depRange.replace(/^workspace:/, "");
 
-        if (depVersion === "*" || depVersion === "^" || depVersion === "~") {
+        if (depRange === "*" || depRange === "^" || depRange === "~") {
           dependencies.push(depName);
           continue;
         }
       } else if (opts?.bumpVersionsWithWorkspaceProtocolOnly === true) {
         continue;
       }
 
-      // internal dependencies only need to semver satisfy, not '==='
-      if (!semver.satisfies(expected, depVersion)) {
+      const range = getValidRange(depRange);
+
+      if ((range && !range.test(expected)) || isProtocolRange(depRange)) {
         valid = false;
         console.error(
           `Package ${chalk.cyan(
             `"${name}"`
           )} must depend on the current version of ${chalk.cyan(
             `"${depName}"`
-          )}: ${chalk.green(`"${expected}"`)} vs ${chalk.red(
-            `"${depVersion}"`
-          )}`
+          )}: ${chalk.green(`"${expected}"`)} vs ${chalk.red(`"${depRange}"`)}`
         );
         continue;
       }
 
+      // `depRange` could have been a tag and if a tag has been used there might have been a reason for that
+      // we should not count this as a local monorepro dependant
+      if (!range) {
+        continue;
+      }
+
       dependencies.push(depName);
     }
 

packages/types/src/index.ts

@@ -67,6 +67,7 @@ export type Config = {
   /** The minimum bump type to trigger automatic update of internal dependencies that are part of the same release */
   updateInternalDependencies: "patch" | "minor";
   ignore: ReadonlyArray<string>;
+  /** This is supposed to be used with pnpm's `link-workspace-packages: false` and Berry's `enableTransparentWorkspaces: false` */
   bumpVersionsWithWorkspaceProtocolOnly?: boolean;
   ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: Required<
     ExperimentalOptions