From 657cdbab603ee1df9aafdd194d4885b1bd473105 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Fri, 21 Oct 2022 11:15:03 -0700 Subject: [PATCH] fix(devkit): incorrect conditional to decide if package should be added (#12749) --- .../devkit/src/utils/package-json.spec.ts | 33 ++++++++ packages/devkit/src/utils/package-json.ts | 80 ++++++++++++------- 2 files changed, 84 insertions(+), 29 deletions(-) diff --git a/packages/devkit/src/utils/package-json.spec.ts b/packages/devkit/src/utils/package-json.spec.ts index 4dc0f52828a12..c87026733c302 100644 --- 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 index 57686280c8c4a..8430cd76fe968 100644 --- 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, existingAltDependencies: Record @@ -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;