fix(core): static-query usage migration strategy should detect ambiguous query usage (#30215)
Currently we always just set the timing to `false` if we aren't
able to analyze a given call expression or new expression. e.g.
```ts
ngOnInit() {
thirdPartyCallSync(() => this.query.doSomething())
}
```
In that case the `thirdPartyCallSync` function comes from the `node_modules`
and is only defined through types while there is no code for the
actual function logic that can be analyzed. This makes it impossible
to tell whether the given call expression actually causes the specified
arrow function to be executed synchronously or not. In order to be able
to make this better, we now peek into the passed arrow function and
check for a synchronous query usage. If so, we set the query timing to
static and mark it as ambiguous. This ensures that the usage strategy is
less "magical" and more correct with third-party code.
Additionally since functions like `setTimeout` are not analyzable but known
to be asynchronous, there is a hard-coded list of known functions which
shouldn't be marked as ambiguous.
Resolves FW-1214. As planned within https://hackmd.io/hPiLWpPlQ4uynC1luIBdfQ
PR Close #30215
This commit is contained in:
Paul Gschwendtner
Alex Rickabaugh
parent
8cec8f5584
commit
8d3365e4fc
|
|
@ -8,9 +8,16 @@
|
|||
|
||||
import * as ts from 'typescript';
|
||||
import {isFunctionLikeDeclaration, unwrapExpression} from '../../../../utils/typescript/functions';
|
||||
import {getPropertyNameText} from '../../../../utils/typescript/property_name';
|
||||
|
||||
export type FunctionContext = Map<ts.Node, ts.Node>;
|
||||
|
||||
export enum ResolvedUsage {
|
||||
SYNCHRONOUS,
|
||||
ASYNCHRONOUS,
|
||||
AMBIGUOUS,
|
||||
}
|
||||
|
||||
/**
|
||||
* List of TypeScript syntax tokens that can be used within a binary expression as
|
||||
* compound assignment. These imply a read and write of the left-side expression.
|
||||
|
|
@ -26,6 +33,19 @@ const BINARY_COMPOUND_TOKENS = [
|
|||
ts.SyntaxKind.SlashEqualsToken,
|
||||
];
|
||||
|
||||
/**
|
||||
* List of known asynchronous external call expressions which aren't analyzable
|
||||
* but are guaranteed to not execute the passed argument synchronously.
|
||||
*/
|
||||
const ASYNC_EXTERNAL_CALLS = [
|
||||
{parent: ['Promise'], name: 'then'},
|
||||
{parent: ['Promise'], name: 'catch'},
|
||||
{parent: [null, 'Window'], name: 'requestAnimationFrame'},
|
||||
{parent: [null, 'Window'], name: 'setTimeout'},
|
||||
{parent: [null, 'Window'], name: 'setInterval'},
|
||||
{parent: ['*'], name: 'addEventListener'},
|
||||
];
|
||||
|
||||
/**
|
||||
* Class that can be used to determine if a given TypeScript node is used within
|
||||
* other given TypeScript nodes. This is achieved by walking through all children
|
||||
|
|
@ -36,9 +56,18 @@ export class DeclarationUsageVisitor {
|
|||
/** Set of visited symbols that caused a jump in control flow. */
|
||||
private visitedJumpExprNodes = new Set<ts.Node>();
|
||||
|
||||
/** Queue of nodes that need to be checked for declaration usage. */
|
||||
/**
|
||||
* Queue of nodes that need to be checked for declaration usage and
|
||||
* are guaranteed to be executed synchronously.
|
||||
*/
|
||||
private nodeQueue: ts.Node[] = [];
|
||||
|
||||
/**
|
||||
* Nodes which need to be checked for declaration usage but aren't
|
||||
* guaranteed to execute synchronously.
|
||||
*/
|
||||
private ambiguousNodeQueue: ts.Node[] = [];
|
||||
|
||||
/**
|
||||
* Function context that holds the TypeScript node values for all parameters
|
||||
* of the currently analyzed function block.
|
||||
|
|
@ -68,6 +97,7 @@ export class DeclarationUsageVisitor {
|
|||
const callExprSymbol = this._getDeclarationSymbolOfNode(node);
|
||||
|
||||
if (!callExprSymbol || !callExprSymbol.valueDeclaration) {
|
||||
this.peekIntoJumpExpression(callExpression);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -77,6 +107,7 @@ export class DeclarationUsageVisitor {
|
|||
// this could cause cycles.
|
||||
if (!isFunctionLikeDeclaration(expressionDecl) ||
|
||||
this.visitedJumpExprNodes.has(expressionDecl) || !expressionDecl.body) {
|
||||
this.peekIntoJumpExpression(callExpression);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -95,6 +126,7 @@ export class DeclarationUsageVisitor {
|
|||
// we should not visit already visited symbols as this could cause cycles.
|
||||
if (!newExprSymbol || !newExprSymbol.valueDeclaration ||
|
||||
!ts.isClassDeclaration(newExprSymbol.valueDeclaration)) {
|
||||
this.peekIntoJumpExpression(node);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -111,6 +143,8 @@ export class DeclarationUsageVisitor {
|
|||
|
||||
this.visitedJumpExprNodes.add(targetConstructor);
|
||||
this.nodeQueue.push(targetConstructor.body);
|
||||
} else {
|
||||
this.peekIntoJumpExpression(node);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -160,7 +194,7 @@ export class DeclarationUsageVisitor {
|
|||
return true;
|
||||
}
|
||||
|
||||
isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
|
||||
getResolvedNodeUsage(searchNode: ts.Node): ResolvedUsage {
|
||||
this.nodeQueue = [searchNode];
|
||||
this.visitedJumpExprNodes.clear();
|
||||
this.context.clear();
|
||||
|
|
@ -171,11 +205,17 @@ export class DeclarationUsageVisitor {
|
|||
// the derived class.
|
||||
this.baseContext.forEach((value, key) => this.context.set(key, value));
|
||||
|
||||
return this.isSynchronouslyUsedInNode(searchNode);
|
||||
}
|
||||
|
||||
private isSynchronouslyUsedInNode(searchNode: ts.Node): ResolvedUsage {
|
||||
this.ambiguousNodeQueue = [];
|
||||
|
||||
while (this.nodeQueue.length) {
|
||||
const node = this.nodeQueue.shift() !;
|
||||
|
||||
if (ts.isIdentifier(node) && this.isReferringToSymbol(node)) {
|
||||
return true;
|
||||
return ResolvedUsage.SYNCHRONOUS;
|
||||
}
|
||||
|
||||
// Handle call expressions within TypeScript nodes that cause a jump in control
|
||||
|
|
@ -218,7 +258,65 @@ export class DeclarationUsageVisitor {
|
|||
this.nodeQueue.push(...node.getChildren());
|
||||
}
|
||||
}
|
||||
return false;
|
||||
|
||||
if (this.ambiguousNodeQueue.length) {
|
||||
// Update the node queue to all stored ambiguous nodes. These nodes are not
|
||||
// guaranteed to be executed and therefore in case of a synchronous usage
|
||||
// within one of those nodes, the resolved usage is ambiguous.
|
||||
this.nodeQueue = this.ambiguousNodeQueue;
|
||||
const usage = this.isSynchronouslyUsedInNode(searchNode);
|
||||
return usage === ResolvedUsage.SYNCHRONOUS ? ResolvedUsage.AMBIGUOUS : usage;
|
||||
}
|
||||
return ResolvedUsage.ASYNCHRONOUS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Peeks into the given jump expression by adding all function like declarations
|
||||
* which are referenced in the jump expression arguments to the ambiguous node
|
||||
* queue. These arguments could technically access the given declaration but it's
|
||||
* not guaranteed that the jump expression is executed. In that case the resolved
|
||||
* usage is ambiguous.
|
||||
*/
|
||||
private peekIntoJumpExpression(jumpExp: ts.CallExpression|ts.NewExpression) {
|
||||
if (!jumpExp.arguments) {
|
||||
return;
|
||||
}
|
||||
|
||||
// For some call expressions we don't want to add the arguments to the
|
||||
// ambiguous node queue. e.g. "setTimeout" is not analyzable but is
|
||||
// guaranteed to execute its argument asynchronously. We handle a subset
|
||||
// of these call expressions by having a hardcoded list of some.
|
||||
if (ts.isCallExpression(jumpExp)) {
|
||||
const symbol = this._getDeclarationSymbolOfNode(jumpExp.expression);
|
||||
if (symbol && symbol.valueDeclaration) {
|
||||
const parentNode = symbol.valueDeclaration.parent;
|
||||
if (parentNode && (ts.isInterfaceDeclaration(parentNode) || ts.isSourceFile(parentNode)) &&
|
||||
(ts.isMethodSignature(symbol.valueDeclaration) ||
|
||||
ts.isFunctionDeclaration(symbol.valueDeclaration)) &&
|
||||
symbol.valueDeclaration.name) {
|
||||
const parentName = ts.isInterfaceDeclaration(parentNode) ? parentNode.name.text : null;
|
||||
const callName = getPropertyNameText(symbol.valueDeclaration.name);
|
||||
if (ASYNC_EXTERNAL_CALLS.some(
|
||||
c =>
|
||||
(c.name === callName &&
|
||||
(c.parent.indexOf(parentName) !== -1 || c.parent.indexOf('*') !== -1)))) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
jumpExp.arguments !.forEach((node: ts.Node) => {
|
||||
node = this._resolveDeclarationOfNode(node);
|
||||
|
||||
if (ts.isVariableDeclaration(node) && node.initializer) {
|
||||
node = node.initializer;
|
||||
}
|
||||
|
||||
if (isFunctionLikeDeclaration(node) && !!node.body) {
|
||||
this.ambiguousNodeQueue.push(node.body);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -252,7 +350,7 @@ export class DeclarationUsageVisitor {
|
|||
}
|
||||
|
||||
if (ts.isIdentifier(argumentNode)) {
|
||||
this.context.set(parameter, this._resolveIdentifier(argumentNode));
|
||||
this.context.set(parameter, this._resolveDeclarationOfNode(argumentNode));
|
||||
} else {
|
||||
this.context.set(parameter, argumentNode);
|
||||
}
|
||||
|
|
@ -260,10 +358,11 @@ export class DeclarationUsageVisitor {
|
|||
}
|
||||
|
||||
/**
|
||||
* Resolves a TypeScript identifier node. For example an identifier can refer to a
|
||||
* function parameter which can be resolved through the function context.
|
||||
* Resolves the declaration of a given TypeScript node. For example an identifier can
|
||||
* refer to a function parameter. This parameter can then be resolved through the
|
||||
* function context.
|
||||
*/
|
||||
private _resolveIdentifier(node: ts.Identifier): ts.Node {
|
||||
private _resolveDeclarationOfNode(node: ts.Node): ts.Node {
|
||||
const symbol = this._getDeclarationSymbolOfNode(node);
|
||||
|
||||
if (!symbol || !symbol.valueDeclaration) {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import {ClassMetadataMap} from '../../angular/ng_query_visitor';
|
|||
import {NgQueryDefinition, QueryTiming, QueryType} from '../../angular/query-definition';
|
||||
import {TimingResult, TimingStrategy} from '../timing-strategy';
|
||||
|
||||
import {DeclarationUsageVisitor, FunctionContext} from './declaration_usage_visitor';
|
||||
import {DeclarationUsageVisitor, FunctionContext, ResolvedUsage} from './declaration_usage_visitor';
|
||||
import {updateSuperClassAbstractMembersContext} from './super_class_context';
|
||||
import {TemplateUsageVisitor} from './template_usage_visitor';
|
||||
|
||||
|
|
@ -48,96 +48,112 @@ export class QueryUsageStrategy implements TimingStrategy {
|
|||
return {timing: null, message: 'Queries defined on accessors cannot be analyzed.'};
|
||||
}
|
||||
|
||||
return {
|
||||
timing:
|
||||
isQueryUsedStatically(query.container, query, this.classMetadata, this.typeChecker, []) ?
|
||||
QueryTiming.STATIC :
|
||||
QueryTiming.DYNAMIC
|
||||
};
|
||||
const usage = this.analyzeQueryUsage(query.container, query, []);
|
||||
|
||||
if (usage === ResolvedUsage.AMBIGUOUS) {
|
||||
return {
|
||||
timing: QueryTiming.STATIC,
|
||||
message: 'Query timing is ambiguous. Please check if the query can be marked as dynamic.'
|
||||
};
|
||||
} else if (usage === ResolvedUsage.SYNCHRONOUS) {
|
||||
return {timing: QueryTiming.STATIC};
|
||||
} else {
|
||||
return {timing: QueryTiming.DYNAMIC};
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a given query is used statically within the given class, its super
|
||||
* class or derived classes.
|
||||
*/
|
||||
private analyzeQueryUsage(
|
||||
classDecl: ts.ClassDeclaration, query: NgQueryDefinition, knownInputNames: string[],
|
||||
functionCtx: FunctionContext = new Map(), visitInheritedClasses = true): ResolvedUsage {
|
||||
const usageVisitor =
|
||||
new DeclarationUsageVisitor(query.property !, this.typeChecker, functionCtx);
|
||||
const classMetadata = this.classMetadata.get(classDecl);
|
||||
let usage: ResolvedUsage = ResolvedUsage.ASYNCHRONOUS;
|
||||
|
||||
// In case there is metadata for the current class, we collect all resolved Angular input
|
||||
// names and add them to the list of known inputs that need to be checked for usages of
|
||||
// the current query. e.g. queries used in an @Input() *setter* are always static.
|
||||
if (classMetadata) {
|
||||
knownInputNames.push(...classMetadata.ngInputNames);
|
||||
}
|
||||
|
||||
// Array of TypeScript nodes which can contain usages of the given query in
|
||||
// order to access it statically.
|
||||
const possibleStaticQueryNodes = filterQueryClassMemberNodes(classDecl, query, knownInputNames);
|
||||
|
||||
// In case nodes that can possibly access a query statically have been found, check
|
||||
// if the query declaration is synchronously used within any of these nodes.
|
||||
if (possibleStaticQueryNodes.length) {
|
||||
possibleStaticQueryNodes.forEach(
|
||||
n => usage = combineResolvedUsage(usage, usageVisitor.getResolvedNodeUsage(n)));
|
||||
}
|
||||
|
||||
if (!classMetadata) {
|
||||
return usage;
|
||||
}
|
||||
|
||||
// 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)) {
|
||||
const template = classMetadata.template;
|
||||
const parsedHtml = parseHtmlGracefully(template.content, template.filePath);
|
||||
const htmlVisitor = new TemplateUsageVisitor(query.property !.name.text);
|
||||
|
||||
if (parsedHtml && htmlVisitor.isQueryUsedStatically(parsedHtml)) {
|
||||
return ResolvedUsage.SYNCHRONOUS;
|
||||
}
|
||||
}
|
||||
|
||||
// In case derived classes should also be analyzed, we determine the classes that derive
|
||||
// from the current class and check if these have input setters or lifecycle hooks that
|
||||
// use the query statically.
|
||||
if (visitInheritedClasses) {
|
||||
classMetadata.derivedClasses.forEach(derivedClass => {
|
||||
usage = combineResolvedUsage(
|
||||
usage, this.analyzeQueryUsage(derivedClass, query, knownInputNames));
|
||||
});
|
||||
}
|
||||
|
||||
// In case the current class has a super class, we determine declared abstract function-like
|
||||
// declarations in the super-class that are implemented in the current class. The super class
|
||||
// will then be analyzed with the abstract declarations mapped to the implemented TypeScript
|
||||
// nodes. This allows us to handle queries which are used in super classes through derived
|
||||
// abstract method declarations.
|
||||
if (classMetadata.superClass) {
|
||||
const superClassDecl = classMetadata.superClass;
|
||||
|
||||
// Update the function context to map abstract declaration nodes to their implementation
|
||||
// node in the base class. This ensures that the declaration usage visitor can analyze
|
||||
// abstract class member declarations.
|
||||
updateSuperClassAbstractMembersContext(classDecl, functionCtx, this.classMetadata);
|
||||
|
||||
usage = combineResolvedUsage(
|
||||
usage, this.analyzeQueryUsage(superClassDecl, query, [], functionCtx, false));
|
||||
}
|
||||
|
||||
return usage;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Checks whether a given query is used statically within the given class, its super
|
||||
* class or derived classes.
|
||||
* Combines two resolved usages based on a fixed priority. "Synchronous" takes
|
||||
* precedence over "Ambiguous" whereas ambiguous takes precedence over "Asynchronous".
|
||||
*/
|
||||
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 classMetadata = classMetadataMap.get(classDecl);
|
||||
|
||||
// In case there is metadata for the current class, we collect all resolved Angular input
|
||||
// names and add them to the list of known inputs that need to be checked for usages of
|
||||
// the current query. e.g. queries used in an @Input() *setter* are always static.
|
||||
if (classMetadata) {
|
||||
knownInputNames.push(...classMetadata.ngInputNames);
|
||||
function combineResolvedUsage(base: ResolvedUsage, target: ResolvedUsage): ResolvedUsage {
|
||||
if (base === ResolvedUsage.SYNCHRONOUS) {
|
||||
return base;
|
||||
} else if (target !== ResolvedUsage.ASYNCHRONOUS) {
|
||||
return target;
|
||||
} else {
|
||||
return ResolvedUsage.ASYNCHRONOUS;
|
||||
}
|
||||
|
||||
// Array of TypeScript nodes which can contain usages of the given query in
|
||||
// order to access it statically.
|
||||
const possibleStaticQueryNodes = filterQueryClassMemberNodes(classDecl, query, knownInputNames);
|
||||
|
||||
// In case nodes that can possibly access a query statically have been found, check
|
||||
// if the query declaration is synchronously used within any of these nodes.
|
||||
if (possibleStaticQueryNodes.length &&
|
||||
possibleStaticQueryNodes.some(n => usageVisitor.isSynchronouslyUsedInNode(n))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!classMetadata) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// 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)) {
|
||||
const template = classMetadata.template;
|
||||
const parsedHtml = parseHtmlGracefully(template.content, template.filePath);
|
||||
const htmlVisitor = new TemplateUsageVisitor(query.property !.name.text);
|
||||
|
||||
if (parsedHtml && htmlVisitor.isQueryUsedStatically(parsedHtml)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// In case derived classes should also be analyzed, we determine the classes that derive
|
||||
// from the current class and check if these have input setters or lifecycle hooks that
|
||||
// use the query statically.
|
||||
if (visitInheritedClasses) {
|
||||
if (classMetadata.derivedClasses.some(
|
||||
derivedClass => isQueryUsedStatically(
|
||||
derivedClass, query, classMetadataMap, typeChecker, knownInputNames))) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// In case the current class has a super class, we determine declared abstract function-like
|
||||
// declarations in the super-class that are implemented in the current class. The super class
|
||||
// will then be analyzed with the abstract declarations mapped to the implemented TypeScript
|
||||
// nodes. This allows us to handle queries which are used in super classes through derived
|
||||
// abstract method declarations.
|
||||
if (classMetadata.superClass) {
|
||||
const superClassDecl = classMetadata.superClass;
|
||||
|
||||
// Update the function context to map abstract declaration nodes to their implementation
|
||||
// node in the base class. This ensures that the declaration usage visitor can analyze
|
||||
// abstract class member declarations.
|
||||
updateSuperClassAbstractMembersContext(classDecl, functionCtx, classMetadataMap);
|
||||
|
||||
if (isQueryUsedStatically(
|
||||
superClassDecl, query, classMetadataMap, typeChecker, [], functionCtx, false)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Filters all class members from the class declaration that can access the
|
||||
* given query statically (e.g. ngOnInit lifecycle hook or @Input setters)
|
||||
|
|
|
|||
|
|
@ -17,7 +17,8 @@ export type TransformedQueryResult = null | {
|
|||
failureMessage: string|null;
|
||||
};
|
||||
|
||||
const TODO_COMMENT = 'TODO: add static flag';
|
||||
const TODO_SPECIFY_COMMENT = 'TODO: add static flag';
|
||||
const TODO_CHECK_COMMENT = 'TODO: check static flag';
|
||||
|
||||
/**
|
||||
* Transforms the given query decorator by explicitly specifying the timing based on the
|
||||
|
|
@ -37,7 +38,9 @@ export function getTransformedQueryCallExpr(
|
|||
// keep the existing options untouched and just add the new property if possible.
|
||||
if (queryArguments.length === 2) {
|
||||
const existingOptions = queryArguments[1];
|
||||
const hasTodoComment = existingOptions.getFullText().includes(TODO_COMMENT);
|
||||
const existingOptionsText = existingOptions.getFullText();
|
||||
const hasTodoComment = existingOptionsText.includes(TODO_SPECIFY_COMMENT) ||
|
||||
existingOptionsText.includes(TODO_CHECK_COMMENT);
|
||||
let newOptionsNode: ts.Expression;
|
||||
let failureMessage: string|null = null;
|
||||
|
||||
|
|
@ -55,7 +58,7 @@ export function getTransformedQueryCallExpr(
|
|||
// 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 && !hasTodoComment) {
|
||||
addQueryTimingTodoToNode(newOptionsNode);
|
||||
addQueryTimingTodoToNode(newOptionsNode, timing === null);
|
||||
}
|
||||
} else {
|
||||
// In case the options query parameter is not an object literal expression, and
|
||||
|
|
@ -63,7 +66,7 @@ export function getTransformedQueryCallExpr(
|
|||
newOptionsNode = existingOptions;
|
||||
// We always want to add a TODO in case the query options cannot be updated.
|
||||
if (!hasTodoComment) {
|
||||
addQueryTimingTodoToNode(existingOptions);
|
||||
addQueryTimingTodoToNode(existingOptions, true);
|
||||
}
|
||||
// If there is a new explicit timing that has been determined for the given query,
|
||||
// we create a transformation failure message that shows developers that they need
|
||||
|
|
@ -85,7 +88,7 @@ export function getTransformedQueryCallExpr(
|
|||
const optionsNode = ts.createObjectLiteral(queryPropertyAssignments);
|
||||
|
||||
if (createTodo) {
|
||||
addQueryTimingTodoToNode(optionsNode);
|
||||
addQueryTimingTodoToNode(optionsNode, timing === null);
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
@ -98,14 +101,15 @@ export function getTransformedQueryCallExpr(
|
|||
|
||||
/**
|
||||
* Adds a to-do to the given TypeScript node which reminds developers to specify
|
||||
* an explicit query timing.
|
||||
* an explicit query timing or to double-check the updated timing.
|
||||
*/
|
||||
function addQueryTimingTodoToNode(node: ts.Node) {
|
||||
ts.setSyntheticLeadingComments(node, [{
|
||||
pos: -1,
|
||||
end: -1,
|
||||
hasTrailingNewLine: false,
|
||||
kind: ts.SyntaxKind.MultiLineCommentTrivia,
|
||||
text: ` ${TODO_COMMENT} `
|
||||
}]);
|
||||
function addQueryTimingTodoToNode(node: ts.Node, addSpecifyTimingTodo: boolean) {
|
||||
ts.setSyntheticLeadingComments(
|
||||
node, [{
|
||||
pos: -1,
|
||||
end: -1,
|
||||
hasTrailingNewLine: false,
|
||||
kind: ts.SyntaxKind.MultiLineCommentTrivia,
|
||||
text: ` ${addSpecifyTimingTodo ? TODO_SPECIFY_COMMENT : TODO_CHECK_COMMENT} `
|
||||
}]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -661,6 +661,19 @@ describe('static-queries migration with usage strategy', () => {
|
|||
});
|
||||
|
||||
it('should not mark queries used in promises as static', async() => {
|
||||
writeFile('/es2015.dom.d.ts', `
|
||||
interface PromiseConstructor {
|
||||
resolve(): Promise;
|
||||
reject(): Promise;
|
||||
}
|
||||
|
||||
interface Promise {
|
||||
then(cb: Function): Promise;
|
||||
catch(cb: Function): Promise;
|
||||
}
|
||||
|
||||
declare var Promise: PromiseConstructor;
|
||||
`);
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
|
||||
|
|
@ -791,6 +804,7 @@ describe('static-queries migration with usage strategy', () => {
|
|||
});
|
||||
|
||||
it('should not mark queries used in setTimeout as static', async() => {
|
||||
writeFile('/lib.dom.d.ts', `declare function setTimeout(cb: Function);`);
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
|
||||
|
|
@ -826,14 +840,19 @@ describe('static-queries migration with usage strategy', () => {
|
|||
});
|
||||
|
||||
it('should not mark queries used in "addEventListener" as static', async() => {
|
||||
writeFile('/lib.dom.d.ts', `
|
||||
interface HTMLElement {
|
||||
addEventListener(cb: Function);
|
||||
}
|
||||
`);
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ElementRef, ${queryType}} from '@angular/core';
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
|
||||
@Component({template: '<span #test></span>'})
|
||||
export class MyComp {
|
||||
private @${queryType}('test') query: any;
|
||||
|
||||
constructor(private elementRef: ElementRef) {}
|
||||
constructor(private elementRef: HTMLElement) {}
|
||||
|
||||
ngOnInit() {
|
||||
this.elementRef.addEventListener(() => {
|
||||
|
|
@ -850,6 +869,7 @@ describe('static-queries migration with usage strategy', () => {
|
|||
});
|
||||
|
||||
it('should not mark queries used in "requestAnimationFrame" as static', async() => {
|
||||
writeFile('/lib.dom.d.ts', `declare function requestAnimationFrame(cb: Function);`);
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ElementRef, ${queryType}} from '@angular/core';
|
||||
|
||||
|
|
@ -1392,6 +1412,75 @@ describe('static-queries migration with usage strategy', () => {
|
|||
.toContain(`@${queryType}('test', { static: true }) query: any;`);
|
||||
});
|
||||
|
||||
it('should mark queries which could be accessed statically within third-party calls as ambiguous',
|
||||
async() => {
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
import {thirdPartySync} from 'my-lib';
|
||||
|
||||
@Component({template: '<span>Template</span>'})
|
||||
export class MyComponent {
|
||||
@${queryType}('test') query: any;
|
||||
@${queryType}('test') query2: any;
|
||||
|
||||
ngOnInit() {
|
||||
const myVarFn = () => this.query2.doSomething();
|
||||
|
||||
thirdPartySync(() => this.query.doSomething());
|
||||
thirdPartySync(myVarFn);
|
||||
}
|
||||
}
|
||||
`);
|
||||
|
||||
writeFile(
|
||||
'/node_modules/my-lib/index.d.ts', `export function thirdPartySync(fn: Function);`);
|
||||
|
||||
await runMigration();
|
||||
|
||||
expect(tree.readContent('/index.ts'))
|
||||
.toContain(
|
||||
`@${queryType}('test', /* TODO: check static flag */ { static: true }) query: any;`);
|
||||
expect(tree.readContent('/index.ts'))
|
||||
.toContain(
|
||||
`@${queryType}('test', /* TODO: check static flag */ { static: true }) query2: any;`);
|
||||
expect(warnOutput.length).toBe(2);
|
||||
expect(warnOutput[0]).toContain('Query timing is ambiguous.');
|
||||
expect(warnOutput[1]).toContain('Query timing is ambiguous.');
|
||||
});
|
||||
|
||||
it('should mark queries which could be accessed statically within third-party new expressions as ambiguous',
|
||||
async() => {
|
||||
writeFile('/index.ts', `
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
import {ThirdParty} from 'my-lib';
|
||||
|
||||
@Component({template: '<span>Template</span>'})
|
||||
export class MyComponent {
|
||||
@${queryType}('test') query: any;
|
||||
|
||||
ngOnInit() {
|
||||
new ThirdParty(() => this.query.doSomething());
|
||||
}
|
||||
}
|
||||
`);
|
||||
|
||||
writeFile('/node_modules/my-lib/index.d.ts', `
|
||||
export declare class ThirdParty {
|
||||
constructor(cb: Function);
|
||||
}
|
||||
`);
|
||||
|
||||
await runMigration();
|
||||
|
||||
expect(tree.readContent('/index.ts'))
|
||||
.toContain(
|
||||
`@${queryType}('test', /* TODO: check static flag */ { static: true }) query: any;`);
|
||||
expect(warnOutput.length).toBe(1);
|
||||
expect(warnOutput[0])
|
||||
.toContain(
|
||||
'Query timing is ambiguous. Please check if the query can be marked as dynamic');
|
||||
});
|
||||
|
||||
it('should properly handle multiple tsconfig files', async() => {
|
||||
writeFile('/src/index.ts', `
|
||||
import {Component, ${queryType}} from '@angular/core';
|
||||
|
|
|
|||
Loading…
Reference in New Issue