Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, []) ?
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`, `
<span #myRef>My Ref</span>
`);

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`, `
<span #myRef>My Ref</span>
`);

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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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: '<span #test></span>'})
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: '<span #test></span>'})
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';
Expand Down