Skip to content
Permalink
Browse files

fix(core): static-query schematic should detect static queries in get…

…ters. (#29609)

Queries can also be statically accessed within getters. e.g.

```ts
ngOnInit() {
  this.myQueryGetter.doSomething();
}
```

In that case we need to check if the `myQueryGetter` definition accesses
a query statically. As we need to use the type checker for every property acess
within lifecylce hooks, the schematic might become slower than before, but considering
that this is a one-time execution, it is totally fine using the type-checker extensively.

PR Close #29609
  • Loading branch information...
devversion authored and jasonaden committed Mar 30, 2019
1 parent b2c03b8 commit 33016b89297f13875ac8af72c3fb14bff838f63c
@@ -74,6 +74,25 @@ export class DeclarationUsageVisitor {
}
}

private visitPropertyAccessExpression(node: ts.PropertyAccessExpression, nodeQueue: ts.Node[]) {
const propertySymbol = this.typeChecker.getSymbolAtLocation(node.name);

if (!propertySymbol || !propertySymbol.valueDeclaration ||
this.visitedJumpExprSymbols.has(propertySymbol)) {
return;
}

const valueDeclaration = propertySymbol.valueDeclaration;

// In case the property access expression refers to a get accessor, we need to visit
// the body of the get accessor declaration as there could be logic that uses the
// given search node synchronously.
if (ts.isGetAccessorDeclaration(valueDeclaration) && valueDeclaration.body) {
this.visitedJumpExprSymbols.add(propertySymbol);
nodeQueue.push(valueDeclaration.body);
}
}

isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
const nodeQueue: ts.Node[] = [searchNode];
this.visitedJumpExprSymbols.clear();
@@ -97,6 +116,12 @@ export class DeclarationUsageVisitor {
this.addNewExpressionToQueue(node, nodeQueue);
}

// Handle property access expressions. These could resolve to get-accessor declarations
// which can contain synchronous logic that accesses the search node.
if (ts.isPropertyAccessExpression(node)) {
this.visitPropertyAccessExpression(node, nodeQueue);
}

// Do not visit nodes that declare a block of statements but are not executed
// synchronously (e.g. function declarations). We only want to check TypeScript
// nodes which are synchronously executed in the control flow.
@@ -780,6 +780,67 @@ describe('static-queries migration', () => {
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});

it('should detect static queries used through getter property access', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
@Component({template: '<span #test></span>'})
export class MyComp {
private @${queryType}('test') query: any;
get myProp() {
return this.query.myValue;
}
ngOnInit() {
this.myProp.test();
}
}
`);

runMigration();

expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});

it('should detect static queries used through external getter access', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
import {External} from './external';
@Component({template: '<span #test></span>'})
export class MyComp {
@${queryType}('test') query: any;
private external = new External(this);
get myProp() {
return this.query.myValue;
}
ngOnInit() {
console.log(this.external.query);
}
}
`);

writeFile('/external.ts', `
import {MyComp} from './index';
export class External {
constructor(private comp: MyComp) {}
get query() { return this.comp.query; }
}
`);

runMigration();

expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});

it('should properly handle multiple tsconfig files', () => {
writeFile('/src/index.ts', `
import {Component, ${queryType}} from '@angular/core';

0 comments on commit 33016b8

Please sign in to comment.
You can’t perform that action at this time.