From 1d3356e4b61f8940790b5d8788796480d4a5120a Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 30 Apr 2018 10:16:31 -0700 Subject: [PATCH 1/3] fix(@schematics/angular): if no property set on cli config dont migrate it --- packages/schematics/angular/migrations/update-6/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/schematics/angular/migrations/update-6/index.ts b/packages/schematics/angular/migrations/update-6/index.ts index 3ea4ec2b6b..a2c1c473c7 100644 --- a/packages/schematics/angular/migrations/update-6/index.ts +++ b/packages/schematics/angular/migrations/update-6/index.ts @@ -106,7 +106,7 @@ function extractCliConfig(config: CliConfig): JsonObject | null { newConfig['packageManager'] = config.packageManager; } - return newConfig; + return Object.getOwnPropertyNames(newConfig).length == 0 ? null : newConfig; } function extractSchematicsConfig(config: CliConfig): JsonObject | null { From 42b5d160fcfc6cb86777a1daf53789bbde12cf14 Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 30 Apr 2018 10:48:18 -0700 Subject: [PATCH 2/3] fix(@schematics/angular): update script now migrate warnings settings --- .../angular/migrations/update-6/index.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/schematics/angular/migrations/update-6/index.ts b/packages/schematics/angular/migrations/update-6/index.ts index a2c1c473c7..db4e9d767c 100644 --- a/packages/schematics/angular/migrations/update-6/index.ts +++ b/packages/schematics/angular/migrations/update-6/index.ts @@ -105,6 +105,20 @@ function extractCliConfig(config: CliConfig): JsonObject | null { if (config.packageManager && config.packageManager !== 'default') { newConfig['packageManager'] = config.packageManager; } + if (config.warnings) { + if (config.warnings.versionMismatch !== undefined) { + newConfig.warnings = { + ...((newConfig.warnings as JsonObject | null) || {}), + ...{ versionMismatch: config.warnings.versionMismatch }, + }; + } + if (config.warnings.typescriptMismatch !== undefined) { + newConfig.warnings = { + ...((newConfig.warnings as JsonObject | null) || {}), + ...{ typescriptMismatch: config.warnings.typescriptMismatch }, + }; + } + } return Object.getOwnPropertyNames(newConfig).length == 0 ? null : newConfig; } From 0ca869b59ac31d9958de5f8cdf6d80e1932d3a23 Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 30 Apr 2018 12:57:09 -0700 Subject: [PATCH 3/3] fix(@schematics/angular): use the JSON AST for changing JSON files in migration Fix angular/angular-cli#10396 --- .../angular/migrations/update-6/index.ts | 155 ++++++++++++++---- .../angular/migrations/update-6/index_spec.ts | 60 ++++++- .../angular/migrations/update-6/json-utils.ts | 70 ++++++++ 3 files changed, 248 insertions(+), 37 deletions(-) create mode 100644 packages/schematics/angular/migrations/update-6/json-utils.ts diff --git a/packages/schematics/angular/migrations/update-6/index.ts b/packages/schematics/angular/migrations/update-6/index.ts index db4e9d767c..09758d5a7e 100644 --- a/packages/schematics/angular/migrations/update-6/index.ts +++ b/packages/schematics/angular/migrations/update-6/index.ts @@ -5,7 +5,16 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import { JsonObject, Path, join, normalize, tags } from '@angular-devkit/core'; +import { + JsonObject, + JsonParseMode, + Path, + join, + normalize, + parseJson, + parseJsonAst, + tags, +} from '@angular-devkit/core'; import { Rule, SchematicContext, @@ -16,6 +25,11 @@ import { import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks'; import { AppConfig, CliConfig } from '../../utility/config'; import { latestVersions } from '../../utility/latest-versions'; +import { + appendPropertyInAstObject, + appendValueInAstArray, + findPropertyInAstObject, +} from './json-utils'; const defaults = { appRoot: 'src', @@ -492,8 +506,6 @@ function extractProjectsConfig(config: CliConfig, tree: Tree): JsonObject { root: project.root, sourceRoot: project.root, projectType: 'application', - cli: {}, - schematics: {}, }; const e2eArchitect: JsonObject = {}; @@ -542,51 +554,91 @@ function updateSpecTsConfig(config: CliConfig): Rule { return (host: Tree, context: SchematicContext) => { const apps = config.apps || []; apps.forEach((app: AppConfig, idx: number) => { - const tsSpecConfigPath = - join(app.root as Path, app.testTsconfig || defaults.testTsConfig); + const testTsConfig = app.testTsconfig || defaults.testTsConfig; + const tsSpecConfigPath = join(normalize(app.root || ''), testTsConfig); const buffer = host.read(tsSpecConfigPath); + if (!buffer) { return; } - const tsCfg = JSON.parse(buffer.toString()); - if (!tsCfg.files) { - tsCfg.files = []; + + + const tsCfgAst = parseJsonAst(buffer.toString(), JsonParseMode.Loose); + if (tsCfgAst.kind != 'object') { + throw new SchematicsException('Invalid tsconfig. Was expecting an object'); } - // Ensure the spec tsconfig contains the polyfills file - if (tsCfg.files.indexOf(app.polyfills || defaults.polyfills) === -1) { - tsCfg.files.push(app.polyfills || defaults.polyfills); - host.overwrite(tsSpecConfigPath, JSON.stringify(tsCfg, null, 2)); + const filesAstNode = findPropertyInAstObject(tsCfgAst, 'files'); + if (filesAstNode && filesAstNode.kind != 'array') { + throw new SchematicsException('Invalid tsconfig "files" property; expected an array.'); } + + const recorder = host.beginUpdate(tsSpecConfigPath); + + const polyfills = app.polyfills || defaults.polyfills; + if (!filesAstNode) { + // Do nothing if the files array does not exist. This means exclude or include are + // set and we shouldn't mess with that. + } else { + if (filesAstNode.value.indexOf(polyfills) == -1) { + appendValueInAstArray(recorder, filesAstNode, polyfills); + } + } + + host.commitUpdate(recorder); }); }; } -function updatePackageJson(packageManager?: string) { +function updatePackageJson(config: CliConfig) { return (host: Tree, context: SchematicContext) => { const pkgPath = '/package.json'; const buffer = host.read(pkgPath); if (buffer == null) { throw new SchematicsException('Could not read package.json'); } - const content = buffer.toString(); - const pkg = JSON.parse(content); + const pkgAst = parseJsonAst(buffer.toString(), JsonParseMode.Strict); - if (pkg === null || typeof pkg !== 'object' || Array.isArray(pkg)) { + if (pkgAst.kind != 'object') { throw new SchematicsException('Error reading package.json'); } - if (!pkg.devDependencies) { - pkg.devDependencies = {}; + + const devDependenciesNode = findPropertyInAstObject(pkgAst, 'devDependencies'); + if (devDependenciesNode && devDependenciesNode.kind != 'object') { + throw new SchematicsException('Error reading package.json; devDependency is not an object.'); } - pkg.devDependencies['@angular-devkit/build-angular'] = latestVersions.DevkitBuildAngular; + const recorder = host.beginUpdate(pkgPath); + const depName = '@angular-devkit/build-angular'; + if (!devDependenciesNode) { + // Haven't found the devDependencies key, add it to the root of the package.json. + appendPropertyInAstObject(recorder, pkgAst, 'devDependencies', { + [depName]: latestVersions.DevkitBuildAngular, + }); + } else { + // Check if there's a build-angular key. + const buildAngularNode = findPropertyInAstObject(devDependenciesNode, depName); + + if (!buildAngularNode) { + // No build-angular package, add it. + appendPropertyInAstObject( + recorder, + devDependenciesNode, + depName, + latestVersions.DevkitBuildAngular, + ); + } else { + const { end, start } = buildAngularNode; + recorder.remove(start.offset, end.offset - start.offset); + recorder.insertRight(start.offset, JSON.stringify(latestVersions.DevkitBuildAngular)); + } + } - host.overwrite(pkgPath, JSON.stringify(pkg, null, 2)); + host.commitUpdate(recorder); - if (packageManager && !['npm', 'yarn', 'cnpm'].includes(packageManager)) { - packageManager = undefined; - } - context.addTask(new NodePackageInstallTask({ packageManager })); + context.addTask(new NodePackageInstallTask({ + packageManager: config.packageManager === 'default' ? undefined : config.packageManager, + })); return host; }; @@ -597,19 +649,52 @@ function updateTsLintConfig(): Rule { const tsLintPath = '/tslint.json'; const buffer = host.read(tsLintPath); if (!buffer) { - return; + return host; } - const tsCfg = JSON.parse(buffer.toString()); + const tsCfgAst = parseJsonAst(buffer.toString(), JsonParseMode.Loose); - if (tsCfg.rules && tsCfg.rules['import-blacklist'] && - tsCfg.rules['import-blacklist'].indexOf('rxjs') !== -1) { + if (tsCfgAst.kind != 'object') { + return host; + } - tsCfg.rules['import-blacklist'] = tsCfg.rules['import-blacklist'] - .filter((rule: string | boolean) => rule !== 'rxjs'); + const rulesNode = findPropertyInAstObject(tsCfgAst, 'rules'); + if (!rulesNode || rulesNode.kind != 'object') { + return host; + } - host.overwrite(tsLintPath, JSON.stringify(tsCfg, null, 2)); + const importBlacklistNode = findPropertyInAstObject(rulesNode, 'import-blacklist'); + if (!importBlacklistNode || importBlacklistNode.kind != 'array') { + return host; } + const recorder = host.beginUpdate(tsLintPath); + for (let i = 0; i < importBlacklistNode.elements.length; i++) { + const element = importBlacklistNode.elements[i]; + if (element.kind == 'string' && element.value == 'rxjs') { + const { start, end } = element; + // Remove this element. + if (i == importBlacklistNode.elements.length - 1) { + // Last element. + if (i > 0) { + // Not first, there's a comma to remove before. + const previous = importBlacklistNode.elements[i - 1]; + recorder.remove(previous.end.offset, end.offset - previous.end.offset); + } else { + // Only element, just remove the whole rule. + const { start, end } = importBlacklistNode; + recorder.remove(start.offset, end.offset - start.offset); + recorder.insertLeft(start.offset, '[]'); + } + } else { + // Middle, just remove the whole node (up to next node start). + const next = importBlacklistNode.elements[i + 1]; + recorder.remove(start.offset, next.start.offset - start.offset); + } + } + } + + host.commitUpdate(recorder); + return host; }; } @@ -627,13 +712,17 @@ export default function (): Rule { if (configBuffer == null) { throw new SchematicsException(`Could not find configuration file (${configPath})`); } - const config = JSON.parse(configBuffer.toString()); + const config = parseJson(configBuffer.toString(), JsonParseMode.Loose); + + if (typeof config != 'object' || Array.isArray(config) || config === null) { + throw new SchematicsException('Invalid angular-cli.json configuration; expected an object.'); + } return chain([ migrateKarmaConfiguration(config), migrateConfiguration(config), updateSpecTsConfig(config), - updatePackageJson(config.packageManager), + updatePackageJson(config), updateTsLintConfig(), (host: Tree, context: SchematicContext) => { context.logger.warn(tags.oneLine`Some configuration options have been changed, diff --git a/packages/schematics/angular/migrations/update-6/index_spec.ts b/packages/schematics/angular/migrations/update-6/index_spec.ts index 573e7fd776..f2c982e43a 100644 --- a/packages/schematics/angular/migrations/update-6/index_spec.ts +++ b/packages/schematics/angular/migrations/update-6/index_spec.ts @@ -9,6 +9,7 @@ import { JsonObject } from '@angular-devkit/core'; import { EmptyTree } from '@angular-devkit/schematics'; import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing'; import * as path from 'path'; +import { latestVersions } from '../../utility/latest-versions'; describe('Migration to v6', () => { @@ -691,7 +692,32 @@ describe('Migration to v6', () => { tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); const content = tree.readContent('/package.json'); const pkg = JSON.parse(content); - expect(pkg.devDependencies['@angular-devkit/build-angular']).toBeDefined(); + expect(pkg.devDependencies['@angular-devkit/build-angular']) + .toBe(latestVersions.DevkitBuildAngular); + }); + + it('should add a dev dependency to @angular-devkit/build-angular (present)', () => { + tree.create(oldConfigPath, JSON.stringify(baseConfig, null, 2)); + tree.overwrite('/package.json', JSON.stringify({ + devDependencies: { + '@angular-devkit/build-angular': '0.0.0', + }, + }, null, 2)); + tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); + const content = tree.readContent('/package.json'); + const pkg = JSON.parse(content); + expect(pkg.devDependencies['@angular-devkit/build-angular']) + .toBe(latestVersions.DevkitBuildAngular); + }); + + it('should add a dev dependency to @angular-devkit/build-angular (no dev)', () => { + tree.create(oldConfigPath, JSON.stringify(baseConfig, null, 2)); + tree.overwrite('/package.json', JSON.stringify({}, null, 2)); + tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); + const content = tree.readContent('/package.json'); + const pkg = JSON.parse(content); + expect(pkg.devDependencies['@angular-devkit/build-angular']) + .toBe(latestVersions.DevkitBuildAngular); }); }); @@ -702,7 +728,7 @@ describe('Migration to v6', () => { beforeEach(() => { tslintConfig = { rules: { - 'import-blacklist': ['rxjs'], + 'import-blacklist': ['some', 'rxjs', 'else'], }, }; }); @@ -712,8 +738,34 @@ describe('Migration to v6', () => { tree.create(tslintPath, JSON.stringify(tslintConfig, null, 2)); tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); const tslint = JSON.parse(tree.readContent(tslintPath)); - const blacklist = tslint.rules['import-blacklist']; - expect(blacklist).toEqual([]); + expect(tslint.rules['import-blacklist']).toEqual(['some', 'else']); + }); + + it('should remove "rxjs" from the "import-blacklist" rule (only)', () => { + tree.create(oldConfigPath, JSON.stringify(baseConfig, null, 2)); + tslintConfig.rules['import-blacklist'] = ['rxjs']; + tree.create(tslintPath, JSON.stringify(tslintConfig, null, 2)); + tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); + const tslint = JSON.parse(tree.readContent(tslintPath)); + expect(tslint.rules['import-blacklist']).toEqual([]); + }); + + it('should remove "rxjs" from the "import-blacklist" rule (first)', () => { + tree.create(oldConfigPath, JSON.stringify(baseConfig, null, 2)); + tslintConfig.rules['import-blacklist'] = ['rxjs', 'else']; + tree.create(tslintPath, JSON.stringify(tslintConfig, null, 2)); + tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); + const tslint = JSON.parse(tree.readContent(tslintPath)); + expect(tslint.rules['import-blacklist']).toEqual(['else']); + }); + + it('should remove "rxjs" from the "import-blacklist" rule (last)', () => { + tree.create(oldConfigPath, JSON.stringify(baseConfig, null, 2)); + tslintConfig.rules['import-blacklist'] = ['some', 'rxjs']; + tree.create(tslintPath, JSON.stringify(tslintConfig, null, 2)); + tree = schematicRunner.runSchematic('migration-01', defaultOptions, tree); + const tslint = JSON.parse(tree.readContent(tslintPath)); + expect(tslint.rules['import-blacklist']).toEqual(['some']); }); it('should work if "rxjs" is not in the "import-blacklist" rule', () => { diff --git a/packages/schematics/angular/migrations/update-6/json-utils.ts b/packages/schematics/angular/migrations/update-6/json-utils.ts new file mode 100644 index 0000000000..6397fa4b61 --- /dev/null +++ b/packages/schematics/angular/migrations/update-6/json-utils.ts @@ -0,0 +1,70 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import { JsonAstArray, JsonAstNode, JsonAstObject, JsonValue } from '@angular-devkit/core'; +import { UpdateRecorder } from '@angular-devkit/schematics'; + +export function appendPropertyInAstObject( + recorder: UpdateRecorder, + node: JsonAstObject, + propertyName: string, + value: JsonValue, + indent = 4, +) { + const indentStr = '\n' + new Array(indent + 1).join(' '); + + if (node.properties.length > 0) { + // Insert comma. + const last = node.properties[node.properties.length - 1]; + recorder.insertRight(last.start.offset + last.text.replace(/\s+$/, '').length, ','); + } + + recorder.insertLeft( + node.end.offset - 1, + ' ' + + `"${propertyName}": ${JSON.stringify(value, null, 2).replace(/\n/g, indentStr)}` + + indentStr.slice(0, -2), + ); +} + + +export function appendValueInAstArray( + recorder: UpdateRecorder, + node: JsonAstArray, + value: JsonValue, + indent = 4, +) { + const indentStr = '\n' + new Array(indent + 1).join(' '); + + if (node.elements.length > 0) { + // Insert comma. + const last = node.elements[node.elements.length - 1]; + recorder.insertRight(last.start.offset + last.text.replace(/\s+$/, '').length, ','); + } + + recorder.insertLeft( + node.end.offset - 1, + ' ' + + JSON.stringify(value, null, 2).replace(/\n/g, indentStr) + + indentStr.slice(0, -2), + ); +} + + +export function findPropertyInAstObject( + node: JsonAstObject, + propertyName: string, +): JsonAstNode | null { + let maybeNode: JsonAstNode | null = null; + for (const property of node.properties) { + if (property.key.value == propertyName) { + maybeNode = property.value; + } + } + + return maybeNode; +}