Fix unversioned private package handling, tagging of ignored (#1361)

* Try and fix unversioned private package handling

* More todos

* Do some more

* remove todo

* More

* rename

* Fix test build

* Fix tag test

* Fix forgotten condition swap

* Add a test for version

* Changeset

* Add test for apply-release-plan

* Update func

* Drop todos, PR commenting is enough

* Resolve TODOs

* one last test

* tweak stuff

* tweak test title

* Update .changeset/sharp-humans-battle.md

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

.changeset/sharp-humans-battle.md

@@ -0,0 +1,7 @@
+---
+"@changesets/assemble-release-plan": patch
+"@changesets/apply-release-plan": patch
+"@changesets/cli": patch
+---
+
+Ensure that `version`/`tag` do not touch private packages with when versioning/tagging is turned off using `versionPackages` config

packages/apply-release-plan/package.json

@@ -23,6 +23,7 @@
     "@changesets/config": "^3.0.0",
     "@changesets/get-version-range-type": "^0.4.0",
     "@changesets/git": "^3.0.0",
+    "@changesets/should-skip-package": "^0.1.0",
     "@changesets/types": "^6.0.0",
     "@manypkg/get-packages": "^1.1.3",
     "detect-indent": "^6.0.0",

packages/apply-release-plan/src/index.test.ts

@@ -2889,6 +2889,46 @@ describe("apply release plan", () => {
       let pathExists = await fs.pathExists(changesetPath);
       expect(pathExists).toEqual(true);
     });
+    it("should NOT delete changesets for private unversioned packages", async () => {
+      const releasePlan = new FakeReleasePlan();
+
+      let changesetPath: string;
+
+      const setupFunc = (tempDir: string) =>
+        Promise.all(
+          releasePlan.getReleasePlan().changesets.map(({ id, summary }) => {
+            const thisPath = path.resolve(tempDir, ".changeset", `${id}.md`);
+            changesetPath = thisPath;
+            const content = `---\n---\n${summary}`;
+            return fs.outputFile(thisPath, content);
+          })
+        );
+
+      await testSetup(
+        {
+          "package.json": JSON.stringify({
+            private: true,
+            workspaces: ["packages/*"],
+          }),
+          "packages/pkg-a/package.json": JSON.stringify({
+            name: "pkg-a",
+            version: "1.0.0",
+            private: true,
+          }),
+        },
+        releasePlan.getReleasePlan(),
+        {
+          ...releasePlan.config,
+          privatePackages: { version: false, tag: false },
+        },
+        undefined,
+        setupFunc
+      );
+
+      // @ts-ignore this is possibly bad
+      let pathExists = await fs.pathExists(changesetPath);
+      expect(pathExists).toEqual(true);
+    });
     it("should delete an old format changeset if it is applied", async () => {
       const releasePlan = new FakeReleasePlan();
 

packages/apply-release-plan/src/index.ts

@@ -1,23 +1,21 @@
+import { defaultConfig } from "@changesets/config";
+import * as git from "@changesets/git";
+import { shouldSkipPackage } from "@changesets/should-skip-package";
 import {
-  ReleasePlan,
-  Config,
   ChangelogFunctions,
-  NewChangeset,
+  Config,
   ModCompWithPackage,
+  NewChangeset,
+  ReleasePlan,
 } from "@changesets/types";
-
-import { defaultConfig } from "@changesets/config";
-import * as git from "@changesets/git";
-import resolveFrom from "resolve-from";
 import { Packages } from "@manypkg/get-packages";
 import detectIndent from "detect-indent";
-
 import fs from "fs-extra";
 import path from "path";
 import prettier from "prettier";
-
-import versionPackage from "./version-package";
+import resolveFrom from "resolve-from";
 import getChangelogEntry from "./get-changelog-entry";
+import versionPackage from "./version-package";
 
 function getPrettierInstance(cwd: string): typeof prettier {
   try {
@@ -158,14 +156,17 @@ export default async function applyReleasePlan(
         let changesetPath = path.resolve(changesetFolder, `${changeset.id}.md`);
         let changesetFolderPath = path.resolve(changesetFolder, changeset.id);
         if (await fs.pathExists(changesetPath)) {
-          // DO NOT remove changeset for ignored packages
-          // Mixed changeset that contains both ignored packages and not ignored packages are disallowed
+          // DO NOT remove changeset for skipped packages
+          // Mixed changeset that contains both skipped packages and not skipped packages are disallowed
           // At this point, we know there is no such changeset, because otherwise the program would've already failed,
-          // so we just check if any ignored package exists in this changeset, and only remove it if none exists
-          // Ignored list is added in v2, so we don't need to do it for v1 changesets
+          // so we just check if any skipped package exists in this changeset, and only remove it if none exists
+          // options to skip packages were added in v2, so we don't need to do it for v1 changesets
           if (
             !changeset.releases.find((release) =>
-              config.ignore.includes(release.name)
+              shouldSkipPackage(packagesByName.get(release.name)!, {
+                ignore: config.ignore,
+                allowPrivatePackages: config.privatePackages.version,
+              })
             )
           ) {
             touchedFiles.push(changesetPath);

packages/assemble-release-plan/package.json

@@ -22,6 +22,7 @@
     "@babel/runtime": "^7.20.1",
     "@changesets/errors": "^0.2.0",
     "@changesets/get-dependents-graph": "^2.0.0",
+    "@changesets/should-skip-package": "^0.1.0",
     "@changesets/types": "^6.0.0",
     "@manypkg/get-packages": "^1.1.3",
     "semver": "^7.5.3"

packages/assemble-release-plan/src/determine-dependents.ts

@@ -1,13 +1,14 @@
-import semverSatisfies from "semver/functions/satisfies";
+import { shouldSkipPackage } from "@changesets/should-skip-package";
 import {
+  Config,
   DependencyType,
   PackageJSON,
   VersionType,
-  Config,
 } from "@changesets/types";
 import { Package } from "@manypkg/get-packages";
-import { InternalRelease, PreInfo } from "./types";
+import semverSatisfies from "semver/functions/satisfies";
 import { incrementVersion } from "./increment";
+import { InternalRelease, PreInfo } from "./types";
 
 /*
   WARNING:
@@ -56,7 +57,12 @@ export default function determineDependents({
         const dependentPackage = packagesByName.get(dependent);
         if (!dependentPackage) throw new Error("Dependency map is incorrect");
 
-        if (config.ignore.includes(dependent)) {
+        if (
+          shouldSkipPackage(dependentPackage, {
+            ignore: config.ignore,
+            allowPrivatePackages: config.privatePackages.version,
+          })
+        ) {
           type = "none";
         } else {
           const dependencyVersionRanges = getDependencyVersionRanges(

packages/assemble-release-plan/src/flatten-releases.ts

@@ -1,22 +1,29 @@
 // This function takes in changesets and returns one release per
 // package listed in the changesets
 
-import { NewChangeset } from "@changesets/types";
+import { shouldSkipPackage } from "@changesets/should-skip-package";
+import { Config, NewChangeset } from "@changesets/types";
 import { Package } from "@manypkg/get-packages";
 import { InternalRelease } from "./types";
 
 export default function flattenReleases(
   changesets: NewChangeset[],
   packagesByName: Map<string, Package>,
-  ignoredPackages: Readonly<string[]>
+  config: Config
 ): Map<string, InternalRelease> {
   let releases: Map<string, InternalRelease> = new Map();
 
   changesets.forEach((changeset) => {
     changeset.releases
-      // Filter out ignored packages because they should not trigger a release
+      // Filter out skipped packages because they should not trigger a release
       // If their dependencies need updates, they will be added to releases by `determineDependents()` with release type `none`
-      .filter(({ name }) => !ignoredPackages.includes(name))
+      .filter(
+        ({ name }) =>
+          !shouldSkipPackage(packagesByName.get(name)!, {
+            ignore: config.ignore,
+            allowPrivatePackages: config.privatePackages.version,
+          })
+      )
       .forEach(({ name, type }) => {
         let release = releases.get(name);
         let pkg = packagesByName.get(name);

packages/assemble-release-plan/src/index.ts

@@ -1,20 +1,21 @@
+import { InternalError } from "@changesets/errors";
+import { getDependentsGraph } from "@changesets/get-dependents-graph";
+import { shouldSkipPackage } from "@changesets/should-skip-package";
 import {
-  ReleasePlan,
   Config,
   NewChangeset,
-  PreState,
   PackageGroup,
+  PreState,
+  ReleasePlan,
 } from "@changesets/types";
+import { Package, Packages } from "@manypkg/get-packages";
+import semverParse from "semver/functions/parse";
+import applyLinks from "./apply-links";
 import determineDependents from "./determine-dependents";
 import flattenReleases from "./flatten-releases";
-import matchFixedConstraint from "./match-fixed-constraint";
-import applyLinks from "./apply-links";
 import { incrementVersion } from "./increment";
-import semverParse from "semver/functions/parse";
-import { InternalError } from "@changesets/errors";
-import { Packages, Package } from "@manypkg/get-packages";
-import { getDependentsGraph } from "@changesets/get-dependents-graph";
-import { PreInfo, InternalRelease } from "./types";
+import matchFixedConstraint from "./match-fixed-constraint";
+import { InternalRelease, PreInfo } from "./types";
 
 type SnapshotReleaseParameters = {
   tag?: string | undefined;
@@ -156,7 +157,8 @@ function assembleReleasePlan(
 
   const relevantChangesets = getRelevantChangesets(
     changesets,
-    refinedConfig.ignore,
+    packagesByName,
+    refinedConfig,
     preState
   );
 
@@ -173,7 +175,7 @@ function assembleReleasePlan(
   let releases = flattenReleases(
     relevantChangesets,
     packagesByName,
-    refinedConfig.ignore
+    refinedConfig
   );
 
   let dependencyGraph = getDependentsGraph(packages, {
@@ -224,7 +226,10 @@ function assembleReleasePlan(
           });
         } else if (
           existingRelease.type === "none" &&
-          !refinedConfig.ignore.includes(pkg.packageJson.name)
+          !shouldSkipPackage(pkg, {
+            ignore: refinedConfig.ignore,
+            allowPrivatePackages: refinedConfig.privatePackages.version,
+          })
         ) {
           existingRelease.type = "patch";
         }
@@ -261,31 +266,33 @@ function assembleReleasePlan(
 
 function getRelevantChangesets(
   changesets: NewChangeset[],
-  ignored: Readonly<string[]>,
+  packagesByName: Map<string, Package>,
+  config: Config,
   preState: PreState | undefined
 ): NewChangeset[] {
   for (const changeset of changesets) {
     // Using the following 2 arrays to decide whether a changeset
-    // contains both ignored and not ignored packages
-    const ignoredPackages = [];
-    const notIgnoredPackages = [];
+    // contains both skipped and not skipped packages
+    const skippedPackages = [];
+    const notSkippedPackages = [];
     for (const release of changeset.releases) {
       if (
-        ignored.find(
-          (ignoredPackageName) => ignoredPackageName === release.name
-        )
+        shouldSkipPackage(packagesByName.get(release.name)!, {
+          ignore: config.ignore,
+          allowPrivatePackages: config.privatePackages.version,
+        })
       ) {
-        ignoredPackages.push(release.name);
+        skippedPackages.push(release.name);
       } else {
-        notIgnoredPackages.push(release.name);
+        notSkippedPackages.push(release.name);
       }
     }
 
-    if (ignoredPackages.length > 0 && notIgnoredPackages.length > 0) {
+    if (skippedPackages.length > 0 && notSkippedPackages.length > 0) {
       throw new Error(
         `Found mixed changeset ${changeset.id}\n` +
-          `Found ignored packages: ${ignoredPackages.join(" ")}\n` +
-          `Found not ignored packages: ${notIgnoredPackages.join(" ")}\n` +
+          `Found ignored packages: ${skippedPackages.join(" ")}\n` +
+          `Found not ignored packages: ${notSkippedPackages.join(" ")}\n` +
           "Mixed changesets that contain both ignored and not ignored packages are not allowed"
       );
     }

packages/assemble-release-plan/src/match-fixed-constraint.ts

@@ -1,3 +1,4 @@
+import { shouldSkipPackage } from "@changesets/should-skip-package";
 import { Config } from "@changesets/types";
 import { Package } from "@manypkg/get-packages";
 import { InternalRelease } from "./types";
@@ -26,7 +27,12 @@ export default function matchFixedConstraint(
 
     // Finally, we update the packages so all of them are on the highest version
     for (let pkgName of fixedPackages) {
-      if (config.ignore.includes(pkgName)) {
+      if (
+        shouldSkipPackage(packagesByName.get(pkgName)!, {
+          ignore: config.ignore,
+          allowPrivatePackages: config.privatePackages.version,
+        })
+      ) {
         continue;
       }
       let release = releases.get(pkgName);

packages/cli/package.json

@@ -78,6 +78,7 @@
     "@changesets/logger": "^0.1.0",
     "@changesets/pre": "^2.0.0",
     "@changesets/read": "^0.6.0",
+    "@changesets/should-skip-package": "^0.1.0",
     "@changesets/types": "^6.0.0",
     "@changesets/write": "^0.3.1",
     "@manypkg/get-packages": "^1.1.3",

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

@@ -4,17 +4,14 @@ import path from "path";
 
 import * as git from "@changesets/git";
 import { info, log, warn } from "@changesets/logger";
+import { shouldSkipPackage } from "@changesets/should-skip-package";
 import { Config } from "@changesets/types";
 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 * as cli from "../../utils/cli-utilities";
+import { getVersionableChangedPackages } from "../../utils/versionablePackages";
 import createChangeset from "./createChangeset";
 import printConfirmationMessage from "./messages";
 
@@ -29,9 +26,12 @@ export default async function add(
       `No packages found. You might have ${packages.tool} workspaces configured but no packages yet?`
     );
   }
-  const versionablePackages = filterVersionablePackages(
-    config,
-    packages.packages
+  const versionablePackages = packages.packages.filter(
+    (pkg) =>
+      !shouldSkipPackage(pkg, {
+        ignore: config.ignore,
+        allowPrivatePackages: config.privatePackages.version,
+      })
   );
   const changesetBase = path.resolve(cwd, ".changeset");
 

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

@@ -38,7 +38,7 @@ function showNonLatestTagWarning(tag?: string, preState?: PreState) {
   warn(importantEnd);
 }
 
-export default async function run(
+export default async function publish(
   cwd: string,
   { otp, tag, gitTag = true }: { otp?: string; tag?: string; gitTag?: boolean },
   config: Config

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

@@ -13,7 +13,7 @@ import {
 } from "@changesets/types";
 import { getVersionableChangedPackages } from "../../utils/versionablePackages";
 
-export default async function getStatus(
+export default async function status(
   cwd: string,
   {
     sinceMaster,