diff --git a/packages/core/schematics/migrations/move-document/index.ts b/packages/core/schematics/migrations/move-document/index.ts index 534af46b20..10e5efd046 100644 --- a/packages/core/schematics/migrations/move-document/index.ts +++ b/packages/core/schematics/migrations/move-document/index.ts @@ -19,15 +19,15 @@ import {addToImport, createImport, removeFromImport} from './move-import'; /** Entry point for the V8 move-document migration. */ export default function(): Rule { return (tree: Tree) => { - const projectTsConfigPaths = getProjectTsConfigPaths(tree); + const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); const basePath = process.cwd(); - if (!projectTsConfigPaths.length) { + if (!buildPaths.length && !testPaths.length) { throw new SchematicsException(`Could not find any tsconfig file. Cannot migrate DOCUMENT to new import source.`); } - for (const tsconfigPath of projectTsConfigPaths) { + for (const tsconfigPath of [...buildPaths, ...testPaths]) { runMoveDocumentMigration(tree, tsconfigPath, basePath); } }; diff --git a/packages/core/schematics/migrations/static-queries/index.ts b/packages/core/schematics/migrations/static-queries/index.ts index 12aea9c7e1..0b4797f49c 100644 --- a/packages/core/schematics/migrations/static-queries/index.ts +++ b/packages/core/schematics/migrations/static-queries/index.ts @@ -19,10 +19,17 @@ import {TypeScriptVisitor, visitAllNodes} from '../../utils/typescript/visit_nod import {NgQueryResolveVisitor} from './angular/ng_query_visitor'; import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy'; +import {QueryTestStrategy} from './strategies/test_strategy/test_strategy'; import {TimingStrategy} from './strategies/timing-strategy'; import {QueryUsageStrategy} from './strategies/usage_strategy/usage_strategy'; import {getTransformedQueryCallExpr} from './transform'; +export enum SELECTED_STRATEGY { + TEMPLATE, + USAGE, + TESTS, +} + /** Entry point for the V8 static-query migration. */ export default function(): Rule { return (tree: Tree, context: SchematicContext) => { @@ -34,7 +41,7 @@ export default function(): Rule { /** Runs the V8 migration static-query migration for all determined TypeScript projects. */ async function runMigration(tree: Tree, context: SchematicContext) { - const projectTsConfigPaths = getProjectTsConfigPaths(tree); + const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); const basePath = process.cwd(); const logger = context.logger; @@ -44,7 +51,7 @@ async function runMigration(tree: Tree, context: SchematicContext) { logger.info('https://github.com/angular/angular/pull/28810'); logger.info(''); - if (!projectTsConfigPaths.length) { + if (!buildPaths.length && !testPaths.length) { throw new SchematicsException( 'Could not find any tsconfig file. Cannot migrate queries ' + 'to explicit timing.'); @@ -52,7 +59,7 @@ async function runMigration(tree: Tree, context: SchematicContext) { // In case prompts are supported, determine the desired migration strategy // by creating a choice prompt. By default the template strategy is used. - let isUsageStrategy = false; + let selectedStrategy: SELECTED_STRATEGY = SELECTED_STRATEGY.TEMPLATE; if (supportsPrompt()) { logger.info('There are two available migration strategies that can be selected:'); logger.info(' • Template strategy - migration tool (short-term gains, rare corrections)'); @@ -70,17 +77,26 @@ async function runMigration(tree: Tree, context: SchematicContext) { default: 'template', }); logger.info(''); - isUsageStrategy = strategyName === 'usage'; + selectedStrategy = + strategyName === 'usage' ? SELECTED_STRATEGY.USAGE : SELECTED_STRATEGY.TEMPLATE; } else { // In case prompts are not supported, we still want to allow developers to opt // into the usage strategy by specifying an environment variable. The tests also // use the environment variable as there is no headless way to select via prompt. - isUsageStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY']; + selectedStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] ? SELECTED_STRATEGY.USAGE : + SELECTED_STRATEGY.TEMPLATE; } const failures = []; - for (const tsconfigPath of projectTsConfigPaths) { - failures.push(...await runStaticQueryMigration(tree, tsconfigPath, basePath, isUsageStrategy)); + + for (const tsconfigPath of buildPaths) { + failures.push(...await runStaticQueryMigration(tree, tsconfigPath, basePath, selectedStrategy)); + } + // For the "test" tsconfig projects we always want to use the test strategy as + // we can't detect the proper timing within spec files. + for (const tsconfigPath of testPaths) { + failures.push( + ...await runStaticQueryMigration(tree, tsconfigPath, basePath, SELECTED_STRATEGY.TESTS)); } if (failures.length) { @@ -99,7 +115,7 @@ async function runMigration(tree: Tree, context: SchematicContext) { * lifecycle hook does not need to be static and can be set up with "static: false". */ async function runStaticQueryMigration( - tree: Tree, tsconfigPath: string, basePath: string, isUsageStrategy: boolean) { + tree: Tree, tsconfigPath: string, basePath: string, selectedStrategy: SELECTED_STRATEGY) { const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath)); const host = ts.createCompilerHost(parsed.options, true); @@ -119,10 +135,11 @@ async function runStaticQueryMigration( const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !); const printer = ts.createPrinter(); const analysisVisitors: TypeScriptVisitor[] = [queryVisitor]; + const failureMessages: string[] = []; // If the "usage" strategy is selected, we also need to add the query visitor // to the analysis visitors so that query usage in templates can be also checked. - if (isUsageStrategy) { + if (selectedStrategy === SELECTED_STRATEGY.USAGE) { analysisVisitors.push(templateVisitor); } @@ -136,7 +153,7 @@ async function runStaticQueryMigration( const {resolvedQueries, classMetadata} = queryVisitor; const {resolvedTemplates} = templateVisitor; - if (isUsageStrategy) { + if (selectedStrategy === SELECTED_STRATEGY.USAGE) { // Add all resolved templates to the class metadata if the usage strategy is used. This // is necessary in order to be able to check component templates for static query usage. resolvedTemplates.forEach(template => { @@ -146,10 +163,14 @@ async function runStaticQueryMigration( }); } - const strategy: TimingStrategy = isUsageStrategy ? - new QueryUsageStrategy(classMetadata, typeChecker) : - new QueryTemplateStrategy(tsconfigPath, classMetadata, host); - const failureMessages: string[] = []; + let strategy: TimingStrategy; + if (selectedStrategy === SELECTED_STRATEGY.USAGE) { + strategy = new QueryUsageStrategy(classMetadata, typeChecker); + } else if (selectedStrategy === SELECTED_STRATEGY.TESTS) { + strategy = new QueryTestStrategy(); + } else { + strategy = new QueryTemplateStrategy(tsconfigPath, classMetadata, host); + } // In case the strategy could not be set up properly, we just exit the // migration. We don't want to throw an exception as this could mean diff --git a/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts index 21d6c217ae..c19c9a6c08 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts +++ b/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts @@ -184,6 +184,8 @@ export class QueryTemplateStrategy implements TimingStrategy { console.error('Could not create Angular AOT compiler to determine query timing.'); console.error('The following diagnostics were detected:\n'); console.error(diagnostics.map(d => d.messageText).join(`\n`)); + console.error('Please make sure that there is no compilation failure. The migration'); + console.error('can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"'); } private _getViewQueryUniqueKey(filePath: string, className: string, propName: string) { diff --git a/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts new file mode 100644 index 0000000000..30c783c0aa --- /dev/null +++ b/packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts @@ -0,0 +1,28 @@ +/** + * @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 {NgQueryDefinition} from '../../angular/query-definition'; +import {TimingResult, TimingStrategy} from '../timing-strategy'; + +/** + * Query timing strategy that is used for queries used within test files. The query + * timing is not analyzed for test files as the template strategy cannot work within + * spec files (due to missing component modules) and the usage strategy is not capable + * of detecting the timing of queries based on how they are used in tests. + */ +export class QueryTestStrategy implements TimingStrategy { + setup() { return true; } + + /** + * Detects the timing for a given query. For queries within tests, we always + * add a TODO and print a message saying that the timing can't be detected for tests. + */ + detectTiming(query: NgQueryDefinition): TimingResult { + return {timing: null, message: 'Timing within tests cannot be detected.'}; + } +} diff --git a/packages/core/schematics/migrations/static-queries/transform.ts b/packages/core/schematics/migrations/static-queries/transform.ts index 7863215946..1139afcdc8 100644 --- a/packages/core/schematics/migrations/static-queries/transform.ts +++ b/packages/core/schematics/migrations/static-queries/transform.ts @@ -10,6 +10,8 @@ import * as ts from 'typescript'; import {getPropertyNameText} from '../../utils/typescript/property_name'; import {NgQueryDefinition, QueryTiming} from './angular/query-definition'; +const TODO_COMMENT = 'TODO: add static flag'; + /** * Transforms the given query decorator by explicitly specifying the timing based on the * determined timing. The updated decorator call expression node will be returned. @@ -39,7 +41,9 @@ export function getTransformedQueryCallExpr( const updatedOptions = ts.updateObjectLiteral( existingOptions, existingOptions.properties.concat(queryPropertyAssignments)); - if (createTodo) { + // In case we want to add a todo and the options do not have the todo + // yet, we add the query timing todo as synthetic multi-line comment. + if (createTodo && !existingOptions.getFullText().includes(TODO_COMMENT)) { addQueryTimingTodoToNode(updatedOptions); } @@ -68,6 +72,6 @@ function addQueryTimingTodoToNode(node: ts.Node) { end: -1, hasTrailingNewLine: false, kind: ts.SyntaxKind.MultiLineCommentTrivia, - text: ' TODO: add static flag ' + text: ` ${TODO_COMMENT} ` }]); } diff --git a/packages/core/schematics/migrations/template-var-assignment/index.ts b/packages/core/schematics/migrations/template-var-assignment/index.ts index f6d534cefe..29fc6203ad 100644 --- a/packages/core/schematics/migrations/template-var-assignment/index.ts +++ b/packages/core/schematics/migrations/template-var-assignment/index.ts @@ -27,16 +27,16 @@ const FAILURE_MESSAGE = `Found assignment to template variable.`; /** Entry point for the V8 template variable assignment schematic. */ export default function(): Rule { return (tree: Tree, context: SchematicContext) => { - const projectTsConfigPaths = getProjectTsConfigPaths(tree); + const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); const basePath = process.cwd(); - if (!projectTsConfigPaths.length) { + if (!buildPaths.length && !testPaths.length) { throw new SchematicsException( 'Could not find any tsconfig file. Cannot check templates for template variable ' + 'assignments.'); } - for (const tsconfigPath of projectTsConfigPaths) { + for (const tsconfigPath of [...buildPaths, ...testPaths]) { runTemplateVariableAssignmentCheck(tree, tsconfigPath, basePath, context.logger); } }; diff --git a/packages/core/schematics/test/move_document_migration_spec.ts b/packages/core/schematics/test/move_document_migration_spec.ts index 64d181876d..85f55411ac 100644 --- a/packages/core/schematics/test/move_document_migration_spec.ts +++ b/packages/core/schematics/test/move_document_migration_spec.ts @@ -29,6 +29,9 @@ describe('move-document migration', () => { lib: ['es2015'], } })); + writeFile('/angular.json', JSON.stringify({ + projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} + })); previousWorkingDir = shx.pwd(); tmpDirPath = getSystemPath(host.root); diff --git a/packages/core/schematics/test/project_tsconfig_paths_spec.ts b/packages/core/schematics/test/project_tsconfig_paths_spec.ts index 31cc793ae7..0091ce0070 100644 --- a/packages/core/schematics/test/project_tsconfig_paths_spec.ts +++ b/packages/core/schematics/test/project_tsconfig_paths_spec.ts @@ -22,7 +22,16 @@ describe('project tsconfig paths', () => { {my_name: {architect: {build: {options: {tsConfig: './my-custom-config.json'}}}}} })); - expect(getProjectTsConfigPaths(testTree)).toEqual(['my-custom-config.json']); + expect(getProjectTsConfigPaths(testTree).buildPaths).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: {my_name: {architect: {test: {options: {tsConfig: './my-test-config.json'}}}}} + })); + + expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['my-test-config.json']); }); it('should detect test tsconfig path inside of .angular.json file', () => { @@ -32,25 +41,23 @@ describe('project tsconfig paths', () => { {with_tests: {architect: {test: {options: {tsConfig: './my-test-config.json'}}}}} })); - expect(getProjectTsConfigPaths(testTree)).toEqual(['my-test-config.json']); + expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['my-test-config.json']); }); it('should detect common tsconfigs if no workspace config could be found', () => { - testTree.create('/tsconfig.json', ''); - testTree.create('/src/tsconfig.json', ''); testTree.create('/src/tsconfig.app.json', ''); + testTree.create('/src/tsconfig.spec.json', ''); - expect(getProjectTsConfigPaths(testTree)).toEqual([ - 'tsconfig.json', 'src/tsconfig.json', 'src/tsconfig.app.json' - ]); + expect(getProjectTsConfigPaths(testTree).buildPaths).toEqual(['src/tsconfig.app.json']); + expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['src/tsconfig.spec.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'}}}}} + projects: {app: {architect: {build: {options: {tsConfig: 'tsconfig.json'}}}}} })); - expect(getProjectTsConfigPaths(testTree)).toEqual(['tsconfig.json']); + expect(getProjectTsConfigPaths(testTree).buildPaths).toEqual(['tsconfig.json']); }); }); diff --git a/packages/core/schematics/test/static_queries_migration_template_spec.ts b/packages/core/schematics/test/static_queries_migration_template_spec.ts index 8cdbf8091b..9aab9a5978 100644 --- a/packages/core/schematics/test/static_queries_migration_template_spec.ts +++ b/packages/core/schematics/test/static_queries_migration_template_spec.ts @@ -31,6 +31,9 @@ describe('static-queries migration with template strategy', () => { lib: ['es2015'], } })); + writeFile('/angular.json', JSON.stringify({ + projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} + })); warnOutput = []; runner.logger.subscribe(logEntry => { @@ -515,5 +518,54 @@ describe('static-queries migration with template strategy', () => { expect(console.error).toHaveBeenCalledTimes(0); }); + + it('should always use the test migration strategy for test tsconfig files', async() => { + writeFile('/src/tsconfig.spec.json', JSON.stringify({ + compilerOptions: { + experimentalDecorators: true, + lib: ['es2015'], + }, + files: [ + 'test.ts', + ], + })); + + writeFile('/src/test.ts', ` + import {ViewChild} from '@angular/core'; + import {AppComponent} from './app.component'; + + @Component({template: 'Test'}) + class MyTestComponent { + @ViewChild('test') query: any; + } + `); + + writeFile('/src/app.component.ts', ` + import {Component, ViewChild} from '@angular/core'; + + @Component({template: ''}) + export class AppComponent { + @ViewChild('test') query: any; + } + `); + + writeFile('/src/app.module.ts', ` + import {NgModule} from '@angular/core'; + import {AppComponent} from './app.component'; + + @NgModule({declarations: [AppComponent]}) + export class MyModule {} + `); + + spyOn(console, 'error').and.callThrough(); + + await runMigration(); + + expect(console.error).toHaveBeenCalledTimes(0); + expect(tree.readContent('/src/test.ts')) + .toContain(`@ViewChild('test', /* TODO: add static flag */ {}) query: any;`); + expect(tree.readContent('/src/app.component.ts')) + .toContain(`@ViewChild('test', { static: true }) query: any;`); + }); }); }); diff --git a/packages/core/schematics/test/static_queries_migration_usage_spec.ts b/packages/core/schematics/test/static_queries_migration_usage_spec.ts index 7b33fc99f8..f0ece7e9da 100644 --- a/packages/core/schematics/test/static_queries_migration_usage_spec.ts +++ b/packages/core/schematics/test/static_queries_migration_usage_spec.ts @@ -35,6 +35,9 @@ describe('static-queries migration with usage strategy', () => { lib: ['es2015'], } })); + writeFile('/angular.json', JSON.stringify({ + projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} + })); previousWorkingDir = shx.pwd(); tmpDirPath = getSystemPath(host.root); @@ -598,6 +601,9 @@ describe('static-queries migration with usage strategy', () => { // Move the tsconfig into a subdirectory. This ensures that the update is properly // recorded for TypeScript projects not at the schematic tree root. host.sync.rename(normalize('/tsconfig.json'), normalize('/src/tsconfig.json')); + writeFile('/angular.json', JSON.stringify({ + projects: {t: {architect: {build: {options: {tsConfig: './src/tsconfig.json'}}}}} + })); await runMigration(); diff --git a/packages/core/schematics/test/template_var_assignment_migration_spec.ts b/packages/core/schematics/test/template_var_assignment_migration_spec.ts index 810d187c9e..4c4d210192 100644 --- a/packages/core/schematics/test/template_var_assignment_migration_spec.ts +++ b/packages/core/schematics/test/template_var_assignment_migration_spec.ts @@ -30,6 +30,9 @@ describe('template variable assignment migration', () => { lib: ['es2015'], } })); + writeFile('/angular.json', JSON.stringify({ + projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} + })); warnOutput = []; runner.logger.subscribe(logEntry => { diff --git a/packages/core/schematics/utils/project_tsconfig_paths.ts b/packages/core/schematics/utils/project_tsconfig_paths.ts index a6ef3219c7..05d46ae1cc 100644 --- a/packages/core/schematics/utils/project_tsconfig_paths.ts +++ b/packages/core/schematics/utils/project_tsconfig_paths.ts @@ -8,6 +8,7 @@ import {normalize} from '@angular-devkit/core'; import {Tree} from '@angular-devkit/schematics'; +import {WorkspaceProject} from '@schematics/angular/utility/workspace-models'; /** Name of the default Angular CLI workspace configuration files. */ const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json']; @@ -16,13 +17,11 @@ 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. */ -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', - ]); +export function getProjectTsConfigPaths(tree: Tree): {buildPaths: string[], testPaths: string[]} { + // Start with some tsconfig paths that are generally used within CLI projects. Note + // that we are not interested in IDE-specific tsconfig files (e.g. /tsconfig.json) + const buildPaths = new Set(['src/tsconfig.app.json']); + const testPaths = new Set(['src/tsconfig.spec.json']); // Add any tsconfig directly referenced in a build or test task of the angular.json workspace. const workspace = getWorkspaceConfigGracefully(tree); @@ -30,23 +29,38 @@ export function getProjectTsConfigPaths(tree: Tree): string[] { if (workspace) { const projects = Object.keys(workspace.projects).map(name => workspace.projects[name]); for (const project of projects) { - ['build', 'test'].forEach(targetName => { - if (project.targets && project.targets[targetName] && project.targets[targetName].options && - project.targets[targetName].options.tsConfig) { - tsconfigPaths.add(normalize(project.targets[targetName].options.tsConfig)); - } + const buildPath = getTargetTsconfigPath(project, 'build'); + const testPath = getTargetTsconfigPath(project, 'test'); - if (project.architect && project.architect[targetName] && - project.architect[targetName].options && - project.architect[targetName].options.tsConfig) { - tsconfigPaths.add(normalize(project.architect[targetName].options.tsConfig)); - } - }); + if (buildPath) { + buildPaths.add(buildPath); + } + + if (testPath) { + testPaths.add(testPath); + } } } // Filter out tsconfig files that don't exist in the CLI project. - return Array.from(tsconfigPaths).filter(p => tree.exists(p)); + return { + buildPaths: Array.from(buildPaths).filter(p => tree.exists(p)), + testPaths: Array.from(testPaths).filter(p => tree.exists(p)), + }; +} + +/** Gets the tsconfig path from the given target within the specified project. */ +function getTargetTsconfigPath(project: WorkspaceProject, targetName: string): string|null { + if (project.targets && project.targets[targetName] && project.targets[targetName].options && + project.targets[targetName].options.tsConfig) { + return normalize(project.targets[targetName].options.tsConfig); + } + + if (project.architect && project.architect[targetName] && project.architect[targetName].options && + project.architect[targetName].options.tsConfig) { + return normalize(project.architect[targetName].options.tsConfig); + } + return null; } /**