fix(misc): update package.json only if needed

Adjusts the schematic to only touch the package.json if really required ISSUES CLOSED: nrwl#2317
juristr · Jan 27, 2020 · 7c67221 · 7c67221
1 parent ea85a11
commit 7c67221
Show file tree

Hide file tree

Showing 4 changed files with 232 additions and 90 deletions.
diff --git a/packages/jest/src/schematics/init/init.ts b/packages/jest/src/schematics/init/init.ts
@@ -1,5 +1,9 @@
 import { mergeWith, chain, url, Tree } from '@angular-devkit/schematics';
-import { addDepsToPackageJson, updateJsonInTree } from '@nrwl/workspace';
+import {
+  addDepsToPackageJson,
+  updateJsonInTree,
+  readJsonInTree
+} from '@nrwl/workspace';
 import {
   jestVersion,
   jestTypesVersion,
@@ -8,23 +12,48 @@ import {
 } from '../../utils/versions';
 import { Rule } from '@angular-devkit/schematics';
 import { stripIndents } from '@angular-devkit/core/src/utils/literals';
+import { noop } from 'rxjs';
+
+/**
+ * verifies whether the given packageJson dependencies require an update
+ * given the deps & devDeps passed in
+ */
+const requiresAddingOfPackages = (packageJsonFile, deps, devDeps): boolean => {
+  let needsDepsUpdate = false;
+  let needsDevDepsUpdate = false;
 
-const updatePackageJson = chain([
-  addDepsToPackageJson(
-    {},
-    {
-      '@nrwl/jest': nxVersion,
-      jest: jestVersion,
-      '@types/jest': jestTypesVersion,
-      'ts-jest': tsJestVersion
-    }
-  ),
-  updateJsonInTree('package.json', json => {
-    json.dependencies = json.dependencies || {};
-    delete json.dependencies['@nrwl/jest'];
-    return json;
-  })
-]);
+  if (Object.keys(deps).length > 0 && packageJsonFile.dependencies) {
+    needsDepsUpdate = !Object.keys(deps).find(
+      entry => packageJsonFile.dependencies[entry]
+    );
+  }
+
+  if (Object.keys(devDeps).length > 0 && packageJsonFile.dependencies) {
+    needsDevDepsUpdate = !Object.keys(devDeps).find(
+      entry => packageJsonFile.devDependencies[entry]
+    );
+  }
+
+  return needsDepsUpdate || needsDevDepsUpdate;
+};
+
+const removeNrwlJestFromDeps = (host: Tree) => {
+  // check whether to update the packge.json is necessary
+  const currentPackageJson = readJsonInTree(host, 'package.json');
+
+  if (
+    currentPackageJson.dependencies &&
+    currentPackageJson.dependencies['@nrwl/jest']
+  ) {
+    return updateJsonInTree('package.json', json => {
+      json.dependencies = json.dependencies || {};
+      delete json.dependencies['@nrwl/jest'];
+      return json;
+    });
+  } else {
+    return noop();
+  }
+};
 
 const createJestConfig = (host: Tree) => {
   if (!host.exists('jest.config.js')) {
@@ -46,5 +75,17 @@ const createJestConfig = (host: Tree) => {
 };
 
 export default function(): Rule {
-  return chain([createJestConfig, updatePackageJson]);
+  return chain([
+    createJestConfig,
+    addDepsToPackageJson(
+      {},
+      {
+        '@nrwl/jest': nxVersion,
+        jest: jestVersion,
+        '@types/jest': jestTypesVersion,
+        'ts-jest': tsJestVersion
+      }
+    ),
+    removeNrwlJestFromDeps
+  ]);
 }
diff --git a/packages/workspace/src/utils/ast-utils.spec.ts b/packages/workspace/src/utils/ast-utils.spec.ts
@@ -1,15 +1,17 @@
 import {
   readJsonInTree,
   renameSyncInTree,
-  renameDirSyncInTree
+  renameDirSyncInTree,
+  addDepsToPackageJson
 } from './ast-utils';
 import {
   SchematicTestRunner,
   UnitTestTree
 } from '@angular-devkit/schematics/testing';
 import { join } from 'path';
-import { Tree } from '@angular-devkit/schematics';
+import { Tree, SchematicContext, TaskId } from '@angular-devkit/schematics';
 import { serializeJson } from './fileutils';
+import { createEmptyWorkspace } from './testing-utils';
 
 describe('readJsonInTree', () => {
   let tree: Tree;
@@ -124,3 +126,40 @@ describe('renameDirSyncInTree', () => {
     });
   });
 });
+
+describe('addDepsToPackageJson', () => {
+  let appTree: Tree;
+
+  beforeEach(() => {
+    appTree = Tree.empty();
+    appTree = createEmptyWorkspace(appTree);
+  });
+
+  it('should not update the package.json if dependencies have already been added', async () => {
+    const devDeps = {
+      '@nrwl/jest': '1.2.3'
+    };
+
+    appTree.overwrite(
+      '/package.json',
+      JSON.stringify({
+        dependencies: {},
+        devDependencies: {
+          ...devDeps
+        }
+      })
+    );
+
+    const testRunner = new SchematicTestRunner('@nrwl/jest', null);
+
+    await testRunner
+      .callRule(() => {
+        return addDepsToPackageJson({}, devDeps);
+      }, appTree)
+      .toPromise();
+
+    expect(
+      testRunner.tasks.find(x => x.name === 'node-package')
+    ).not.toBeDefined();
+  });
+});
diff --git a/packages/workspace/src/utils/ast-utils.ts b/packages/workspace/src/utils/ast-utils.ts
@@ -9,7 +9,8 @@ import {
   Rule,
   Tree,
   SchematicContext,
-  DirEntry
+  DirEntry,
+  noop
 } from '@angular-devkit/schematics';
 import * as ts from 'typescript';
 import * as stripJsonComments from 'strip-json-comments';
@@ -534,30 +535,77 @@ export function readWorkspace(host: Tree): any {
   return readJsonInTree(host, path);
 }
 
+/**
+ * verifies whether the given packageJson dependencies require an update
+ * given the deps & devDeps passed in
+ */
+function requiresAddingOfPackages(packageJsonFile, deps, devDeps): boolean {
+  let needsDepsUpdate = false;
+  let needsDevDepsUpdate = false;
+
+  packageJsonFile.dependencies = packageJsonFile.dependencies || {};
+  packageJsonFile.devDependencies = packageJsonFile.devDependencies || {};
+
+  if (Object.keys(deps).length > 0) {
+    needsDepsUpdate = !Object.keys(deps).find(
+      entry => packageJsonFile.dependencies[entry]
+    );
+  }
+
+  if (Object.keys(devDeps).length > 0) {
+    needsDevDepsUpdate = !Object.keys(devDeps).find(
+      entry => packageJsonFile.devDependencies[entry]
+    );
+  }
+
+  return needsDepsUpdate || needsDevDepsUpdate;
+}
+
 let installAdded = false;
 
+/**
+ * Updates the package.json given the passed deps and/or devDeps. Only updates
+ * if the packages are not yet present
+ *
+ * @param host the schematic tree
+ * @param deps the package.json dependencies to add
+ * @param devDeps the package.json devDependencies to add
+ * @param addInstall default `true`; set to false to avoid installs
+ */
 export function addDepsToPackageJson(
   deps: any,
   devDeps: any,
   addInstall = true
 ): Rule {
-  return updateJsonInTree('package.json', (json, context: SchematicContext) => {
-    json.dependencies = {
-      ...(json.dependencies || {}),
-      ...deps,
-      ...(json.dependencies || {})
-    };
-    json.devDependencies = {
-      ...(json.devDependencies || {}),
-      ...devDeps,
-      ...(json.devDependencies || {})
-    };
-    if (addInstall && !installAdded) {
-      context.addTask(new NodePackageInstallTask());
-      installAdded = true;
+  return (host: Tree, context: SchematicContext) => {
+    const currentPackageJson = readJsonInTree(host, 'package.json');
+
+    if (requiresAddingOfPackages(currentPackageJson, deps, devDeps)) {
+      return updateJsonInTree(
+        'package.json',
+        (json, context: SchematicContext) => {
+          json.dependencies = {
+            ...(json.dependencies || {}),
+            ...deps,
+            ...(json.dependencies || {})
+          };
+          json.devDependencies = {
+            ...(json.devDependencies || {}),
+            ...devDeps,
+            ...(json.devDependencies || {})
+          };
+
+          if (addInstall && !installAdded) {
+            context.addTask(new NodePackageInstallTask());
+            installAdded = true;
+          }
+          return json;
+        }
+      );
+    } else {
+      return noop();
     }
-    return json;
-  });
+  };
 }
 
 export function updatePackageJsonDependencies(

diff --git a/packages/workspace/src/utils/lint.ts b/packages/workspace/src/utils/lint.ts
@@ -68,67 +68,81 @@ export function addLintFiles(
       );
     }
 
+    const chainedCommands = [];
+
     if (linter === 'tslint') {
-      if (!host.exists('/tslint.json')) {
-        host.create('/tslint.json', globalTsLint);
-      }
-      if (!options.onlyGlobal) {
-        host.create(
-          join(projectRoot as any, `tslint.json`),
-          JSON.stringify({
-            extends: `${offsetFromRoot(projectRoot)}tslint.json`,
-            rules: {}
-          })
-        );
-      }
-      return;
+      chainedCommands.push((host: Tree) => {
+        if (!host.exists('/tslint.json')) {
+          host.create('/tslint.json', globalTsLint);
+        }
+        if (!options.onlyGlobal) {
+          host.create(
+            join(projectRoot as any, `tslint.json`),
+            JSON.stringify({
+              extends: `${offsetFromRoot(projectRoot)}tslint.json`,
+              rules: {}
+            })
+          );
+        }
+      });
+
+      return chain(chainedCommands);
     }
 
+    debugger;
     if (linter === 'eslint') {
       if (!host.exists('/.eslintrc')) {
-        host.create('/.eslintrc', globalESLint);
-        addDepsToPackageJson(
-          {
-            ...(options.extraPackageDeps
-              ? options.extraPackageDeps.dependencies
-              : {})
-          },
-          {
-            '@nrwl/eslint-plugin-nx': nxVersion,
-            '@typescript-eslint/parser': typescriptESLintVersion,
-            '@typescript-eslint/eslint-plugin': typescriptESLintVersion,
-            eslint: eslintVersion,
-            'eslint-config-prettier': eslintConfigPrettierVersion,
-            ...(options.extraPackageDeps
-              ? options.extraPackageDeps.devDependencies
-              : {})
-          }
-        )(host, context);
+        chainedCommands.push((host: Tree) => {
+          host.create('/.eslintrc', globalESLint);
+
+          return addDepsToPackageJson(
+            {
+              ...(options.extraPackageDeps
+                ? options.extraPackageDeps.dependencies
+                : {})
+            },
+            {
+              '@nrwl/eslint-plugin-nx': nxVersion,
+              '@typescript-eslint/parser': typescriptESLintVersion,
+              '@typescript-eslint/eslint-plugin': typescriptESLintVersion,
+              eslint: eslintVersion,
+              'eslint-config-prettier': eslintConfigPrettierVersion,
+              ...(options.extraPackageDeps
+                ? options.extraPackageDeps.devDependencies
+                : {})
+            }
+          );
+        });
       }
+
       if (!options.onlyGlobal) {
-        let configJson;
-        const rootConfig = `${offsetFromRoot(projectRoot)}.eslintrc`;
-        if (options.localConfig) {
-          const extendsOption = options.localConfig.extends
-            ? Array.isArray(options.localConfig.extends)
-              ? options.localConfig.extends
-              : [options.localConfig.extends]
-            : [];
-          configJson = {
-            ...options.localConfig,
-            extends: [...extendsOption, rootConfig]
-          };
-        } else {
-          configJson = {
-            extends: rootConfig,
-            rules: {}
-          };
-        }
-        host.create(
-          join(projectRoot as any, `.eslintrc`),
-          JSON.stringify(configJson)
-        );
+        chainedCommands.push((host: Tree) => {
+          let configJson;
+          const rootConfig = `${offsetFromRoot(projectRoot)}.eslintrc`;
+          if (options.localConfig) {
+            const extendsOption = options.localConfig.extends
+              ? Array.isArray(options.localConfig.extends)
+                ? options.localConfig.extends
+                : [options.localConfig.extends]
+              : [];
+            configJson = {
+              ...options.localConfig,
+              extends: [...extendsOption, rootConfig]
+            };
+          } else {
+            configJson = {
+              extends: rootConfig,
+              rules: {}
+            };
+          }
+          host.create(
+            join(projectRoot as any, `.eslintrc`),
+            JSON.stringify(configJson)
+          );
+        });
       }
+
+      return chain(chainedCommands);
     }
   };
 }