fix(core): static-query schematic should detect static queries in getters. (#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
This commit is contained in:
parent
b2c03b8cd4
commit
33016b8929
@ -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 {
|
isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
|
||||||
const nodeQueue: ts.Node[] = [searchNode];
|
const nodeQueue: ts.Node[] = [searchNode];
|
||||||
this.visitedJumpExprSymbols.clear();
|
this.visitedJumpExprSymbols.clear();
|
||||||
@ -97,6 +116,12 @@ export class DeclarationUsageVisitor {
|
|||||||
this.addNewExpressionToQueue(node, nodeQueue);
|
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
|
// 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
|
// synchronously (e.g. function declarations). We only want to check TypeScript
|
||||||
// nodes which are synchronously executed in the control flow.
|
// nodes which are synchronously executed in the control flow.
|
||||||
|
@ -780,6 +780,67 @@ describe('static-queries migration', () => {
|
|||||||
.toContain(`@${queryType}('test', { static: true }) query: any;`);
|
.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', () => {
|
it('should properly handle multiple tsconfig files', () => {
|
||||||
writeFile('/src/index.ts', `
|
writeFile('/src/index.ts', `
|
||||||
import {Component, ${queryType}} from '@angular/core';
|
import {Component, ${queryType}} from '@angular/core';
|
||||||
|
Loading…
x
Reference in New Issue
Block a user