From d1dbc1c1c03bcc916914ff18a5de89f594a448a6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sat, 9 Mar 2019 17:07:22 +0100 Subject: [PATCH] fix(schematics): do not run migrations multiple times Currently when someone runs the update schematic within a CLI project, the migration could be executed multiple times for the same `tsconfig` path This can happen because the `getProjectTsConfigPaths` function _can_ incorrectly return the same tsconfig multiple times. The paths are not properly deduped as we don't normalize the determined project tsconfig paths. e.g. `/tsconfig.json` exists and the common tsconfig path is detected. At the same time the workspace configuration specifies `tsconfig.json` in the project options. Both paths refer to the same tsconfig project, but are different strings and therefore aren't filtered out by the `Set` which contains all possible tsconfig locations --> migrations are executed for both paths. --- .../project-tsconfig-paths.spec.ts | 43 +++++++------------ .../upgrade-rules/project-tsconfig-paths.ts | 28 ++++++------ 2 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.spec.ts b/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.spec.ts index 633b2f4526fd..8008dc9c64ca 100644 --- a/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.spec.ts +++ b/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.spec.ts @@ -3,7 +3,6 @@ import {UnitTestTree} from '@angular-devkit/schematics/testing'; import {getProjectTsConfigPaths} from './project-tsconfig-paths'; describe('ng-update project-tsconfig-paths', () => { - let testTree: UnitTestTree; beforeEach(() => { @@ -13,39 +12,19 @@ describe('ng-update project-tsconfig-paths', () => { it('should detect build tsconfig path inside of angular.json file', () => { testTree.create('/my-custom-config.json', ''); testTree.create('/angular.json', JSON.stringify({ - projects: { - my_name: { - architect: { - build: { - options: { - tsConfig: './my-custom-config.json' - } - } - } - } - } + projects: {my_name: {architect: {build: {options: {tsConfig: './my-custom-config.json'}}}}} })); - expect(getProjectTsConfigPaths(testTree)).toEqual(['./my-custom-config.json']); + expect(getProjectTsConfigPaths(testTree)).toEqual(['my-custom-config.json']); }); it('should detect test tsconfig path inside of .angular.json file', () => { testTree.create('/my-test-config.json', ''); testTree.create('/.angular.json', JSON.stringify({ - projects: { - with_tests: { - architect: { - test: { - options: { - tsConfig: './my-test-config.json' - } - } - } - } - } + projects: {with_tests: {architect: {test: {options: {tsConfig: './my-test-config.json'}}}}} })); - expect(getProjectTsConfigPaths(testTree)).toEqual(['./my-test-config.json']); + expect(getProjectTsConfigPaths(testTree)).toEqual(['my-test-config.json']); }); it('should detect common tsconfigs if no workspace config could be found', () => { @@ -53,7 +32,17 @@ describe('ng-update project-tsconfig-paths', () => { testTree.create('/src/tsconfig.json', ''); testTree.create('/src/tsconfig.app.json', ''); - expect(getProjectTsConfigPaths(testTree)) - .toEqual(['./tsconfig.json', './src/tsconfig.json', './src/tsconfig.app.json']); + expect(getProjectTsConfigPaths(testTree)).toEqual([ + 'tsconfig.json', 'src/tsconfig.json', 'src/tsconfig.app.json' + ]); + }); + + it('should not return duplicate tsconfig files', () => { + testTree.create('/tsconfig.json', ''); + testTree.create('/.angular.json', JSON.stringify({ + projects: {app: {architect: {test: {options: {tsConfig: 'tsconfig.json'}}}}} + })); + + expect(getProjectTsConfigPaths(testTree)).toEqual(['tsconfig.json']); }); }); diff --git a/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.ts b/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.ts index f65897025c04..0aa9164f6eda 100644 --- a/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.ts +++ b/src/cdk/schematics/ng-update/upgrade-rules/project-tsconfig-paths.ts @@ -6,8 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ +import {normalize} from '@angular-devkit/core'; import {Tree} from '@angular-devkit/schematics'; +/** Name of the default Angular CLI workspace configuration files. */ +const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json']; + /** * Gets all tsconfig paths from a CLI project by reading the workspace configuration * and looking for common tsconfig locations. @@ -15,9 +19,9 @@ import {Tree} from '@angular-devkit/schematics'; export function getProjectTsConfigPaths(tree: Tree): string[] { // Start with some tsconfig paths that are generally used within CLI projects. const tsconfigPaths = new Set([ - './tsconfig.json', - './src/tsconfig.json', - './src/tsconfig.app.json', + 'tsconfig.json', + 'src/tsconfig.json', + 'src/tsconfig.app.json', ]); // Add any tsconfig directly referenced in a build or test task of the angular.json workspace. @@ -26,18 +30,15 @@ export function getProjectTsConfigPaths(tree: Tree): string[] { if (workspace) { for (const project of Object.values(workspace.projects)) { ['build', 'test'].forEach(targetName => { - if (project.targets && - project.targets[targetName] && - project.targets[targetName].options && + if (project.targets && project.targets[targetName] && project.targets[targetName].options && project.targets[targetName].options.tsConfig) { - tsconfigPaths.add(project.targets[targetName].options.tsConfig); + tsconfigPaths.add(normalize(project.targets[targetName].options.tsConfig)); } - if (project.architect && - project.architect[targetName] && + if (project.architect && project.architect[targetName] && project.architect[targetName].options && project.architect[targetName].options.tsConfig) { - tsconfigPaths.add(project.architect[targetName].options.tsConfig); + tsconfigPaths.add(normalize(project.architect[targetName].options.tsConfig)); } }); } @@ -47,9 +48,6 @@ export function getProjectTsConfigPaths(tree: Tree): string[] { return Array.from(tsconfigPaths).filter(p => tree.exists(p)); } -/** Name of the default Angular CLI workspace configuration files. */ -const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json']; - /** * Resolve the workspace configuration of the specified tree gracefully. We cannot use the utility * functions from the default Angular schematics because those might not be present in older @@ -57,8 +55,8 @@ const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json']; * the CLI project could be still using `.angular-cli.json` instead of thew new config. */ function getWorkspaceConfigGracefully(tree: Tree): any { - const path = defaultWorkspaceConfigPaths.filter(filePath => tree.exists(filePath))[0]; - const configBuffer = tree.read(path); + const path = defaultWorkspaceConfigPaths.find(filePath => tree.exists(filePath)); + const configBuffer = tree.read(path!); if (!path || !configBuffer) { return null;