Skip to content

Commit 0cdf598

Browse files
devversionalxhub
authored andcommitted
fix(core): static-query migration should not fallback to test strategy (#30458)
Currently if something fails in the selected strategy (e.g. AOT failures), the migration currently accidentally falls back to the test strategy. This is not helpful as we want to give developers the possibility to re-run the migration after fixing potential AOT failures. PR Close #30458
1 parent 6ceb903 commit 0cdf598

2 files changed

Lines changed: 52 additions & 9 deletions

File tree

‎packages/core/schematics/migrations/static-queries/index.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,12 @@ async function runMigration(tree: Tree, context: SchematicContext) {
6161
'to explicit timing.');
6262
}
6363

64+
const analyzedFiles = new Set<string>();
6465
const buildProjects = new Set<AnalyzedProject>();
6566
const failures = [];
6667

6768
for (const tsconfigPath of buildPaths) {
68-
const project = analyzeProject(tree, tsconfigPath, basePath);
69+
const project = analyzeProject(tree, tsconfigPath, basePath, analyzedFiles);
6970
if (project) {
7071
buildProjects.add(project);
7172
}
@@ -83,7 +84,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
8384
// For the "test" tsconfig projects we always want to use the test strategy as
8485
// we can't detect the proper timing within spec files.
8586
for (const tsconfigPath of testPaths) {
86-
const project = await analyzeProject(tree, tsconfigPath, basePath);
87+
const project = await analyzeProject(tree, tsconfigPath, basePath, analyzedFiles);
8788
if (project) {
8889
failures.push(
8990
...await runStaticQueryMigration(tree, project, SELECTED_STRATEGY.TESTS, logger));
@@ -103,7 +104,8 @@ async function runMigration(tree: Tree, context: SchematicContext) {
103104
* Analyzes the given TypeScript project by looking for queries that need to be
104105
* migrated. In case there are no queries that can be migrated, null is returned.
105106
*/
106-
function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
107+
function analyzeProject(
108+
tree: Tree, tsconfigPath: string, basePath: string, analyzedFiles: Set<string>):
107109
AnalyzedProject|null {
108110
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
109111
const host = ts.createCompilerHost(parsed.options, true);
@@ -125,7 +127,16 @@ function analyzeProject(tree: Tree, tsconfigPath: string, basePath: string):
125127

126128
// Analyze all project source-files and collect all queries that
127129
// need to be migrated.
128-
sourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));
130+
sourceFiles.forEach(sourceFile => {
131+
const relativePath = relative(basePath, sourceFile.fileName);
132+
133+
// Only look for queries within the current source files if the
134+
// file has not been analyzed before.
135+
if (!analyzedFiles.has(relativePath)) {
136+
analyzedFiles.add(relativePath);
137+
queryVisitor.visitNode(sourceFile);
138+
}
139+
});
129140

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

‎packages/core/schematics/test/static_queries_migration_template_spec.ts‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,6 @@ describe('static-queries migration with template strategy', () => {
515515
await runMigration();
516516

517517
expect(errorOutput.length).toBe(1);
518-
expect(errorOutput[0])
519-
.toMatch(/^Error: Could not create Angular AOT compiler to determine query timing./);
520518
expect(errorOutput[0]).toMatch(/Cannot determine the module for class MyComp/);
521519
});
522520

@@ -673,15 +671,49 @@ describe('static-queries migration with template strategy', () => {
673671
export class MyModule {}
674672
`);
675673

676-
spyOn(console, 'error').and.callThrough();
677-
678674
await runMigration();
679675

680-
expect(console.error).toHaveBeenCalledTimes(0);
676+
expect(errorOutput.length).toBe(0);
681677
expect(tree.readContent('/src/test.ts'))
682678
.toContain(`@ViewChild('test', /* TODO: add static flag */ {}) query: any;`);
683679
expect(tree.readContent('/src/app.component.ts'))
684680
.toContain(`@ViewChild('test', { static: true }) query: any;`);
685681
});
682+
683+
it('should not fall back to test strategy if selected strategy fails', async() => {
684+
writeFile('/src/tsconfig.spec.json', JSON.stringify({
685+
compilerOptions: {
686+
experimentalDecorators: true,
687+
lib: ['es2015'],
688+
},
689+
files: [
690+
'test.ts',
691+
],
692+
}));
693+
694+
writeFile('/src/test.ts', `import * as mod from './app.module';`);
695+
writeFile('/src/app.component.ts', `
696+
import {Component, ViewChild} from '@angular/core';
697+
698+
@Component({template: '<span #test>Test</span>'})
699+
export class AppComponent {
700+
@ViewChild('test') query: any;
701+
}
702+
`);
703+
704+
writeFile('/src/app.module.ts', `
705+
import {NgModule} from '@angular/core';
706+
import {AppComponent} from './app.component';
707+
708+
@NgModule({declarations: [AppComponent, ThisCausesAnError]})
709+
export class MyModule {}
710+
`);
711+
712+
await runMigration();
713+
714+
expect(errorOutput.length).toBe(1);
715+
expect(errorOutput[0]).toMatch(/Unexpected value 'undefined'/);
716+
expect(tree.readContent('/src/app.component.ts')).toContain(`@ViewChild('test') query: any;`);
717+
});
686718
});
687719
});

0 commit comments

Comments
 (0)