diff --git a/packages/core/schematics/migrations/static-queries/BUILD.bazel b/packages/core/schematics/migrations/static-queries/BUILD.bazel index aa2c77ec83..00da5b6919 100644 --- a/packages/core/schematics/migrations/static-queries/BUILD.bazel +++ b/packages/core/schematics/migrations/static-queries/BUILD.bazel @@ -11,6 +11,7 @@ ts_library( ], deps = [ "//packages/compiler", + "//packages/compiler-cli", "//packages/core/schematics/utils", "@npm//@angular-devkit/schematics", "@npm//@types/node", 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 413d9334bd..929f0fd872 100644 --- a/packages/core/schematics/migrations/static-queries/angular/query-definition.ts +++ b/packages/core/schematics/migrations/static-queries/angular/query-definition.ts @@ -12,7 +12,7 @@ import {NgDecorator} from '../../../utils/ng_decorators'; /** Timing of a given query. Either static or dynamic. */ export enum QueryTiming { STATIC, - DYNAMIC + DYNAMIC, } /** Type of a given query. */ @@ -24,13 +24,10 @@ export enum QueryType { export interface NgQueryDefinition { /** Type of the query definition. */ type: QueryType; - /** Property that declares the query. */ property: ts.PropertyDeclaration; - /** Decorator that declares this as a query. */ decorator: NgDecorator; - /** Class declaration that holds this query. */ container: ts.ClassDeclaration; } diff --git a/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts b/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts index 73b301089d..7e4fd64c86 100644 --- a/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts +++ b/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts @@ -63,8 +63,8 @@ export class Rule extends Rules.TypedRule { // query definitions to explicitly declare the query timing (static or dynamic) queries.forEach(q => { const queryExpr = q.decorator.node.expression; - const timing = usageStrategy.detectTiming(q); - const transformedNode = getTransformedQueryCallExpr(q, timing); + const {timing, message} = usageStrategy.detectTiming(q); + const transformedNode = getTransformedQueryCallExpr(q, timing, !!message); if (!transformedNode) { return; diff --git a/packages/core/schematics/migrations/static-queries/index.ts b/packages/core/schematics/migrations/static-queries/index.ts index b8b91ddbbc..cabccfa242 100644 --- a/packages/core/schematics/migrations/static-queries/index.ts +++ b/packages/core/schematics/migrations/static-queries/index.ts @@ -6,24 +6,27 @@ * found in the LICENSE file at https://angular.io/license */ -import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics'; +import {logging} from '@angular-devkit/core'; +import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics'; import {dirname, relative} from 'path'; import * as ts from 'typescript'; import {NgComponentTemplateVisitor} from '../../utils/ng_component_template'; import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; -import {visitAllNodes} from '../../utils/typescript/visit_nodes'; +import {TypeScriptVisitor, visitAllNodes} from '../../utils/typescript/visit_nodes'; import {NgQueryResolveVisitor} from './angular/ng_query_visitor'; +import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy'; +import {TimingStrategy} from './strategies/timing-strategy'; import {QueryUsageStrategy} from './strategies/usage_strategy/usage_strategy'; import {getTransformedQueryCallExpr} from './transform'; - +type Logger = logging.LoggerApi; /** Entry point for the V8 static-query migration. */ export default function(): Rule { - return (tree: Tree) => { + return (tree: Tree, context: SchematicContext) => { const projectTsConfigPaths = getProjectTsConfigPaths(tree); const basePath = process.cwd(); @@ -34,7 +37,7 @@ export default function(): Rule { } for (const tsconfigPath of projectTsConfigPaths) { - runStaticQueryMigration(tree, tsconfigPath, basePath); + runStaticQueryMigration(tree, tsconfigPath, basePath, context.logger); } }; } @@ -45,7 +48,8 @@ export default function(): Rule { * the current usage of the query property. e.g. a view query that is not used in any * lifecycle hook does not need to be static and can be set up with "static: false". */ -function runStaticQueryMigration(tree: Tree, tsconfigPath: string, basePath: string) { +function runStaticQueryMigration( + tree: Tree, tsconfigPath: string, basePath: string, logger: Logger) { const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath)); const host = ts.createCompilerHost(parsed.options, true); @@ -58,46 +62,67 @@ function runStaticQueryMigration(tree: Tree, tsconfigPath: string, basePath: str return buffer ? buffer.toString() : undefined; }; + const isUsageStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY']; const program = ts.createProgram(parsed.fileNames, parsed.options, host); const typeChecker = program.getTypeChecker(); const queryVisitor = new NgQueryResolveVisitor(typeChecker); const templateVisitor = new NgComponentTemplateVisitor(typeChecker); const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !); const printer = ts.createPrinter(); + const analysisVisitors: TypeScriptVisitor[] = [queryVisitor]; + + // If the "usage" strategy is selected, we also need to add the query visitor + // to the analysis visitors so that query usage in templates can be also checked. + if (isUsageStrategy) { + analysisVisitors.push(templateVisitor); + } - // Analyze source files by detecting queries, class relations and component templates. rootSourceFiles.forEach(sourceFile => { - // The visit utility function only traverses the source file once. We don't want to + // The visit utility function only traverses a source file once. We don't want to // traverse through all source files multiple times for each visitor as this could be // slow. - visitAllNodes(sourceFile, [queryVisitor, templateVisitor]); + visitAllNodes(sourceFile, analysisVisitors); }); const {resolvedQueries, classMetadata} = queryVisitor; + const {resolvedTemplates} = templateVisitor; - // Add all resolved templates to the class metadata so that we can also - // check component templates for static query usage. - templateVisitor.resolvedTemplates.forEach(template => { - if (classMetadata.has(template.container)) { - classMetadata.get(template.container) !.template = template; - } - }); + if (isUsageStrategy) { + // Add all resolved templates to the class metadata if the usage strategy is used. This + // is necessary in order to be able to check component templates for static query usage. + resolvedTemplates.forEach(template => { + if (classMetadata.has(template.container)) { + classMetadata.get(template.container) !.template = template; + } + }); + } - const usageStrategy = new QueryUsageStrategy(classMetadata, typeChecker); + const strategy: TimingStrategy = isUsageStrategy ? + new QueryUsageStrategy(classMetadata, typeChecker) : + new QueryTemplateStrategy(tsconfigPath, classMetadata, host); + const detectionMessages: string[] = []; + + // In case the strategy could not be set up properly, we just exit the + // migration. We don't want to throw an exception as this could mean + // that other migrations are interrupted. + if (!strategy.setup()) { + return; + } // Walk through all source files that contain resolved queries and update // the source files if needed. Note that we need to update multiple queries // within a source file within the same recorder in order to not throw off // the TypeScript node offsets. resolvedQueries.forEach((queries, sourceFile) => { - const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); + const relativePath = relative(basePath, sourceFile.fileName); + const update = tree.beginUpdate(relativePath); - // Compute the query usage for all resolved queries and update the - // query definitions to explicitly declare the query timing (static or dynamic) + // Compute the query timing for all resolved queries and update the + // query definitions to explicitly set the determined query timing. queries.forEach(q => { const queryExpr = q.decorator.node.expression; - const timing = usageStrategy.detectTiming(q); - const transformedNode = getTransformedQueryCallExpr(q, timing); + const {timing, message} = strategy.detectTiming(q); + const transformedNode = getTransformedQueryCallExpr(q, timing, !!message); if (!transformedNode) { return; @@ -109,8 +134,24 @@ function runStaticQueryMigration(tree: Tree, tsconfigPath: string, basePath: str // call expression node. update.remove(queryExpr.getStart(), queryExpr.getWidth()); update.insertRight(queryExpr.getStart(), newText); + + const {line, character} = + ts.getLineAndCharacterOfPosition(sourceFile, q.decorator.node.getStart()); + detectionMessages.push(`${relativePath}@${line + 1}:${character + 1}: ${message}`); }); tree.commitUpdate(update); }); + + if (detectionMessages.length) { + logger.info('------ Static Query migration ------'); + logger.info('In preparation for Ivy, developers can now explicitly specify the'); + logger.info('timing of their queries. Read more about this here:'); + logger.info('https://github.com/angular/angular/pull/28810'); + logger.info(''); + logger.info('Some queries cannot be migrated automatically. Please go through'); + logger.info('those manually and apply the appropriate timing:'); + detectionMessages.forEach(failure => logger.warn(`⮑ ${failure}`)); + logger.info('------------------------------------------------'); + } } 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 new file mode 100644 index 0000000000..1c61347362 --- /dev/null +++ b/packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts @@ -0,0 +1,178 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {AotCompiler, CompileDirectiveMetadata, CompileMetadataResolver, CompileNgModuleMetadata, NgAnalyzedModules, StaticSymbol, TemplateAst, findStaticQueryIds, staticViewQueryIds} from '@angular/compiler'; +import {Diagnostic, createProgram, readConfiguration} from '@angular/compiler-cli'; +import {resolve} from 'path'; +import * as ts from 'typescript'; + +import {hasPropertyNameText} from '../../../../utils/typescript/property_name'; +import {ClassMetadataMap} from '../../angular/ng_query_visitor'; +import {NgQueryDefinition, QueryTiming, QueryType} from '../../angular/query-definition'; +import {TimingResult, TimingStrategy} from '../timing-strategy'; + +const QUERY_NOT_DECLARED_IN_COMPONENT_MESSAGE = 'Timing could not be determined. This happens ' + + 'if the query is not declared in any component.'; + +export class QueryTemplateStrategy implements TimingStrategy { + private compiler: AotCompiler|null = null; + private metadataResolver: CompileMetadataResolver|null = null; + private analyzedQueries = new Map(); + + constructor( + private projectPath: string, private classMetadata: ClassMetadataMap, + private host: ts.CompilerHost) {} + + /** + * Sets up the template strategy by creating the AngularCompilerProgram. Returns false if + * the AOT compiler program could not be created due to failure diagnostics. + */ + setup() { + const {rootNames, options} = readConfiguration(this.projectPath); + const aotProgram = createProgram({rootNames, options, host: this.host}); + + // The "AngularCompilerProgram" does not expose the "AotCompiler" instance, nor does it + // expose the logic that is necessary to analyze the determined modules. We work around + // this by just accessing the necessary private properties using the bracket notation. + this.compiler = (aotProgram as any)['compiler']; + this.metadataResolver = this.compiler !['_metadataResolver']; + const analyzedModules = (aotProgram as any)['analyzedModules'] as NgAnalyzedModules; + + const ngDiagnostics = [ + ...aotProgram.getNgStructuralDiagnostics(), + ...aotProgram.getNgSemanticDiagnostics(), + ]; + + if (ngDiagnostics.length) { + this._printDiagnosticFailures(ngDiagnostics); + return false; + } + + analyzedModules.files.forEach(file => { + file.directives.forEach(directive => this._analyzeDirective(directive, analyzedModules)); + }); + return true; + } + + /** Analyzes a given directive by determining the timing of all matched view queries. */ + private _analyzeDirective(symbol: StaticSymbol, analyzedModules: NgAnalyzedModules) { + const metadata = this.metadataResolver !.getDirectiveMetadata(symbol); + const ngModule = analyzedModules.ngModuleByPipeOrDirective.get(symbol); + + if (!metadata.isComponent || !ngModule) { + return; + } + + const parsedTemplate = this._parseTemplate(metadata, ngModule); + const queryTimingMap = findStaticQueryIds(parsedTemplate); + const {staticQueryIds} = staticViewQueryIds(queryTimingMap); + + metadata.viewQueries.forEach((query, index) => { + // Query ids are computed by adding "one" to the index. This is done within + // the "view_compiler.ts" in order to support using a bloom filter for queries. + const queryId = index + 1; + const queryKey = + this._getViewQueryUniqueKey(symbol.filePath, symbol.name, query.propertyName); + this.analyzedQueries.set( + queryKey, staticQueryIds.has(queryId) ? QueryTiming.STATIC : QueryTiming.DYNAMIC); + }); + } + + /** Detects the timing of the query definition. */ + 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)) { + // 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 classMetadata = this.classMetadata.get(query.container); + + // In case there is no class metadata or there are no derived classes that + // could access the current query, we just look for the query analysis of + // the class that declares the query. e.g. only the template of the class + // that declares the view query affects the query timing. + if (!classMetadata || !classMetadata.derivedClasses.length) { + const timing = this._getQueryTimingFromClass(query.container, propertyName); + + if (timing === null) { + return {timing: null, message: QUERY_NOT_DECLARED_IN_COMPONENT_MESSAGE}; + } + + return {timing}; + } + + let resolvedTiming: QueryTiming|null = null; + let timingMismatch = false; + + // In case there are multiple components that use the same query (e.g. through inheritance), + // we need to check if all components use the query with the same timing. If that is not + // the case, the query timing is ambiguous and the developer needs to fix the query manually. + [query.container, ...classMetadata.derivedClasses].forEach(classDecl => { + const classTiming = this._getQueryTimingFromClass(classDecl, propertyName); + + if (classTiming === null) { + return; + } + + // In case there is no resolved timing yet, save the new timing. Timings from other + // components that use the query with a different timing, cause the timing to be + // mismatched. In that case we can't detect a working timing for all components. + if (resolvedTiming === null) { + resolvedTiming = classTiming; + } else if (resolvedTiming !== classTiming) { + timingMismatch = true; + } + }); + + if (resolvedTiming === null) { + return {timing: QueryTiming.DYNAMIC, message: QUERY_NOT_DECLARED_IN_COMPONENT_MESSAGE}; + } else if (timingMismatch) { + return {timing: null, message: 'Multiple components use the query with different timings.'}; + } + return {timing: resolvedTiming}; + } + + /** + * Gets the timing that has been resolved for a given query when it's used within the + * specified class declaration. e.g. queries from an inherited class can be used. + */ + private _getQueryTimingFromClass(classDecl: ts.ClassDeclaration, queryName: string): QueryTiming + |null { + if (!classDecl.name) { + return null; + } + const filePath = classDecl.getSourceFile().fileName; + const queryKey = this._getViewQueryUniqueKey(filePath, classDecl.name.text, queryName); + + if (this.analyzedQueries.has(queryKey)) { + return this.analyzedQueries.get(queryKey) !; + } + return null; + } + + private _parseTemplate(component: CompileDirectiveMetadata, ngModule: CompileNgModuleMetadata): + TemplateAst[] { + return this + .compiler !['_parseTemplate'](component, ngModule, ngModule.transitiveModule.directives) + .template; + } + + private _printDiagnosticFailures(diagnostics: (ts.Diagnostic|Diagnostic)[]) { + console.error('Could not create Angular AOT compiler to determine query timing.'); + console.error('The following diagnostics were detected:\n'); + console.error(diagnostics.map(d => d.messageText).join(`\n`)); + } + + private _getViewQueryUniqueKey(filePath: string, className: string, propName: string) { + return `${resolve(filePath)}#${className}-${propName}`; + } +} diff --git a/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts b/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts new file mode 100644 index 0000000000..404c0aa167 --- /dev/null +++ b/packages/core/schematics/migrations/static-queries/strategies/timing-strategy.ts @@ -0,0 +1,20 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {NgQueryDefinition, QueryTiming} from '../angular/query-definition'; + +export interface TimingStrategy { + /** Sets up the given strategy. Should return false if the strategy could not be set up. */ + setup(): boolean; + /** Detects the timing result for a given query. */ + detectTiming(query: NgQueryDefinition): TimingResult; +} + +export type TimingResult = { + timing: QueryTiming | null; message?: string; +}; 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 3aa4801d2a..75d31337c9 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 @@ -12,7 +12,7 @@ import {parseHtmlGracefully} from '../../../../utils/parse_html'; import {hasPropertyNameText} from '../../../../utils/typescript/property_name'; import {ClassMetadataMap} from '../../angular/ng_query_visitor'; import {NgQueryDefinition, QueryTiming, QueryType} from '../../angular/query-definition'; -import {TimingStrategy} from '../../timing-strategy'; +import {TimingResult, TimingStrategy} from '../timing-strategy'; import {DeclarationUsageVisitor, FunctionContext} from './declaration_usage_visitor'; import {updateSuperClassAbstractMembersContext} from './super_class_context'; @@ -37,14 +37,23 @@ const STATIC_QUERY_LIFECYCLE_HOOKS = { export class QueryUsageStrategy implements TimingStrategy { constructor(private classMetadata: ClassMetadataMap, private typeChecker: ts.TypeChecker) {} + setup() { + // No setup is needed for this strategy and therefore we always return "true" as + // the setup is successful. + return true; + } + /** * Analyzes the usage of the given query and determines the query timing based * on the current usage of the query. */ - detectTiming(query: NgQueryDefinition): QueryTiming { - return isQueryUsedStatically(query.container, query, this.classMetadata, this.typeChecker, []) ? - QueryTiming.STATIC : - QueryTiming.DYNAMIC; + detectTiming(query: NgQueryDefinition): TimingResult { + return { + timing: + isQueryUsedStatically(query.container, query, this.classMetadata, this.typeChecker, []) ? + QueryTiming.STATIC : + QueryTiming.DYNAMIC + }; } } diff --git a/packages/core/schematics/migrations/static-queries/timing-strategy.ts b/packages/core/schematics/migrations/static-queries/timing-strategy.ts deleted file mode 100644 index c8af666961..0000000000 --- a/packages/core/schematics/migrations/static-queries/timing-strategy.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {NgQueryDefinition, QueryTiming} from './angular/query-definition'; - -export interface TimingStrategy { detectTiming(query: NgQueryDefinition): QueryTiming; } diff --git a/packages/core/schematics/migrations/static-queries/transform.ts b/packages/core/schematics/migrations/static-queries/transform.ts index 1a4b37a575..7863215946 100644 --- a/packages/core/schematics/migrations/static-queries/transform.ts +++ b/packages/core/schematics/migrations/static-queries/transform.ts @@ -15,11 +15,14 @@ import {NgQueryDefinition, QueryTiming} from './angular/query-definition'; * determined timing. The updated decorator call expression node will be returned. */ export function getTransformedQueryCallExpr( - query: NgQueryDefinition, timing: QueryTiming): ts.CallExpression|null { + query: NgQueryDefinition, timing: QueryTiming | null, createTodo: boolean): ts.CallExpression| + null { const queryExpr = query.decorator.node.expression; const queryArguments = queryExpr.arguments; - const timingPropertyAssignment = ts.createPropertyAssignment( - 'static', timing === QueryTiming.STATIC ? ts.createTrue() : ts.createFalse()); + const queryPropertyAssignments = timing === null ? + [] : + [ts.createPropertyAssignment( + 'static', timing === QueryTiming.STATIC ? ts.createTrue() : ts.createFalse())]; // If the query decorator is already called with two arguments, we need to // keep the existing options untouched and just add the new property if needed. @@ -34,13 +37,37 @@ export function getTransformedQueryCallExpr( } const updatedOptions = ts.updateObjectLiteral( - existingOptions, existingOptions.properties.concat(timingPropertyAssignment)); + existingOptions, existingOptions.properties.concat(queryPropertyAssignments)); + + if (createTodo) { + addQueryTimingTodoToNode(updatedOptions); + } + return ts.updateCall( queryExpr, queryExpr.expression, queryExpr.typeArguments, [queryArguments[0], updatedOptions]); } + const optionsNode = ts.createObjectLiteral(queryPropertyAssignments); + + if (createTodo) { + addQueryTimingTodoToNode(optionsNode); + } + return ts.updateCall( - queryExpr, queryExpr.expression, queryExpr.typeArguments, - [queryArguments[0], ts.createObjectLiteral([timingPropertyAssignment])]); + queryExpr, queryExpr.expression, queryExpr.typeArguments, [queryArguments[0], optionsNode]); +} + +/** + * Adds a to-do to the given TypeScript node which reminds developers to specify + * an explicit query timing. + */ +function addQueryTimingTodoToNode(node: ts.Node) { + ts.setSyntheticLeadingComments(node, [{ + pos: -1, + end: -1, + hasTrailingNewLine: false, + kind: ts.SyntaxKind.MultiLineCommentTrivia, + text: ' TODO: add static flag ' + }]); } diff --git a/packages/core/schematics/test/static_queries_migration_template_spec.ts b/packages/core/schematics/test/static_queries_migration_template_spec.ts new file mode 100644 index 0000000000..5345f3c1f7 --- /dev/null +++ b/packages/core/schematics/test/static_queries_migration_template_spec.ts @@ -0,0 +1,490 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {getSystemPath, normalize, virtualFs} from '@angular-devkit/core'; +import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing'; +import {HostTree} from '@angular-devkit/schematics'; +import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing'; +import * as shx from 'shelljs'; + +describe('static-queries migration with template strategy', () => { + let runner: SchematicTestRunner; + let host: TempScopedNodeJsSyncHost; + let tree: UnitTestTree; + let tmpDirPath: string; + let previousWorkingDir: string; + let warnOutput: string[]; + + beforeEach(() => { + runner = new SchematicTestRunner('test', require.resolve('../migrations.json')); + host = new TempScopedNodeJsSyncHost(); + tree = new UnitTestTree(new HostTree(host)); + + writeFile('/tsconfig.json', JSON.stringify({ + compilerOptions: { + experimentalDecorators: true, + lib: ['es2015'], + } + })); + + warnOutput = []; + runner.logger.subscribe(logEntry => { + if (logEntry.level === 'warn') { + warnOutput.push(logEntry.message); + } + }); + + previousWorkingDir = shx.pwd(); + tmpDirPath = getSystemPath(host.root); + + // Switch into the temporary directory path. This allows us to run + // the schematic against our custom unit test tree. + shx.cd(tmpDirPath); + + writeFakeAngular(); + }); + + afterEach(() => { + shx.cd(previousWorkingDir); + shx.rm('-r', tmpDirPath); + }); + + function writeFakeAngular() { writeFile('/node_modules/@angular/core/index.d.ts', ``); } + + function writeFakeLibrary(selectorName = 'my-lib-selector') { + writeFile('/node_modules/my-lib/index.d.ts', `export * from './public-api';`); + writeFile('/node_modules/my-lib/public-api.d.ts', `export declare class MyLibComponent {}`); + writeFile('/node_modules/my-lib/index.metadata.json', JSON.stringify({ + __symbolic: 'module', + version: 4, + metadata: { + MyLibComponent: { + __symbolic: 'class', + decorators: [{ + __symbolic: 'call', + expression: { + __symbolic: 'reference', + module: '@angular/core', + name: 'Component', + line: 0, + character: 0 + }, + arguments: [{ + selector: selectorName, + template: `My Lib Component`, + }] + }], + members: {} + }, + }, + origins: { + MyLibComponent: './public-api', + }, + importAs: 'my-lib', + })); + } + + function writeFile(filePath: string, contents: string) { + host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); + } + + function runMigration() { runner.runSchematic('migration-v8-static-queries', {}, tree); } + + describe('ViewChild', () => { + + it('should detect queries selecting elements through template reference', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + @Component({template: \` + + + +
+ +
+ \`}) + export class MyComp { + private @ViewChild('myButton') query: any; + private @ViewChild('myStaticButton') query2: any; + } + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myButton', { static: false }) query: any;`); + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myStaticButton', { static: true }) query2: any;`); + }); + + it('should detect queries selecting ng-template as static', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + @Component({template: \` + + My template + + \`}) + export class MyComp { + private @ViewChild('myTmpl') query: any; + } + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myTmpl', { static: true }) query: any;`); + }); + + it('should detect queries selecting component view providers through string token', () => { + writeFile('/index.ts', ` + import {Component, Directive, NgModule, ViewChild} from '@angular/core'; + + @Directive({ + selector: '[myDirective]', + providers: [ + {provide: 'my-token', useValue: 'test'} + ] + }) + export class MyDirective {} + + @Directive({ + selector: '[myDirective2]', + providers: [ + {provide: 'my-token-2', useValue: 'test'} + ] + }) + export class MyDirective2 {} + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp { + private @ViewChild('my-token') query: any; + private @ViewChild('my-token-2') query2: any; + } + + @NgModule({declarations: [MyComp, MyDirective, MyDirective2]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + + + + + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('my-token', { static: true }) query: any;`); + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('my-token-2', { static: false }) query2: any;`); + }); + + it('should detect queries selecting component view providers using class token', () => { + writeFile('/index.ts', ` + import {Component, Directive, NgModule, ViewChild} from '@angular/core'; + + export class MyService {} + export class MyService2 {} + + @Directive({ + selector: '[myDirective]', + providers: [MyService] + }) + export class MyDirective {} + + @Directive({ + selector: '[myDirective2]', + providers: [MyService2] + }) + export class MyDirective2 {} + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp { + private @ViewChild(MyService) query: any; + private @ViewChild(MyService2) query2: any; + } + + @NgModule({declarations: [MyComp, MyDirective, MyDirective2]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + + + + + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(MyService, { static: true }) query: any;`); + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(MyService2, { static: false }) query2: any;`); + }); + + it('should detect queries selecting component', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + import {HomeComponent, HomeComponent2} from './home-comp'; + + @Component({ + template: \` + + + + + \` + }) + export class MyComp { + private @ViewChild(HomeComponent) query: any; + private @ViewChild(HomeComponent2) query2: any; + } + + @NgModule({declarations: [MyComp, HomeComponent, HomeComponent2]}) + export class MyModule {} + `); + + writeFile(`/home-comp.ts`, ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'home-comp', + template: 'Home' + }) + export class HomeComponent {} + + @Component({ + selector: 'home-comp2', + template: 'Home 2' + }) + export class HomeComponent2 {} + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(HomeComponent, { static: true }) query: any;`); + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(HomeComponent2, { static: false }) query2: any;`); + }); + + it('should detect queries selecting third-party component', () => { + writeFakeLibrary(); + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + import {MyLibComponent} from 'my-lib'; + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp { + private @ViewChild(MyLibComponent) query: any; + } + + @NgModule({declarations: [MyComp, MyLibComponent]}) + export class MyModule {} + `); + + writeFile('/my-tmpl.html', ` + My projected content + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(MyLibComponent, { static: true }) query: any;`); + }); + + it('should detect queries selecting third-party component with multiple selectors', () => { + writeFakeLibrary('a-selector, test-selector'); + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + import {MyLibComponent} from 'my-lib'; + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp { + private @ViewChild(MyLibComponent) query: any; + } + + @NgModule({declarations: [MyComp, MyLibComponent]}) + export class MyModule {} + `); + + writeFile('/my-tmpl.html', ` + Match 1 + + Match 2 + + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild(MyLibComponent, { static: false }) query: any;`); + }); + + it('should detect queries within structural directive', () => { + writeFile('/index.ts', ` + import {Component, Directive, NgModule, ViewChild} from '@angular/core'; + + @Directive({selector: '[ngIf]'}) + export class FakeNgIf {} + + @Component({templateUrl: 'my-tmpl.html'}) + export class MyComp { + private @ViewChild('myRef') query: any; + private @ViewChild('myRef2') query2: any; + } + + @NgModule({declarations: [MyComp, FakeNgIf]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + No asterisk + With asterisk + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myRef', { static: true }) query: any;`); + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myRef2', { static: false }) query2: any;`); + }); + + it('should detect inherited queries', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + export class BaseClass { + @ViewChild('myRef') query: any; + } + + @Component({templateUrl: 'my-tmpl.html'}) + export class MyComp extends BaseClass {} + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + writeFile(`/my-tmpl.html`, ` + My Ref + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myRef', { static: true }) query: any;`); + }); + + it('should add a todo if a query is not declared in any component', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild, SomeToken} from '@angular/core'; + + export class NotAComponent { + @ViewChild('myRef', {read: SomeToken}) query: any; + } + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain( + `@ViewChild('myRef', /* TODO: add static flag */ { read: SomeToken }) query: any;`); + expect(warnOutput.length).toBe(1); + expect(warnOutput[0]) + .toMatch( + /^⮑ {3}index.ts@5:11:.+could not be determined.+not declared in any component/); + }); + + it('should add a todo if a query is used multiple times with different timing', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ViewChild} from '@angular/core'; + + export class BaseClass { + @ViewChild('myRef') query: any; + } + + @Component({template: '

'}) + export class FirstComp extends BaseClass {} + + @Component({template: ''}) + export class SecondComp extends BaseClass {} + + @NgModule({declarations: [FirstComp, SecondComp]}) + export class MyModule {} + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ViewChild('myRef', /* TODO: add static flag */ {}) query: any;`); + expect(warnOutput.length).toBe(1); + expect(warnOutput[0]) + .toMatch( + /^⮑ {3}index.ts@5:11: Multiple components use the query with different timings./); + }); + + it('should gracefully exit migration if queries could not be analyzed', () => { + writeFile('/index.ts', ` + import {Component, ViewChild} from '@angular/core'; + + @Component({template: '

'}) + export class MyComp { + @ViewChild('myRef') query: any; + } + + // **NOTE**: Analysis will fail as there is no "NgModule" that declares the component. + `); + + spyOn(console, 'error'); + + // We don't expect an error to be thrown as this could interrupt other + // migrations which are scheduled with "ng update" in the CLI. + expect(() => runMigration()).not.toThrow(); + + expect(console.error) + .toHaveBeenCalledWith('Could not create Angular AOT compiler to determine query timing.'); + expect(console.error) + .toHaveBeenCalledWith( + jasmine.stringMatching(/Cannot determine the module for class MyComp/)); + }); + + it('should add a todo for content queries which are not detectable', () => { + writeFile('/index.ts', ` + import {Component, NgModule, ContentChild} from '@angular/core'; + + @Component({template: '

'}) + export class MyComp { + @ContentChild('myRef') query: any; + } + + @NgModule({declarations: [MyComp]}) + export class MyModule {} + `); + + runMigration(); + + expect(tree.readContent('/index.ts')) + .toContain(`@ContentChild('myRef', /* TODO: add static flag */ {}) query: any;`); + expect(warnOutput.length).toBe(1); + expect(warnOutput[0]) + .toMatch(/^⮑ {3}index.ts@6:11: Content queries cannot be migrated automatically\./); + }); + }); +}); diff --git a/packages/core/schematics/test/static_queries_migration_spec.ts b/packages/core/schematics/test/static_queries_migration_usage_spec.ts similarity index 98% rename from packages/core/schematics/test/static_queries_migration_spec.ts rename to packages/core/schematics/test/static_queries_migration_usage_spec.ts index bbc12f92c4..025618e949 100644 --- a/packages/core/schematics/test/static_queries_migration_spec.ts +++ b/packages/core/schematics/test/static_queries_migration_usage_spec.ts @@ -12,13 +12,19 @@ import {HostTree} from '@angular-devkit/schematics'; import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing'; import * as shx from 'shelljs'; -describe('static-queries migration', () => { +describe('static-queries migration with usage strategy', () => { let runner: SchematicTestRunner; let host: TempScopedNodeJsSyncHost; let tree: UnitTestTree; let tmpDirPath: string; let previousWorkingDir: 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 + // way to pass options to the migration without using environment variables. + beforeAll(() => process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] = 'true'); + afterAll(() => process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] = ''); + beforeEach(() => { runner = new SchematicTestRunner('test', require.resolve('../migrations.json')); host = new TempScopedNodeJsSyncHost(); diff --git a/packages/core/schematics/tsconfig.json b/packages/core/schematics/tsconfig.json index 2fccbe4bea..fc07f36196 100644 --- a/packages/core/schematics/tsconfig.json +++ b/packages/core/schematics/tsconfig.json @@ -10,7 +10,9 @@ "baseUrl": ".", "paths": { "@angular/compiler": ["../../compiler"], - "@angular/compiler/*": ["../../compiler/*"] + "@angular/compiler/*": ["../../compiler/*"], + "@angular/compiler-cli": ["../../compiler-cli"], + "@angular/compiler-cli/*": ["../../compiler-cli/*"] } }, "bazelOptions": {