Change external mismatch check & fix to most common

.changeset/rare-steaks-cheer.md

@@ -0,0 +1,5 @@
+---
+"@manypkg/cli": minor
+---
+
+Change external mismatch behaviour to suggest and fix to the most commonly used dependency range in the workspace. If all ranges are only used once it will pick the highest.

README.md

@@ -78,7 +78,8 @@ So that only a single version of an external package will be installed because h
 
 ### How it's fixed
 
-The highest range of the dependency is set as the range at every non-peer dependency place it is depended on.
+The most commonly used range of the dependency is set as the range at every non-peer dependency place it is depended on.
+If for some reason, every range is used the same amount of times, they'll all be fixed to the highest version.
 
 ### Examples
 
@@ -120,7 +121,19 @@ The highest range of the dependency is set as the range at every non-peer depend
 }
 ```
 
-This example will cause an error because the range `1.0.0` for `some-external-package` specified in `@manypkg-example/pkg-a` is not equal(`===`) to the range `2.0.0` specified in `@manypkg-example/pkg-b`.
+`packages/pkg-c/package.json`
+
+```json
+{
+  "name": "@manypkg-example/pkg-c",
+  "version": "1.0.0",
+  "dependencies": {
+    "some-external-package": "1.0.0"
+  }
+}
+```
+
+This example will cause an error because the range `2.0.0` for `some-external-package` specified in `@manypkg-example/pkg-b` is not equal(`===`) to the range `1.0.0` specified in `@manypkg-example/pkg-a` and `@manypkg-example/pkg-c`.
 
 </details>
 

packages/cli/src/checks/EXTERNAL_MISMATCH.ts

@@ -1,6 +1,6 @@
 import {
   makeCheck,
-  getHighestExternalRanges,
+  getMostCommonRangeMap,
   NORMAL_DEPENDENCY_TYPES
 } from "./utils";
 import { Package } from "@manypkg/get-packages";
@@ -11,31 +11,31 @@ type ErrorType = {
   workspace: Package;
   dependencyName: string;
   dependencyRange: string;
-  highestDependencyRange: string;
+  mostCommonDependencyRange: string;
 };
 
 export default makeCheck<ErrorType>({
   validate: (workspace, allWorkspace) => {
     let errors: ErrorType[] = [];
-    let highestExternalRanges = getHighestExternalRanges(allWorkspace);
+    let mostCommonRangeMap = getMostCommonRangeMap(allWorkspace);
     for (let depType of NORMAL_DEPENDENCY_TYPES) {
       let deps = workspace.packageJson[depType];
 
       if (deps) {
         for (let depName in deps) {
           let range = deps[depName];
-          let highestRange = highestExternalRanges.get(depName);
+          let mostCommonRange = mostCommonRangeMap.get(depName);
           if (
-            highestRange !== undefined &&
-            highestRange !== range &&
+            mostCommonRange !== undefined &&
+            mostCommonRange !== range &&
             validRange(range)
           ) {
             errors.push({
               type: "EXTERNAL_MISMATCH",
               workspace,
               dependencyName: depName,
               dependencyRange: range,
-              highestDependencyRange: highestRange
+              mostCommonDependencyRange: mostCommonRange
             });
           }
         }
@@ -47,12 +47,12 @@ export default makeCheck<ErrorType>({
     for (let depType of NORMAL_DEPENDENCY_TYPES) {
       let deps = error.workspace.packageJson[depType];
       if (deps && deps[error.dependencyName]) {
-        deps[error.dependencyName] = error.highestDependencyRange;
+        deps[error.dependencyName] = error.mostCommonDependencyRange;
       }
     }
     return { requiresInstall: true };
   },
   print: error =>
-    `${error.workspace.packageJson.name} has a dependency on ${error.dependencyName}@${error.dependencyRange} but the highest range in the repo is ${error.highestDependencyRange}, the range should be set to ${error.highestDependencyRange}`,
+    `${error.workspace.packageJson.name} has a dependency on ${error.dependencyName}@${error.dependencyRange} but the most common range in the repo is ${error.mostCommonDependencyRange}, the range should be set to ${error.mostCommonDependencyRange}`,
   type: "all"
 });

packages/cli/src/checks/INVALID_DEV_AND_PEER_DEPENDENCY_RELATIONSHIP.ts

@@ -1,4 +1,4 @@
-import { makeCheck, getHighestExternalRanges } from "./utils";
+import { makeCheck, getMostCommonRangeMap } from "./utils";
 import { Package } from "@manypkg/get-packages";
 import { upperBoundOfRangeAWithinBoundsOfB } from "sembear";
 import semver from "semver";
@@ -21,7 +21,7 @@ export default makeCheck<ErrorType>({
     if (peerDeps) {
       for (let depName in peerDeps) {
         if (!devDeps[depName]) {
-          let highestRanges = getHighestExternalRanges(allWorkspaces);
+          let highestRanges = getMostCommonRangeMap(allWorkspaces);
           let idealDevVersion = highestRanges.get(depName);
           let isInternalDependency = allWorkspaces.has(depName);
           if (isInternalDependency) {
@@ -48,7 +48,7 @@ export default makeCheck<ErrorType>({
             peerDeps[depName]
           )
         ) {
-          let highestRanges = getHighestExternalRanges(allWorkspaces);
+          let highestRanges = getMostCommonRangeMap(allWorkspaces);
           let idealDevVersion = highestRanges.get(depName);
           if (idealDevVersion === undefined) {
             idealDevVersion = peerDeps[depName];

packages/cli/src/checks/__tests__/EXTERNAL_MISMATCH.ts

@@ -24,7 +24,7 @@ it("should error if the ranges are valid and they are not equal", () => {
       Object {
         "dependencyName": "something",
         "dependencyRange": "1.0.0",
-        "highestDependencyRange": "2.0.0",
+        "mostCommonDependencyRange": "2.0.0",
         "type": "EXTERNAL_MISMATCH",
         "workspace": Object {
           "dir": "some/fake/dir/pkg-1",
@@ -41,6 +41,178 @@ it("should error if the ranges are valid and they are not equal", () => {
   `);
 });
 
+it("should error and return the correct mostCommonDependencyRange when the ranges are valid, they are not equal and there are more than 2", () => {
+  let ws = getWS();
+
+  ws.get("pkg-1")!.packageJson.dependencies = { something: "1.0.0" };
+
+  let pkg2 = getFakeWS("pkg-2");
+  pkg2.packageJson.dependencies = {
+    something: "2.0.0"
+  };
+  ws.set("pkg-2", pkg2);
+
+  let pkg3 = getFakeWS("pkg-3");
+  pkg3.packageJson.dependencies = {
+    something: "1.0.0"
+  };
+
+  ws.set("pkg-3", pkg3);
+  let errors = internalMismatch.validate(
+    ws.get("pkg-1")!,
+    ws,
+    rootWorkspace,
+    {}
+  );
+  expect(errors.length).toEqual(0);
+
+  errors = internalMismatch.validate(pkg2, ws, rootWorkspace, {});
+  expect(errors.length).toEqual(1);
+  expect(errors).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "dependencyName": "something",
+        "dependencyRange": "2.0.0",
+        "mostCommonDependencyRange": "1.0.0",
+        "type": "EXTERNAL_MISMATCH",
+        "workspace": Object {
+          "dir": "some/fake/dir/pkg-2",
+          "packageJson": Object {
+            "dependencies": Object {
+              "something": "2.0.0",
+            },
+            "name": "pkg-2",
+            "version": "1.0.0",
+          },
+        },
+      },
+    ]
+  `);
+});
+
+it("should error and return the correct mostCommonDependencyRange when the ranges are valid, but the 2nd dependnecy is most common", () => {
+  let ws = getWS();
+
+  ws.get("pkg-1")!.packageJson.dependencies = { something: "2.0.0" };
+
+  let pkg2 = getFakeWS("pkg-2");
+  pkg2.packageJson.dependencies = {
+    something: "1.0.0"
+  };
+  ws.set("pkg-2", pkg2);
+
+  let pkg3 = getFakeWS("pkg-3");
+  pkg3.packageJson.dependencies = {
+    something: "1.0.0"
+  };
+
+  ws.set("pkg-3", pkg3);
+  let errors = internalMismatch.validate(
+    ws.get("pkg-1")!,
+    ws,
+    rootWorkspace,
+    {}
+  );
+  console.log(errors);
+
+  expect(errors.length).toEqual(1);
+  expect(errors).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "dependencyName": "something",
+        "dependencyRange": "2.0.0",
+        "mostCommonDependencyRange": "1.0.0",
+        "type": "EXTERNAL_MISMATCH",
+        "workspace": Object {
+          "dir": "some/fake/dir/pkg-1",
+          "packageJson": Object {
+            "dependencies": Object {
+              "something": "2.0.0",
+            },
+            "name": "pkg-1",
+            "version": "1.0.0",
+          },
+        },
+      },
+    ]
+  `);
+
+  errors = internalMismatch.validate(pkg2, ws, rootWorkspace, {});
+  expect(errors.length).toEqual(0);
+});
+
+it("should error and return the correct mostCommonDependencyRange when the ranges are valid, but everything wants a different version", () => {
+  let ws = getWS();
+
+  ws.get("pkg-1")!.packageJson.dependencies = { something: "1.0.0" };
+
+  let pkg2 = getFakeWS("pkg-2");
+  pkg2.packageJson.dependencies = {
+    something: "2.0.0"
+  };
+  ws.set("pkg-2", pkg2);
+
+  let pkg3 = getFakeWS("pkg-3");
+  pkg3.packageJson.dependencies = {
+    something: "3.0.0"
+  };
+
+  ws.set("pkg-3", pkg3);
+  let errors = internalMismatch.validate(
+    ws.get("pkg-1")!,
+    ws,
+    rootWorkspace,
+    {}
+  );
+  expect(errors.length).toEqual(1);
+  expect(errors).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "dependencyName": "something",
+        "dependencyRange": "1.0.0",
+        "mostCommonDependencyRange": "3.0.0",
+        "type": "EXTERNAL_MISMATCH",
+        "workspace": Object {
+          "dir": "some/fake/dir/pkg-1",
+          "packageJson": Object {
+            "dependencies": Object {
+              "something": "1.0.0",
+            },
+            "name": "pkg-1",
+            "version": "1.0.0",
+          },
+        },
+      },
+    ]
+  `);
+
+  errors = internalMismatch.validate(pkg2, ws, rootWorkspace, {});
+  expect(errors.length).toEqual(1);
+  expect(errors).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "dependencyName": "something",
+        "dependencyRange": "2.0.0",
+        "mostCommonDependencyRange": "3.0.0",
+        "type": "EXTERNAL_MISMATCH",
+        "workspace": Object {
+          "dir": "some/fake/dir/pkg-2",
+          "packageJson": Object {
+            "dependencies": Object {
+              "something": "2.0.0",
+            },
+            "name": "pkg-2",
+            "version": "1.0.0",
+          },
+        },
+      },
+    ]
+  `);
+
+  errors = internalMismatch.validate(pkg3, ws, rootWorkspace, {});
+  expect(errors.length).toEqual(0);
+});
+
 it("should not error if the value is not a valid semver range", () => {
   let ws = getWS();
 

packages/cli/src/checks/utils.ts

@@ -117,32 +117,55 @@ function weakMemoize<Arg, Ret>(func: (arg: Arg) => Ret): (arg: Arg) => Ret {
   };
 }
 
-export let getHighestExternalRanges = weakMemoize(function getHighestVersions(
+export let getMostCommonRangeMap = weakMemoize(function getMostCommonRanges(
   allPackages: Map<string, Package>
 ) {
-  let highestExternalRanges = new Map<string, string>();
+  let dependencyRangesMapping = new Map<string, {[key: string]: number}>();
+
   for (let [pkgName, pkg] of allPackages) {
     for (let depType of NORMAL_DEPENDENCY_TYPES) {
       let deps = pkg.packageJson[depType];
       if (deps) {
         for (let depName in deps) {
+          const depSpecifier = deps[depName];
           if (!allPackages.has(depName)) {
             if (!semver.validRange(deps[depName])) {
               continue;
             }
-            let highestExternalRange = highestExternalRanges.get(depName);
-            if (
-              !highestExternalRange ||
-              highest([highestExternalRange, deps[depName]]) === deps[depName]
-            ) {
-              highestExternalRanges.set(depName, deps[depName]);
-            }
+            let dependencyRanges = dependencyRangesMapping.get(depName) || {};
+            const specifierCount = dependencyRanges[depSpecifier] || 0;
+            dependencyRanges[depSpecifier] = specifierCount + 1;
+            dependencyRangesMapping.set(depName, dependencyRanges);
           }
         }
       }
     }
   }
-  return highestExternalRanges;
+
+  let mostCommonRangeMap = new Map<string, string>();
+
+  for (let [depName, specifierMap] of dependencyRangesMapping) {
+    const specifierMapEntryArray = Object.entries(specifierMap);
+
+    const [first] = specifierMapEntryArray;
+    const maxValue = specifierMapEntryArray.reduce((acc, value) => {
+      if(acc[1] === value[1]) {
+        // If all dependency ranges occurances are equal, pick the highest.
+        // It's impossible to infer intention of the developer 
+        // when all ranges occur an equal amount of times
+        const highestRange = highest([acc[0], value[0]]);
+        return [ highestRange, acc[1] ];
+      }
+
+      if(acc[1] > value[1]) {
+        return acc;
+      }
+      return value;
+    }, first);
+
+    mostCommonRangeMap.set(depName, maxValue[0])
+  }
+  return mostCommonRangeMap;
 });
 
 export function versionRangeToRangeType(versionRange: string) {