Fix a bug with prereleases where a package would not be bumped with t…

…he highest bump type of all changesets from previous prereleases within the same prerelease run (#263)

* Fix a bug with prereleases where a package would not be bumped with the highest bump type of all changesets from previous prereleases within the same prerelease run

* A better test name

* Fix some things and add a VSCode config for debugging tests

* Remove a console.log

* Update dirty-coins-enjoy.md

.changeset/dirty-coins-enjoy.md

@@ -0,0 +1,6 @@
+---
+"@changesets/assemble-release-plan": patch
+"@changesets/cli": patch
+---
+
+Fixed a bug where only the unreleased pre-release changesets were taken into account when calculating the new version, not previously released changesets.

.vscode/launch.json

@@ -0,0 +1,18 @@
+{
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "Debug Jest Tests",
+      "type": "node",
+      "request": "launch",
+      "runtimeArgs": [
+        "--inspect-brk",
+        "${workspaceRoot}/node_modules/.bin/jest",
+        "--runInBand"
+      ],
+      "console": "integratedTerminal",
+      "internalConsoleOptions": "neverOpen",
+      "port": 9229
+    }
+  ]
+}

packages/assemble-release-plan/src/apply-links.ts

@@ -16,7 +16,7 @@ import { InternalRelease } from "./types";
 */
 function applyLinks(
   releases: Map<string, InternalRelease>,
-  workspaces: Workspace[],
+  workspacesByName: Map<string, Workspace>,
   linked: Linked
 ): boolean {
   let updated = false;
@@ -49,9 +49,7 @@ function applyLinks(
 
     // Next we determine what the highest version among the linked packages will be
     for (let linkedPackage of linkedPackages) {
-      let workspace = workspaces.find(
-        workspace => workspace.name === linkedPackage
-      );
+      let workspace = workspacesByName.get(linkedPackage);
 
       if (workspace) {
         if (

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

@@ -23,20 +23,14 @@ import { incrementVersion } from "./increment";
 */
 export default function getDependents(
   releases: Map<string, InternalRelease>,
-  workspaces: Workspace[],
+  workspacesByName: Map<string, Workspace>,
   dependencyGraph: Map<string, string[]>,
   preInfo: PreInfo | undefined
 ): boolean {
   let updated = false;
   // NOTE this is intended to be called recursively
   let pkgsToSearch = [...releases.values()];
 
-  let pkgJsonsByName = new Map(
-    // TODO this seems an inefficient use of getting the whole workspaces
-    // Should we ask for this to be simplified 'above'?
-    workspaces.map(({ name, config }) => [name, config])
-  );
-
   while (pkgsToSearch.length > 0) {
     // nextRelease is our dependency, think of it as "avatar"
     const nextRelease = pkgsToSearch.shift();
@@ -54,10 +48,10 @@ export default function getDependents(
       .map(dependent => {
         let type: VersionType | undefined;
 
-        const dependentPkgJSON = pkgJsonsByName.get(dependent);
-        if (!dependentPkgJSON) throw new Error("Dependency map is incorrect");
+        const dependentWorkspace = workspacesByName.get(dependent);
+        if (!dependentWorkspace) throw new Error("Dependency map is incorrect");
         const { depTypes, versionRange } = getDependencyVersionRange(
-          dependentPkgJSON,
+          dependentWorkspace.config,
           nextRelease.name
         );
 
@@ -82,7 +76,7 @@ export default function getDependents(
             type = "patch";
           }
         }
-        return { name: dependent, type, pkgJSON: dependentPkgJSON };
+        return { name: dependent, type, pkgJSON: dependentWorkspace.config };
       })
       .filter(({ type }) => type)
       .forEach(

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

@@ -6,14 +6,14 @@ import { InternalRelease } from "./types";
 
 export default function flattenReleases(
   changesets: NewChangeset[],
-  workspaces: Workspace[]
+  workspacesByName: Map<string, Workspace>
 ): Map<string, InternalRelease> {
   let releases: Map<string, InternalRelease> = new Map();
 
   changesets.forEach(changeset => {
     changeset.releases.forEach(({ name, type }) => {
       let release = releases.get(name);
-      let ws = workspaces.find(ws => ws.name === name);
+      let ws = workspacesByName.get(name);
       if (!ws) {
         throw new Error(`Could not find package information for ${name}`);
       }

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

@@ -10,9 +10,8 @@ export function incrementVersion(
   if (preInfo !== undefined && preInfo.state.mode !== "exit") {
     let preVersion = preInfo.preVersions.get(release.name);
     if (preVersion === undefined) {
-      console.log(preInfo.preVersions.entries());
       throw new InternalError(
-        `preVersion for ${release.name} does not exis when preState is defined`
+        `preVersion for ${release.name} does not exist when preState is defined`
       );
     }
     // why are we adding this ourselves rather than passing 'pre' + versionType to semver.inc?

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

@@ -43,6 +43,9 @@ function assembleReleasePlan(
 
   let workspacesByName = new Map(workspaces.map(x => [x.name, x]));
 
+  let unfilteredChangesets = changesets;
+
+  let preVersions = new Map();
   if (updatedPreState !== undefined) {
     for (let workspace of workspaces) {
       if (updatedPreState.initialVersions[workspace.name] === undefined) {
@@ -56,21 +59,6 @@ function assembleReleasePlan(
       changesets = changesets.filter(
         changeset => !usedChangesetIds.has(changeset.id)
       );
-    }
-  }
-  // releases is, at this point a list of all packages we are going to releases,
-  // flattened down to one release per package, having a reference back to their
-  // changesets, and with a calculated new versions
-  let releases = flattenReleases(changesets, workspaces);
-  let preVersions = new Map();
-  if (updatedPreState !== undefined) {
-    for (let [name, release] of releases) {
-      releases.set(name, {
-        ...release,
-        oldVersion: updatedPreState.initialVersions[name]
-      });
-    }
-    if (updatedPreState.mode !== "exit") {
       for (let workspace of workspaces) {
         preVersions.set(
           workspace.name,
@@ -90,20 +78,66 @@ function assembleReleasePlan(
         }
       }
     }
+    for (let workspace of workspaces) {
+      workspacesByName.set(workspace.name, {
+        ...workspace,
+        config: {
+          ...workspace.config,
+          version: updatedPreState.initialVersions[workspace.name]
+        }
+      });
+    }
   }
 
-  if (updatedPreState !== undefined && updatedPreState.mode === "exit") {
-    for (let workspace of workspaces) {
-      if (semver.parse(workspace.config.version)!.prerelease.length) {
-        if (!releases.has(workspace.name)) {
-          releases.set(workspace.name, {
-            type: "patch",
-            name: workspace.name,
-            changesets: [],
-            oldVersion: updatedPreState.initialVersions[workspace.name]
-          });
+  // releases is, at this point a list of all packages we are going to releases,
+  // flattened down to one release per package, having a reference back to their
+  // changesets, and with a calculated new versions
+  let releases = flattenReleases(changesets, workspacesByName);
+
+  if (updatedPreState !== undefined) {
+    if (updatedPreState.mode === "exit") {
+      for (let workspace of workspaces) {
+        if (preVersions.get(workspace.name) !== -1) {
+          if (!releases.has(workspace.name)) {
+            releases.set(workspace.name, {
+              type: "patch",
+              name: workspace.name,
+              changesets: [],
+              oldVersion: workspace.config.version
+            });
+          }
         }
       }
+    } else {
+      // for every release in pre mode, we want versions to be bumped to the highest bump type
+      // across all the changesets even if the package doesn't have a changeset that releases
+      // to the highest bump type in a given release in pre mode and importantly
+      // we don't want to add any new releases, we only want to update ones that will already happen
+      // because if they're not being released, the version will already have been bumped with the highest bump type
+      let releasesFromUnfilteredChangesets = flattenReleases(
+        unfilteredChangesets,
+        workspacesByName
+      );
+
+      releases.forEach((value, key) => {
+        let releaseFromUnfilteredChangesets = releasesFromUnfilteredChangesets.get(
+          key
+        );
+        if (releaseFromUnfilteredChangesets === undefined) {
+          throw new InternalError(
+            "releaseFromUnfilteredChangesets is undefined"
+          );
+        }
+
+        releases.set(key, {
+          ...value,
+          // note that we're only setting the type, not the changesets which could be different(the name and oldVersion would be the same so they don't matter)
+          // because the changesets on a given release refer to why a given package is being released
+          // NOT why it's being released with a given bump type
+          // (the bump type could change because of this, linked or peer dependencies)
+          type: releaseFromUnfilteredChangesets.type
+        });
+      });
     }
   }
 
@@ -120,13 +154,13 @@ function assembleReleasePlan(
     // The map passed in to determineDependents will be mutated
     let dependentAdded = determineDependents(
       releases,
-      workspaces,
+      workspacesByName,
       dependentsGraph,
       preInfo
     );
 
     // The map passed in to determineDependents will be mutated
-    let linksUpdated = applyLinks(releases, workspaces, config.linked);
+    let linksUpdated = applyLinks(releases, workspacesByName, config.linked);
 
     releaseObjectValidated = !linksUpdated && !dependentAdded;
   }

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

@@ -586,4 +586,52 @@ describe("pre", () => {
       }
     ]);
   });
+  it.only("should use the highest bump type for between all prereleases for every prerelease", async () => {
+    let cwd = f.copy("simple-project");
+    await writeChangeset(
+      {
+        releases: [{ name: "pkg-a", type: "major" }],
+        summary: "a very useful summary for the first change"
+      },
+      cwd
+    );
+
+    await pre(cwd, { command: "enter", tag: "next" });
+    await version(cwd, modifiedDefaultConfig);
+    let workspaces = (await getWorkspaces({ cwd }))!;
+
+    expect(workspaces.map(x => x.config)).toEqual([
+      {
+        dependencies: { "pkg-b": "1.0.0" },
+        name: "pkg-a",
+        version: "2.0.0-next.0"
+      },
+      {
+        name: "pkg-b",
+        version: "1.0.0"
+      }
+    ]);
+
+    await writeChangeset(
+      {
+        releases: [{ name: "pkg-a", type: "minor" }],
+        summary: "a very useful summary for the second change"
+      },
+      cwd
+    );
+    await version(cwd, modifiedDefaultConfig);
+    workspaces = (await getWorkspaces({ cwd }))!;
+
+    expect(workspaces.map(x => x.config)).toEqual([
+      {
+        dependencies: { "pkg-b": "1.0.0" },
+        name: "pkg-a",
+        version: "2.0.0-next.1"
+      },
+      {
+        name: "pkg-b",
+        version: "1.0.0"
+      }
+    ]);
+  });
 });

yarn.lock

@@ -2834,6 +2834,15 @@ get-value@^2.0.3, get-value@^2.0.6:
   resolved "https://registry.yarnpkg.com/get-value/-/get-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28"
   integrity sha1-3BXKHGcjh8p2vTesCjlbogQqLCg=
 
+get-workspaces@^0.5.0:
+  version "0.5.2"
+  resolved "https://registry.yarnpkg.com/get-workspaces/-/get-workspaces-0.5.2.tgz#4fb2031ec1bdd32d3c1f3de2589ce8af5c2e24ee"
+  integrity sha512-99x72taQ9OUHhCmBS0B2WI/zwOtBOBPoyVNGs9+B0ag2GGhCjl/EaU9VQ8Zorx64TyVj1Am7bO+0J1KwDqo7OA==
+  dependencies:
+    "@changesets/types" "^0.4.0"
+    fs-extra "^7.0.1"
+    globby "^9.2.0"
+
 getpass@^0.1.1:
   version "0.1.7"
   resolved "https://registry.yarnpkg.com/getpass/-/getpass-0.1.7.tgz#5eff8e3e684d569ae4cb2b1282604e8ba62149fa"