diff --git a/packages/core/schematics/migrations/static-queries/angular/ng_query_visitor.ts b/packages/core/schematics/migrations/static-queries/angular/ng_query_visitor.ts index a413099724..a44897c6e2 100644 --- a/packages/core/schematics/migrations/static-queries/angular/ng_query_visitor.ts +++ b/packages/core/schematics/migrations/static-queries/angular/ng_query_visitor.ts @@ -11,6 +11,7 @@ import * as ts from 'typescript'; import {ResolvedTemplate} from '../../../utils/ng_component_template'; import {getAngularDecorators} from '../../../utils/ng_decorators'; import {findParentClassDeclaration, getBaseTypeIdentifiers} from '../../../utils/typescript/class_declaration'; +import {getPropertyNameText} from '../../../utils/typescript/property_name'; import {getInputNamesOfClass} from './directive_inputs'; import {NgQueryDefinition, QueryType} from './query-definition'; @@ -53,12 +54,30 @@ export class NgQueryResolveVisitor { case ts.SyntaxKind.ClassDeclaration: this.visitClassDeclaration(node as ts.ClassDeclaration); break; + case ts.SyntaxKind.GetAccessor: + case ts.SyntaxKind.SetAccessor: + this.visitAccessorDeclaration(node as ts.AccessorDeclaration); + break; } ts.forEachChild(node, n => this.visitNode(n)); } private visitPropertyDeclaration(node: ts.PropertyDeclaration) { + this._recordQueryDeclaration(node, node, getPropertyNameText(node.name)); + } + + private visitAccessorDeclaration(node: ts.AccessorDeclaration) { + this._recordQueryDeclaration(node, null, getPropertyNameText(node.name)); + } + + private visitClassDeclaration(node: ts.ClassDeclaration) { + this._recordClassInputSetters(node); + this._recordClassInheritances(node); + } + + private _recordQueryDeclaration( + node: ts.Node, property: ts.PropertyDeclaration|null, queryName: string|null) { if (!node.decorators || !node.decorators.length) { return; } @@ -83,18 +102,15 @@ export class NgQueryResolveVisitor { const newQueries = this.resolvedQueries.get(sourceFile) || []; this.resolvedQueries.set(sourceFile, newQueries.concat({ + name: queryName, type: queryDecorator.name === 'ViewChild' ? QueryType.ViewChild : QueryType.ContentChild, - property: node, + node, + property, decorator: queryDecorator, container: queryContainer, })); } - private visitClassDeclaration(node: ts.ClassDeclaration) { - this._recordClassInputSetters(node); - this._recordClassInheritances(node); - } - private _recordClassInputSetters(node: ts.ClassDeclaration) { const resolvedInputNames = getInputNamesOfClass(node, this.typeChecker); diff --git a/packages/core/schematics/migrations/static-queries/angular/query-definition.ts b/packages/core/schematics/migrations/static-queries/angular/query-definition.ts index 929f0fd872..46af4df795 100644 --- a/packages/core/schematics/migrations/static-queries/angular/query-definition.ts +++ b/packages/core/schematics/migrations/static-queries/angular/query-definition.ts @@ -22,10 +22,17 @@ export enum QueryType { } export interface NgQueryDefinition { + /** Name of the query. Set to "null" in case the query name is not statically analyzable. */ + name: string|null; /** Type of the query definition. */ type: QueryType; - /** Property that declares the query. */ - property: ts.PropertyDeclaration; + /** Node that declares this query. */ + node: ts.Node; + /** + * Property declaration that refers to the query value. For accessors there + * is no property that is guaranteed to access the query value. + */ + property: ts.PropertyDeclaration|null; /** Decorator that declares this as a query. */ decorator: NgDecorator; /** Class declaration that holds this query. */ 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 dc2c489f2c..f7c0084470 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 @@ -99,13 +99,13 @@ export class QueryTemplateStrategy implements TimingStrategy { detectTiming(query: NgQueryDefinition): TimingResult { if (query.type === QueryType.ContentChild) { return {timing: null, message: 'Content queries cannot be migrated automatically.'}; - } else if (!hasPropertyNameText(query.property.name)) { + } else if (!query.name) { // In case the query property name is not statically analyzable, we mark this // query as unresolved. NGC currently skips these view queries as well. return {timing: null, message: 'Query is not statically analyzable.'}; } - const propertyName = query.property.name.text; + const propertyName = query.name; const classMetadata = this.classMetadata.get(query.container); // In case there is no class metadata or there are no derived classes that diff --git a/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts index 735ec58faa..4ee07dc6f5 100644 --- a/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts +++ b/packages/core/schematics/migrations/static-queries/strategies/usage_strategy/usage_strategy.ts @@ -44,6 +44,10 @@ export class QueryUsageStrategy implements TimingStrategy { * on the current usage of the query. */ detectTiming(query: NgQueryDefinition): TimingResult { + if (query.property === null) { + return {timing: null, message: 'Queries defined on accessors cannot be analyzed.'}; + } + return { timing: isQueryUsedStatically(query.container, query, this.classMetadata, this.typeChecker, []) ? @@ -62,7 +66,7 @@ function isQueryUsedStatically( classDecl: ts.ClassDeclaration, query: NgQueryDefinition, classMetadataMap: ClassMetadataMap, typeChecker: ts.TypeChecker, knownInputNames: string[], functionCtx: FunctionContext = new Map(), visitInheritedClasses = true): boolean { - const usageVisitor = new DeclarationUsageVisitor(query.property, typeChecker, functionCtx); + const usageVisitor = new DeclarationUsageVisitor(query.property !, typeChecker, functionCtx); const classMetadata = classMetadataMap.get(classDecl); // In case there is metadata for the current class, we collect all resolved Angular input @@ -90,10 +94,10 @@ function isQueryUsedStatically( // In case there is a component template for the current class, we check if the // template statically accesses the current query. In case that's true, the query // can be marked as static. - if (classMetadata.template && hasPropertyNameText(query.property.name)) { + if (classMetadata.template && hasPropertyNameText(query.property !.name)) { const template = classMetadata.template; const parsedHtml = parseHtmlGracefully(template.content, template.filePath); - const htmlVisitor = new TemplateUsageVisitor(query.property.name.text); + const htmlVisitor = new TemplateUsageVisitor(query.property !.name.text); if (parsedHtml && htmlVisitor.isQueryUsedStatically(parsedHtml)) { return true; 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 8a97036433..e8f6b96815 100644 --- a/packages/core/schematics/test/static_queries_migration_template_spec.ts +++ b/packages/core/schematics/test/static_queries_migration_template_spec.ts @@ -401,6 +401,55 @@ describe('static-queries migration with template strategy', () => { .toContain(`@ViewChild('myRef', { static: true }) query: any;`); }); + it('should detect queries declared on setter', async() => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + @Component({templateUrl: 'my-tmpl.html'}) + export class MyComp { + @ViewChild('myRef') + set query(result: any) { /* noop */} + } + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + My Ref + `); + + await runMigration(); + + expect(tree.readContent('/index.ts')) + .toMatch(/@ViewChild\('myRef', { static: true }\)\s+set query/); + }); + + it('should detect queries declared on getter', async() => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + @Component({templateUrl: 'my-tmpl.html'}) + export class MyComp { + @ViewChild('myRef') + get query() { return null; } + set query(result: any) { /* noop */} + } + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + My Ref + `); + + await runMigration(); + + expect(tree.readContent('/index.ts')) + .toMatch(/@ViewChild\('myRef', { static: true }\)\s+get query/); + }); + it('should add a todo if a query is not declared in any component', async() => { writeFile('/index.ts', ` import {Component, NgModule, ViewChild, SomeToken} from '@angular/core'; 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 4397e09686..0e4d0f5563 100644 --- a/packages/core/schematics/test/static_queries_migration_usage_spec.ts +++ b/packages/core/schematics/test/static_queries_migration_usage_spec.ts @@ -18,6 +18,7 @@ describe('static-queries migration with usage strategy', () => { let tree: UnitTestTree; let tmpDirPath: string; let previousWorkingDir: string; + let warnOutput: string[] = []; // Enables the query usage strategy when running the `static-query` migration. By // default the schematic runs the template strategy and there is currently no easy @@ -39,6 +40,13 @@ describe('static-queries migration with usage strategy', () => { projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} })); + warnOutput = []; + runner.logger.subscribe(logEntry => { + if (logEntry.level === 'warn') { + warnOutput.push(logEntry.message); + } + }); + previousWorkingDir = shx.pwd(); tmpDirPath = getSystemPath(host.root); @@ -252,6 +260,47 @@ describe('static-queries migration with usage strategy', () => { .toContain(`@${queryType}('test', { /* test */ read: null, static: true }) query: any;`); }); + it('should add a todo for queries declared on setter', async() => { + writeFile('/index.ts', ` + import {Component, ${queryType}} from '@angular/core'; + + @Component({template: ''}) + export class MyComp { + @${queryType}('test') + set query(result: any) {}; + } + `); + + await runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@${queryType}('test', /* TODO: add static flag */ {})`); + expect(warnOutput.length).toBe(1); + expect(warnOutput[0]) + .toMatch(/index.ts@6:11: Queries defined on accessors cannot be analyzed.$/); + }); + + it('should add a todo for queries declared on getter', async() => { + writeFile('/index.ts', ` + import {Component, ${queryType}} from '@angular/core'; + + @Component({template: ''}) + export class MyComp { + @${queryType}('test') + get query() { return null; } + set query(result: any) {} + } + `); + + await runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@${queryType}('test', /* TODO: add static flag */ {})`); + expect(warnOutput.length).toBe(1); + expect(warnOutput[0]) + .toMatch(/index.ts@6:11: Queries defined on accessors cannot be analyzed.$/); + }); + it('should not overwrite existing explicit query timing', async() => { writeFile('/index.ts', ` import {Component, ${queryType}} from '@angular/core';