From 7c672210406b8ebd26c883d3eec387497d29c7a8 Mon Sep 17 00:00:00 2001 From: Juri Date: Fri, 17 Jan 2020 12:03:47 +0100 Subject: [PATCH] fix(misc): update package.json only if needed Adjusts the schematic to only touch the package.json if really required ISSUES CLOSED: #2317 --- packages/jest/src/schematics/init/init.ts | 77 ++++++++--- .../workspace/src/utils/ast-utils.spec.ts | 43 ++++++- packages/workspace/src/utils/ast-utils.ts | 82 +++++++++--- packages/workspace/src/utils/lint.ts | 120 ++++++++++-------- 4 files changed, 232 insertions(+), 90 deletions(-) diff --git a/packages/jest/src/schematics/init/init.ts b/packages/jest/src/schematics/init/init.ts index 8fb9cdfb5fa89..3296e6087f528 100644 --- 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 index ffd5be5430d61..a479adc885617 100644 --- 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 index 4bc5664a33e8a..7c8e171b0267b 100644 --- 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 index cb376515abe45..d214d85c5e547 100644 --- 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); } }; }