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 @@ -54,6 +54,8 @@ export class NgQueryResolveVisitor {
this.visitClassDeclaration(node as ts.ClassDeclaration);
break;
}

ts.forEachChild(node, n => this.visitNode(n));
}

private visitPropertyDeclaration(node: ts.PropertyDeclaration) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript';

import {NgComponentTemplateVisitor} from '../../../utils/ng_component_template';
import {visitAllNodes} from '../../../utils/typescript/visit_nodes';
import {NgQueryResolveVisitor} from '../angular/ng_query_visitor';
import {QueryTiming} from '../angular/query-definition';
import {QueryUsageStrategy} from '../strategies/usage_strategy/usage_strategy';
Expand All @@ -35,10 +34,8 @@ export class Rule extends Rules.TypedRule {

// 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
// traverse through all source files multiple times for each visitor as this could be
// slow.
visitAllNodes(sourceFile, [queryVisitor, templateVisitor]);
queryVisitor.visitNode(sourceFile);
templateVisitor.visitNode(sourceFile);
});

const {resolvedQueries, classMetadata} = queryVisitor;
Expand Down
144 changes: 72 additions & 72 deletions packages/core/schematics/migrations/static-queries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,24 @@ import * as ts from 'typescript';

import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {getInquirer, supportsPrompt} from '../../utils/schematics_prompt';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {TypeScriptVisitor, visitAllNodes} from '../../utils/typescript/visit_nodes';

import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy';
import {QueryTestStrategy} from './strategies/test_strategy/test_strategy';
import {TimingStrategy} from './strategies/timing-strategy';
import {QueryUsageStrategy} from './strategies/usage_strategy/usage_strategy';
import {SELECTED_STRATEGY, promptForMigrationStrategy} from './strategy_prompt';
import {getTransformedQueryCallExpr} from './transform';

export enum SELECTED_STRATEGY {
TEMPLATE,
USAGE,
TESTS,
interface AnalyzedProject {
program: ts.Program;
host: ts.CompilerHost;
queryVisitor: NgQueryResolveVisitor;
sourceFiles: ts.SourceFile[];
basePath: string;
typeChecker: ts.TypeChecker;
tsconfigPath: string;
}

/** Entry point for the V8 static-query migration. */
Expand Down Expand Up @@ -57,99 +60,96 @@ async function runMigration(tree: Tree, context: SchematicContext) {
'to explicit timing.');
}

// In case prompts are supported, determine the desired migration strategy
// by creating a choice prompt. By default the template strategy is used.
let selectedStrategy: SELECTED_STRATEGY = SELECTED_STRATEGY.TEMPLATE;
if (supportsPrompt()) {
logger.info('There are two available migration strategies that can be selected:');
logger.info(' • Template strategy - migration tool (short-term gains, rare corrections)');
logger.info(' • Usage strategy - best practices (long-term gains, manual corrections)');
logger.info('For an easy migration, the template strategy is recommended. The usage');
logger.info('strategy can be used for best practices and a code base that will be more');
logger.info('flexible to changes going forward.');
const {strategyName} = await getInquirer().prompt<{strategyName: string}>({
type: 'list',
name: 'strategyName',
message: 'What migration strategy do you want to use?',
choices: [
{name: 'Template strategy', value: 'template'}, {name: 'Usage strategy', value: 'usage'}
],
default: 'template',
});
logger.info('');
selectedStrategy =
strategyName === 'usage' ? SELECTED_STRATEGY.USAGE : SELECTED_STRATEGY.TEMPLATE;
} else {
// In case prompts are not supported, we still want to allow developers to opt
// into the usage strategy by specifying an environment variable. The tests also
// use the environment variable as there is no headless way to select via prompt.
selectedStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] ? SELECTED_STRATEGY.USAGE :
SELECTED_STRATEGY.TEMPLATE;
}

const buildProjects = new Set<AnalyzedProject>();
const failures = [];

for (const tsconfigPath of buildPaths) {
failures.push(...await runStaticQueryMigration(tree, tsconfigPath, basePath, selectedStrategy));
const project = analyzeProject(tree, tsconfigPath, basePath);
if (project) {
buildProjects.add(project);
}
}

// In case there are projects which contain queries that need to be migrated,
// we want to prompt for the migration strategy and run the migration.
if (buildProjects.size) {
const strategy = await promptForMigrationStrategy(logger);
for (let project of Array.from(buildProjects.values())) {
failures.push(...await runStaticQueryMigration(tree, project, strategy));
}
}

// For the "test" tsconfig projects we always want to use the test strategy as
// we can't detect the proper timing within spec files.
for (const tsconfigPath of testPaths) {
failures.push(
...await runStaticQueryMigration(tree, tsconfigPath, basePath, SELECTED_STRATEGY.TESTS));
const project = await analyzeProject(tree, tsconfigPath, basePath);
if (project) {
failures.push(...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS));
}
}

if (failures.length) {
logger.info('Some queries cannot be migrated automatically. Please go through');
logger.info('those manually and apply the appropriate timing:');
logger.info('Some queries could not be migrated automatically. Please go');
logger.info('through those manually and apply the appropriate timing:');
failures.forEach(failure => logger.warn(`⮑ ${failure}`));
}

logger.info('------------------------------------------------');
}

/**
* Runs the static query migration for the given TypeScript project. The schematic
* analyzes all queries within the project and sets up the query timing based on
* 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".
* Analyzes the given TypeScript project by looking for queries that need to be
* migrated. In case there are no queries that can be migrated, null is returned.
*/
async function runStaticQueryMigration(
tree: Tree, tsconfigPath: string, basePath: string, selectedStrategy: SELECTED_STRATEGY) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = ts.createCompilerHost(parsed.options, true);

// We need to overwrite the host "readFile" method, as we want the TypeScript
// program to be based on the file contents in the virtual file tree. Otherwise
// if we run the migration for multiple tsconfig files which have intersecting
// source files, it can end up updating query definitions multiple times.
host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined;
};
function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
AnalyzedProject|null {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = ts.createCompilerHost(parsed.options, true);

// We need to overwrite the host "readFile" method, as we want the TypeScript
// program to be based on the file contents in the virtual file tree. Otherwise
// if we run the migration for multiple tsconfig files which have intersecting
// source files, it can end up updating query definitions multiple times.
host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined;
};

const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
const queryVisitor = new NgQueryResolveVisitor(typeChecker);

// Analyze all project source-files and collect all queries that
// need to be migrated.
sourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));

if (queryVisitor.resolvedQueries.size === 0) {
return null;
}

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) !);
return {program, host, tsconfigPath, typeChecker, basePath, queryVisitor, sourceFiles};
}

/**
* Runs the static query migration for the given project. The schematic analyzes all
* queries within the project and sets up the query timing based on 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".
*/
async function runStaticQueryMigration(
tree: Tree, project: AnalyzedProject, selectedStrategy: SELECTED_STRATEGY) {
const {sourceFiles, typeChecker, host, queryVisitor, tsconfigPath, basePath} = project;
const printer = ts.createPrinter();
const analysisVisitors: TypeScriptVisitor[] = [queryVisitor];
const failureMessages: string[] = [];
const templateVisitor = new NgComponentTemplateVisitor(typeChecker);

// 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 (selectedStrategy === SELECTED_STRATEGY.USAGE) {
analysisVisitors.push(templateVisitor);
sourceFiles.forEach(s => templateVisitor.visitNode(s));
}

rootSourceFiles.forEach(sourceFile => {
// 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, analysisVisitors);
});

const {resolvedQueries, classMetadata} = queryVisitor;
const {resolvedTemplates} = templateVisitor;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @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 {logging} from '@angular-devkit/core';

import {getInquirer, supportsPrompt} from '../../utils/schematics_prompt';

export enum SELECTED_STRATEGY {
TEMPLATE,
USAGE,
TESTS,
}

/**
* Prompts the user for the migration strategy that should be used. Defaults to the
* template strategy as it provides a migration with rare manual corrections.
* */
export async function promptForMigrationStrategy(logger: logging.LoggerApi) {
if (supportsPrompt()) {
logger.info('There are two available migration strategies that can be selected:');
logger.info(' • Template strategy - migration tool (short-term gains, rare corrections)');
logger.info(' • Usage strategy - best practices (long-term gains, manual corrections)');
logger.info('For an easy migration, the template strategy is recommended. The usage');
logger.info('strategy can be used for best practices and a code base that will be more');
logger.info('flexible to changes going forward.');
const {strategyName} = await getInquirer().prompt<{strategyName: string}>({
type: 'list',
name: 'strategyName',
message: 'What migration strategy do you want to use?',
choices: [
{name: 'Template strategy', value: 'template'}, {name: 'Usage strategy', value: 'usage'}
],
default: 'template',
});
logger.info('');
return strategyName === 'usage' ? SELECTED_STRATEGY.USAGE : SELECTED_STRATEGY.TEMPLATE;
} else {
// In case prompts are not supported, we still want to allow developers to opt
// into the usage strategy by specifying an environment variable. The tests also
// use the environment variable as there is no headless way to select via prompt.
return !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] ? SELECTED_STRATEGY.USAGE :
SELECTED_STRATEGY.TEMPLATE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import * as ts from 'typescript';

import {NgComponentTemplateVisitor} from '../../../utils/ng_component_template';
import {createHtmlSourceFile} from '../../../utils/tslint/tslint_html_source_file';
import {visitAllNodes} from '../../../utils/typescript/visit_nodes';
import {analyzeResolvedTemplate} from '../analyze_template';

const FAILURE_MESSAGE = 'Found assignment to template variable. This does not work with Ivy and ' +
Expand All @@ -27,7 +26,7 @@ export class Rule extends Rules.TypedRule {
const failures: RuleFailure[] = [];

// Analyze the current source files by detecting all referenced HTML templates.
visitAllNodes(sourceFile, [templateVisitor]);
templateVisitor.visitNode(sourceFile);

const {resolvedTemplates} = templateVisitor;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ 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 {analyzeResolvedTemplate} from './analyze_template';

Expand Down Expand Up @@ -64,7 +63,7 @@ function runTemplateVariableAssignmentCheck(
const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);

// Analyze source files by detecting HTML templates.
rootSourceFiles.forEach(sourceFile => visitAllNodes(sourceFile, [templateVisitor]));
rootSourceFiles.forEach(sourceFile => templateVisitor.visitNode(sourceFile));

const {resolvedTemplates} = templateVisitor;
const collectedFailures: string[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1367,5 +1367,22 @@ describe('static-queries migration with usage strategy', () => {
expect(tree.readContent('/src/index.ts'))
.toContain(`@${queryType}('test', { static: false }) query: any;`);
});

it(`should not prompt for migration strategy if no @${queryType} query is used`, async() => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
@Component({template: '<span #test></span>'})
export class NoQueriesDeclared {
}
`);

const testModule = require('../migrations/static-queries/strategy_prompt');
spyOn(testModule, 'promptForMigrationStrategy').and.callThrough();

await runMigration();

expect(testModule.promptForMigrationStrategy).toHaveBeenCalledTimes(0);
});
}
});
2 changes: 2 additions & 0 deletions packages/core/schematics/utils/ng_component_template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export class NgComponentTemplateVisitor {
if (node.kind === ts.SyntaxKind.ClassDeclaration) {
this.visitClassDeclaration(node as ts.ClassDeclaration);
}

ts.forEachChild(node, n => this.visitNode(n));
}

private visitClassDeclaration(node: ts.ClassDeclaration) {
Expand Down
16 changes: 0 additions & 16 deletions packages/core/schematics/utils/typescript/visit_nodes.ts

This file was deleted.