fix(devkit): incorrect conditional to decide if package should be add…

…ed (#12749)
nrwl · Oct 21, 2022 · 657cdba · 657cdba · vercel · Oct 21, 2022
1 parent 3263599
commit 657cdba
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 29 deletions.
diff --git a/packages/devkit/src/utils/package-json.spec.ts b/packages/devkit/src/utils/package-json.spec.ts
@@ -275,4 +275,37 @@ describe('addDependenciesToPackageJson', () => {
     });
     expect(installTask).toBeDefined();
   });
+
+  it('should not overwrite dependencies when they exist in devDependencies or vice versa and the new version is tilde', () => {
+    // ARRANGE
+    writeJson(tree, 'package.json', {
+      dependencies: {
+        tslib: '2.4.0',
+      },
+      devDependencies: {
+        nx: '15.0.0',
+      },
+    });
+
+    // ACT
+    const installTask = addDependenciesToPackageJson(
+      tree,
+      {
+        tslib: '~2.3.0',
+      },
+      {
+        nx: '15.0.0',
+      }
+    );
+
+    // ASSERT
+    const { dependencies, devDependencies } = readJson(tree, 'package.json');
+    expect(dependencies).toEqual({
+      tslib: '2.4.0',
+    });
+    expect(devDependencies).toEqual({
+      nx: '15.0.0',
+    });
+    expect(installTask).toBeDefined();
+  });
 });
diff --git a/packages/devkit/src/utils/package-json.ts b/packages/devkit/src/utils/package-json.ts
@@ -5,6 +5,7 @@ import { GeneratorCallback } from 'nx/src/config/misc-interfaces';
 import { coerce, gt } from 'semver';
 
 const NON_SEMVER_TAGS = {
+  '*': 2,
   next: 1,
   latest: 0,
   previous: -1,
@@ -24,6 +25,27 @@ function filterExistingDependencies(
     .reduce((acc, d) => ({ ...acc, [d]: dependencies[d] }), {});
 }
 
+function isIncomingVersionGreater(
+  incomingVersion: string,
+  existingVersion: string
+) {
+  if (
+    incomingVersion in NON_SEMVER_TAGS &&
+    existingVersion in NON_SEMVER_TAGS
+  ) {
+    return NON_SEMVER_TAGS[incomingVersion] > NON_SEMVER_TAGS[existingVersion];
+  }
+
+  if (
+    incomingVersion in NON_SEMVER_TAGS ||
+    existingVersion in NON_SEMVER_TAGS
+  ) {
+    return true;
+  }
+
+  return gt(coerce(incomingVersion), coerce(existingVersion));
+}
+
 function updateExistingDependenciesVersion(
   dependencies: Record<string, string>,
   existingAltDependencies: Record<string, string>
@@ -37,23 +59,7 @@ function updateExistingDependenciesVersion(
       const incomingVersion = dependencies[d];
       const existingVersion = existingAltDependencies[d];
 
-      if (
-        incomingVersion in NON_SEMVER_TAGS &&
-        existingVersion in NON_SEMVER_TAGS
-      ) {
-        return (
-          NON_SEMVER_TAGS[incomingVersion] > NON_SEMVER_TAGS[existingVersion]
-        );
-      }
-
-      if (
-        incomingVersion in NON_SEMVER_TAGS ||
-        existingVersion in NON_SEMVER_TAGS
-      ) {
-        return true;
-      }
-
-      return gt(coerce(incomingVersion), coerce(existingVersion));
+      return isIncomingVersionGreater(incomingVersion, existingVersion);
     })
     .reduce((acc, d) => ({ ...acc, [d]: dependencies[d] }), {});
 }
@@ -205,21 +211,37 @@ function requiresAddingOfPackages(packageJsonFile, deps, devDeps): boolean {
   packageJsonFile.devDependencies = packageJsonFile.devDependencies || {};
 
   if (Object.keys(deps).length > 0) {
-    needsDepsUpdate =
-      Object.keys(deps).some((entry) => !packageJsonFile.dependencies[entry]) ||
-      Object.keys(deps).some(
-        (entry) => !packageJsonFile.devDependencies[entry]
-      );
+    needsDepsUpdate = Object.keys(deps).some((entry) => {
+      const incomingVersion = deps[entry];
+      if (packageJsonFile.dependencies[entry]) {
+        const existingVersion = packageJsonFile.dependencies[entry];
+        return isIncomingVersionGreater(incomingVersion, existingVersion);
+      }
+
+      if (packageJsonFile.devDependencies[entry]) {
+        const existingVersion = packageJsonFile.devDependencies[entry];
+        return isIncomingVersionGreater(incomingVersion, existingVersion);
+      }
+
+      return true;
+    });
   }
 
   if (Object.keys(devDeps).length > 0) {
-    needsDevDepsUpdate =
-      Object.keys(devDeps).some(
-        (entry) => !packageJsonFile.devDependencies[entry]
-      ) ||
-      Object.keys(devDeps).some(
-        (entry) => !packageJsonFile.dependencies[entry]
-      );
+    needsDevDepsUpdate = Object.keys(devDeps).some((entry) => {
+      const incomingVersion = devDeps[entry];
+      if (packageJsonFile.devDependencies[entry]) {
+        const existingVersion = packageJsonFile.devDependencies[entry];
+        return isIncomingVersionGreater(incomingVersion, existingVersion);
+      }
+      if (packageJsonFile.dependencies[entry]) {
+        const existingVersion = packageJsonFile.dependencies[entry];
+
+        return isIncomingVersionGreater(incomingVersion, existingVersion);
+      }
+
+      return true;
+    });
   }
 
   return needsDepsUpdate || needsDevDepsUpdate;