Fixed changeset status combined with ignored and non-versionable pr…

…ivate packages (#1354)

* Fixed `changeset status` combined with ignored and non-versionable private packages

* add tests

* extract common bits

.changeset/beige-lies-beam.md

@@ -0,0 +1,8 @@
+---
+"@changesets/cli": patch
+---
+
+Fixed an issue with `changeset status` incorrectly returning an error status in two cases:
+
+- for changed ignored packages
+- for changed private packages when `privatePackage.version` was set to `false`

packages/cli/src/commands/add/index.ts

@@ -1,19 +1,22 @@
 import chalk from "chalk";
-import path from "path";
 import { spawn } from "child_process";
+import path from "path";
 
-import * as cli from "../../utils/cli-utilities";
 import * as git from "@changesets/git";
 import { info, log, warn } from "@changesets/logger";
 import { Config } from "@changesets/types";
-import { getPackages } from "@manypkg/get-packages";
 import writeChangeset from "@changesets/write";
+import { getPackages } from "@manypkg/get-packages";
+import * as cli from "../../utils/cli-utilities";
 
+import { ExternalEditor } from "external-editor";
 import { getCommitFunctions } from "../../commit/getCommitFunctions";
+import {
+  filterVersionablePackages,
+  getVersionableChangedPackages,
+} from "../../utils/versionablePackages";
 import createChangeset from "./createChangeset";
 import printConfirmationMessage from "./messages";
-import { ExternalEditor } from "external-editor";
-import { isListablePackage } from "./isListablePackage";
 
 export default async function add(
   cwd: string,
@@ -26,8 +29,9 @@ export default async function add(
       `No packages found. You might have ${packages.tool} workspaces configured but no packages yet?`
     );
   }
-  const listablePackages = packages.packages.filter((pkg) =>
-    isListablePackage(config, pkg.packageJson)
+  const versionablePackages = filterVersionablePackages(
+    config,
+    packages.packages
   );
   const changesetBase = path.resolve(cwd, ".changeset");
 
@@ -39,17 +43,17 @@ export default async function add(
       summary: ``,
     };
   } else {
-    const changedPackages = await git.getChangedPackagesSinceRef({
-      cwd,
-      ref: config.baseBranch,
-      changedFilePatterns: config.changedFilePatterns,
-    });
-    const changedPackagesName = changedPackages
-      .filter((pkg) => isListablePackage(config, pkg.packageJson))
-      .map((pkg) => pkg.packageJson.name);
+    const changedPackagesNames = (
+      await getVersionableChangedPackages(config, {
+        cwd,
+      })
+    ).map((pkg) => pkg.packageJson.name);
 
-    newChangeset = await createChangeset(changedPackagesName, listablePackages);
-    printConfirmationMessage(newChangeset, listablePackages.length > 1);
+    newChangeset = await createChangeset(
+      changedPackagesNames,
+      versionablePackages
+    );
+    printConfirmationMessage(newChangeset, versionablePackages.length > 1);
 
     if (!newChangeset.confirmed) {
       newChangeset = {

packages/cli/src/commands/status/__tests__/status.ts

@@ -1,13 +1,18 @@
-import { defaultConfig } from "@changesets/config";
+import { defaultConfig, read } from "@changesets/config";
 import * as git from "@changesets/git";
 import { gitdir, silenceLogsInBlock } from "@changesets/test-utils";
 import { ReleasePlan } from "@changesets/types";
 import writeChangeset from "@changesets/write";
+import { getPackages } from "@manypkg/get-packages";
 import fs from "fs-extra";
 import path from "path";
 import spawn from "spawndamnit";
 import status from "..";
 
+async function readConfig(cwd: string) {
+  return read(cwd, await getPackages(cwd));
+}
+
 function replaceHumanIds(releaseObj: ReleasePlan | undefined) {
   if (!releaseObj) {
     return;
@@ -412,4 +417,101 @@ describe("status", () => {
       }
     `);
   });
+
+  it("should not exit early with a non-zero error code when only changed packages are ignored", async () => {
+    const cwd = await gitdir({
+      "package.json": JSON.stringify({
+        private: true,
+        workspaces: ["packages/*"],
+      }),
+      "packages/pkg-a/package.json": JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+      }),
+      "packages/pkg-a/src/a.js": 'export default "a"',
+      "packages/pkg-b/package.json": JSON.stringify({
+        name: "pkg-b",
+        version: "1.0.0",
+      }),
+      "packages/pkg-b/src/b.js": 'export default "b"',
+      ".changeset/config.json": JSON.stringify({
+        ignore: ["pkg-b"],
+      }),
+    });
+
+    // @ts-ignore
+    jest.spyOn(process, "exit").mockImplementation(() => {});
+
+    await spawn("git", ["checkout", "-b", "new-branch"], { cwd });
+
+    await fs.outputFile(
+      path.join(cwd, "packages/pkg-b/b.js"),
+      'export default "updated b"'
+    );
+    await git.add(".", cwd);
+    await git.commit("updated b", cwd);
+
+    const releaseObj = await status(
+      cwd,
+      { since: "main" },
+      await readConfig(cwd)
+    );
+
+    expect(process.exit).not.toHaveBeenCalled();
+    expect(releaseObj).toEqual({
+      changesets: [],
+      releases: [],
+      preState: undefined,
+    });
+  });
+
+  it("should not exit early with a non-zero error code when only changed packages are private and versioning for private packages is turned off", async () => {
+    const cwd = await gitdir({
+      "package.json": JSON.stringify({
+        private: true,
+        workspaces: ["packages/*"],
+      }),
+      "packages/pkg-a/package.json": JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+      }),
+      "packages/pkg-a/src/a.js": 'export default "a"',
+      "packages/pkg-b/package.json": JSON.stringify({
+        name: "pkg-b",
+        private: true,
+        version: "1.0.0",
+      }),
+      "packages/pkg-b/src/b.js": 'export default "b"',
+      ".changeset/config.json": JSON.stringify({
+        privatePackages: {
+          version: false,
+        },
+      }),
+    });
+
+    // @ts-ignore
+    jest.spyOn(process, "exit").mockImplementation(() => {});
+
+    await spawn("git", ["checkout", "-b", "new-branch"], { cwd });
+
+    await fs.outputFile(
+      path.join(cwd, "packages/pkg-b/b.js"),
+      'export default "updated b"'
+    );
+    await git.add(".", cwd);
+    await git.commit("updated b", cwd);
+
+    const releaseObj = await status(
+      cwd,
+      { since: "main" },
+      await readConfig(cwd)
+    );
+
+    expect(process.exit).not.toHaveBeenCalled();
+    expect(releaseObj).toEqual({
+      changesets: [],
+      releases: [],
+      preState: undefined,
+    });
+  });
 });

packages/cli/src/commands/status/index.ts

@@ -1,17 +1,17 @@
 import chalk from "chalk";
-import table from "tty-table";
 import fs from "fs-extra";
 import path from "path";
+import table from "tty-table";
 
-import * as git from "@changesets/git";
 import getReleasePlan from "@changesets/get-release-plan";
-import { error, log, info, warn } from "@changesets/logger";
+import { error, info, log, warn } from "@changesets/logger";
 import {
-  VersionType,
-  Release,
   ComprehensiveRelease,
   Config,
+  Release,
+  VersionType,
 } from "@changesets/types";
+import { getVersionableChangedPackages } from "../../utils/versionablePackages";
 
 export default async function getStatus(
   cwd: string,
@@ -38,10 +38,9 @@ export default async function getStatus(
     since === undefined ? (sinceMaster ? "master" : undefined) : since;
   const releasePlan = await getReleasePlan(cwd, sinceBranch, config);
   const { changesets, releases } = releasePlan;
-  const changedPackages = await git.getChangedPackagesSinceRef({
+  const changedPackages = await getVersionableChangedPackages(config, {
     cwd,
-    ref: sinceBranch || config.baseBranch,
-    changedFilePatterns: config.changedFilePatterns,
+    ref: sinceBranch,
   });
 
   if (changedPackages.length > 0 && changesets.length === 0) {

packages/cli/src/utils/versionablePackages.ts

@@ -0,0 +1,48 @@
+import { Config } from "@changesets/types";
+import { getChangedPackagesSinceRef } from "@changesets/git";
+import { Package } from "@manypkg/get-packages";
+
+function isVersionablePackage(
+  { packageJson }: Package,
+  {
+    ignoredPackages,
+    versionPrivatePackages,
+  }: {
+    ignoredPackages: Set<string>;
+    versionPrivatePackages: boolean;
+  }
+) {
+  if (ignoredPackages.has(packageJson.name)) {
+    return false;
+  }
+
+  if (packageJson.private && !versionPrivatePackages) {
+    return false;
+  }
+
+  const hasVersionField = !!packageJson.version;
+  return hasVersionField;
+}
+
+export function filterVersionablePackages(config: Config, packages: Package[]) {
+  const options = {
+    ignoredPackages: new Set(config.ignore),
+    versionPrivatePackages: config.privatePackages.version,
+  };
+  return packages.filter((pkg) => isVersionablePackage(pkg, options));
+}
+
+export async function getVersionableChangedPackages(
+  config: Config,
+  options: {
+    cwd: string;
+    ref?: string;
+  }
+) {
+  const changedPackages = await getChangedPackagesSinceRef({
+    ref: config.baseBranch,
+    changedFilePatterns: config.changedFilePatterns,
+    ...options,
+  });
+  return filterVersionablePackages(config, changedPackages);
+}