diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts b/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts index 84d0a5f2922..b2157b1c47f 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts @@ -8,12 +8,8 @@ import ts from 'typescript'; import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator'; +import {applyImportManagerChanges} from '../../../../utils/tsurge/helpers/apply_import_manager'; import {MigrationResult} from '../result'; -import { - absoluteFrom, - absoluteFromSourceFile, -} from '../../../../../../compiler-cli/src/ngtsc/file_system'; -import {Replacement, TextUpdate} from '../../../../utils/tsurge/replacement'; /** * Phase that applies all changes recorded by the import manager in @@ -24,57 +20,5 @@ export function pass10_applyImportManager( result: MigrationResult, sourceFiles: readonly ts.SourceFile[], ) { - const {newImports, updatedImports, deletedImports} = importManager.finalize(); - const printer = ts.createPrinter({}); - const pathToFile = new Map(sourceFiles.map((s) => [s.fileName, s])); - - // Capture new imports - newImports.forEach((newImports, fileName) => { - newImports.forEach((newImport) => { - const printedImport = printer.printNode( - ts.EmitHint.Unspecified, - newImport, - pathToFile.get(fileName)!, - ); - result.replacements.push( - new Replacement( - absoluteFrom(fileName), - new TextUpdate({position: 0, end: 0, toInsert: `${printedImport}\n`}), - ), - ); - }); - }); - - // Capture updated imports - for (const [oldBindings, newBindings] of updatedImports.entries()) { - const printedBindings = printer.printNode( - ts.EmitHint.Unspecified, - newBindings, - oldBindings.getSourceFile(), - ); - result.replacements.push( - new Replacement( - absoluteFromSourceFile(oldBindings.getSourceFile()), - new TextUpdate({ - position: oldBindings.getStart(), - end: oldBindings.getEnd(), - toInsert: printedBindings, - }), - ), - ); - } - - // Update removed imports - for (const removedImport of deletedImports) { - result.replacements.push( - new Replacement( - absoluteFromSourceFile(removedImport.getSourceFile()), - new TextUpdate({ - position: removedImport.getStart(), - end: removedImport.getEnd(), - toInsert: '', - }), - ), - ); - } + applyImportManagerChanges(importManager, result.replacements, sourceFiles); } diff --git a/packages/core/schematics/migrations/signal-queries-migration/convert_query_property.ts b/packages/core/schematics/migrations/signal-queries-migration/convert_query_property.ts index 5af12d3fcb9..4a4c04fc720 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/convert_query_property.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/convert_query_property.ts @@ -14,6 +14,7 @@ import {ImportManager} from '../../../../compiler-cli/private/migrations'; import assert from 'assert'; import {WrappedNodeExpr} from '@angular/compiler'; import {removeFromUnionIfPossible} from '../signal-migration/src/utils/remove_from_union'; +import {extractQueryListType} from './query_list_type'; const printer = ts.createPrinter(); @@ -58,6 +59,16 @@ export function computeReplacementsToMigrateQuery( ]; let type = node.type; + // For multi queries, attempt to unwrap `QueryList` types, or infer the + // type from the initializer, if possible. + if (!metadata.queryInfo.first) { + if (type === undefined && node.initializer !== undefined) { + type = extractQueryListType(node.initializer); + } else if (type !== undefined) { + type = extractQueryListType(type); + } + } + if (metadata.queryInfo.read !== null) { assert(metadata.queryInfo.read instanceof WrappedNodeExpr); optionProperties.push( diff --git a/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts b/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts index 1c89aeabae3..da8011f9171 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts @@ -19,7 +19,8 @@ interface TestCase { focus?: boolean; } -const testCases: TestCase[] = [ +const declarationTestCases: TestCase[] = [ + // View Child { id: 'viewChild with string locator and nullable', before: `@ViewChild('myBtn') button: MyButton|undefined = undefined;`, @@ -50,6 +51,129 @@ const testCases: TestCase[] = [ before: `@ViewChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`, after: `readonly buttonEl = viewChild.required('myBtn', { read: ElementRef });`, }, + // Content Child + { + id: 'contentChild with string locator and nullable', + before: `@ContentChild('myBtn') button: MyButton|undefined = undefined;`, + after: `readonly button = contentChild('myBtn');`, + }, + { + id: 'contentChild with class type locator and nullable', + before: `@ContentChild(MyButton) button: MyButton|undefined = undefined;`, + after: `readonly button = contentChild(MyButton);`, + }, + { + id: 'contentChild with class type locator and nullable via question-mark shorthand', + before: `@ContentChild(MyButton) button?: MyButton;`, + after: `readonly button = contentChild(MyButton);`, + }, + { + id: 'contentChild with class type locator and exclamation mark to simulate required', + before: `@ContentChild(MyButton) button!: MyButton;`, + after: `readonly button = contentChild.required(MyButton);`, + }, + { + id: 'contentChild with string locator and read option, nullable shorthand', + before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl?: ElementRef;`, + after: `readonly buttonEl = contentChild('myBtn', { read: ElementRef });`, + }, + { + id: 'contentChild with string locator and read option, required', + before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`, + after: `readonly buttonEl = contentChild.required('myBtn', { read: ElementRef });`, + }, + { + id: 'contentChild with string locator and read option, required', + before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`, + after: `readonly buttonEl = contentChild.required('myBtn', { read: ElementRef });`, + }, + { + id: 'contentChild with descendants option', + before: `@ContentChild('myBtn', {descendants: false}) buttonEl!: ElementRef;`, + after: `readonly buttonEl = contentChild.required('myBtn', { descendants: false });`, + }, + // ViewChildren + { + id: 'viewChildren with string locator and nullable', + before: `@ViewChildren('myBtn') button?: QueryList;`, + after: `readonly button = viewChildren('myBtn');`, + }, + { + id: 'viewChildren with class type locator and nullable', + before: `@ViewChildren(MyButton) button?: QueryList;`, + after: `readonly button = viewChildren(MyButton);`, + }, + { + id: 'viewChildren with class type locator and exclamation mark', + before: `@ViewChildren(MyButton) button!: QueryList;`, + after: `readonly button = viewChildren(MyButton);`, + }, + { + id: 'viewChild with string locator and read option, nullable shorthand', + before: `@ViewChildren('myBtn', {read: ElementRef}) buttonEl?: QueryList;`, + after: `readonly buttonEl = viewChildren('myBtn', { read: ElementRef });`, + }, + { + id: 'viewChildren with string locator and read option, required', + before: `@ViewChildren('myBtn', {read: ElementRef}) buttonEl!: QueryList;`, + after: `readonly buttonEl = viewChildren('myBtn', { read: ElementRef });`, + }, + { + id: 'viewChildren with query list as initializer value', + before: `@ViewChildren('myBtn') buttonEl = new QueryList()`, + after: `readonly buttonEl = viewChildren('myBtn');`, + }, + { + id: 'viewChildren with query list as initializer value, and descendants option', + before: `@ViewChildren('myBtn', {descendants: false}) buttonEl = new QueryList()`, + after: `readonly buttonEl = viewChildren('myBtn', { descendants: false });`, + }, + { + id: 'viewChildren with query list as initializer value, and descendants option but same as default', + before: `@ViewChildren('myBtn', {descendants: true}) buttonEl = new QueryList()`, + after: `readonly buttonEl = viewChildren('myBtn');`, + }, + // ContentChildren + { + id: 'contentChildren with string locator and nullable', + before: `@ContentChildren('myBtn') button?: QueryList;`, + after: `readonly button = contentChildren('myBtn');`, + }, + { + id: 'contentChildren with class type locator and nullable', + before: `@ContentChildren(MyButton) button?: QueryList;`, + after: `readonly button = contentChildren(MyButton);`, + }, + { + id: 'contentChildren with class type locator and exclamation mark', + before: `@ContentChildren(MyButton) button!: QueryList;`, + after: `readonly button = contentChildren(MyButton);`, + }, + { + id: 'contentChildren with string locator and read option, nullable shorthand', + before: `@ContentChildren('myBtn', {read: ElementRef}) buttonEl?: QueryList;`, + after: `readonly buttonEl = contentChildren('myBtn', { read: ElementRef });`, + }, + { + id: 'contentChildren with string locator and read option, required', + before: `@ContentChildren('myBtn', {read: ElementRef}) buttonEl!: QueryList;`, + after: `readonly buttonEl = contentChildren('myBtn', { read: ElementRef });`, + }, + { + id: 'contentChildren with query list as initializer value', + before: `@ContentChildren('myBtn') buttonEl = new QueryList()`, + after: `readonly buttonEl = contentChildren('myBtn');`, + }, + { + id: 'contentChildren with query list as initializer value, and descendants option', + before: `@ContentChildren('myBtn', {descendants: true}) buttonEl = new QueryList()`, + after: `readonly buttonEl = contentChildren('myBtn', { descendants: true });`, + }, + { + id: 'contentChildren with query list as initializer value, and descendants option but same as default', + before: `@ContentChildren('myBtn', {descendants: false}) buttonEl = new QueryList()`, + after: `readonly buttonEl = contentChildren('myBtn');`, + }, ]; describe('signal queries migration', () => { @@ -57,19 +181,24 @@ describe('signal queries migration', () => { initMockFileSystem('Native'); }); - describe('test cases', () => { - for (const c of testCases) { + describe('declaration test cases', () => { + for (const c of declarationTestCases) { (c.focus ? fit : it)(c.id, async () => { const fs = await runTsurgeMigration(new SignalQueriesMigration(), [ { name: absoluteFrom('/app.component.ts'), isProgramRootFile: true, - contents: populateTestCaseComponent(c.before), + contents: populateDeclarationTestCaseComponent(c.before), }, ]); - const actual = fs.readFile(absoluteFrom('/app.component.ts')); - const expected = populateTestCaseComponent(c.after); + let actual = fs.readFile(absoluteFrom('/app.component.ts')); + let expected = populateDeclarationTestCaseComponent(c.after); + + // Cut off the string before the class declaration. + // The import diff is irrelevant here for now. + actual = actual.substring(actual.indexOf('@Directive')); + expected = expected.substring(expected.indexOf('@Directive')); if (actual !== expected) { expect(diffText(expected, actual)).toBe(''); @@ -102,9 +231,59 @@ describe('signal queries migration', () => { expect(actual).not.toContain(`viewChild`); expect(actual).toContain(`@ViewChild('labelRef') label?: ElementRef;`); }); + + it('should update imports when migrating', async () => { + const fs = await runTsurgeMigration(new SignalQueriesMigration(), [ + { + name: absoluteFrom('/app.component.ts'), + isProgramRootFile: true, + contents: ` + import {ViewChild, ElementRef, Directive} from '@angular/core'; + + @Directive() + class MyComp { + @ViewChild('labelRef') label?: ElementRef; + } + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app.component.ts')); + expect(actual).toContain(`import { ElementRef, Directive, viewChild } from '@angular/core';`); + expect(actual).toContain(`label = viewChild('labelRef')`); + }); + + it('should not remove imports when partially migrating', async () => { + const fs = await runTsurgeMigration(new SignalQueriesMigration(), [ + { + name: absoluteFrom('/app.component.ts'), + isProgramRootFile: true, + contents: ` + import {ViewChild, ElementRef, Directive} from '@angular/core'; + + @Directive() + class MyComp { + @ViewChild('labelRef') label?: ElementRef; + @ViewChild('labelRef2') label2?: ElementRef; + + click() { + this.label2 = undefined; + } + } + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app.component.ts')); + expect(actual).toContain( + `import { ViewChild, ElementRef, Directive, viewChild } from '@angular/core';`, + ); + expect(actual).toContain(`label = viewChild('labelRef')`); + expect(actual).toContain(`@ViewChild('labelRef2') label2?: ElementRef;`); + }); }); -function populateTestCaseComponent(declaration: string): string { +function populateDeclarationTestCaseComponent(declaration: string): string { return ` import { ViewChild, diff --git a/packages/core/schematics/migrations/signal-queries-migration/migration.ts b/packages/core/schematics/migrations/signal-queries-migration/migration.ts index fc7ae2cc4a9..19884c31220 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration.ts @@ -24,6 +24,9 @@ import {unwrapParent} from '../signal-migration/src/utils/unwrap_parent'; import {writeBinaryOperators} from '../signal-migration/src/utils/write_operators'; import {computeReplacementsToMigrateQuery} from './convert_query_property'; import {ImportManager, PartialEvaluator} from '../../../../compiler-cli/private/migrations'; +import {applyImportManagerChanges} from '../../utils/tsurge/helpers/apply_import_manager'; +import {QueryFunctionName} from '../../../../compiler-cli/src/ngtsc/annotations'; +import {queryFunctionNameToDecorator} from './query_api_names'; export interface CompilationUnitData { knownQueryFields: Record; @@ -112,6 +115,9 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< const replacements: Replacement[] = []; const importManager = new ImportManager(); + const filesWithMigratedQueries = new Map>(); + const filesWithIncompleteMigration = new Map>(); + const isMigratedQuery = (id: ClassPropertyID) => globalMetadata.knownQueryFields[id] !== undefined && globalMetadata.problematicQueries[id] === undefined; @@ -124,7 +130,13 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< evaluator, projectDirAbsPath, ); - if (extractedQuery !== null && isMigratedQuery(extractedQuery.id)) { + if (extractedQuery !== null) { + if (!isMigratedQuery(extractedQuery.id)) { + updateFileState(filesWithIncompleteMigration, node, extractedQuery.kind); + return; + } + + updateFileState(filesWithMigratedQueries, node, extractedQuery.kind); replacements.push( ...computeReplacementsToMigrateQuery( node as ts.PropertyDeclaration, @@ -132,6 +144,7 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< importManager, ), ); + return; } // Migrate references to queries, if those are migrated too. @@ -153,6 +166,34 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< ts.forEachChild(sf, visitor); } + // Remove imports if possible. + for (const [file, types] of filesWithMigratedQueries) { + for (const type of types) { + if (!filesWithIncompleteMigration.get(file)?.has(type)) { + importManager.removeImport(file, queryFunctionNameToDecorator(type), '@angular/core'); + } + } + } + + applyImportManagerChanges(importManager, replacements, sourceFiles); + return replacements; } } + +/** + * Updates the given map to capture the given query type. + * The map may track migrated queries in a file, or query types + * that couldn't be migrated. + */ +function updateFileState( + stateMap: Map>, + node: ts.Node, + queryType: QueryFunctionName, +): void { + const file = node.getSourceFile(); + if (!stateMap.has(file)) { + stateMap.set(file, new Set()); + } + stateMap.get(file)!.add(queryType); +} diff --git a/packages/core/schematics/migrations/signal-queries-migration/query_api_names.ts b/packages/core/schematics/migrations/signal-queries-migration/query_api_names.ts new file mode 100644 index 00000000000..c7169757ce3 --- /dev/null +++ b/packages/core/schematics/migrations/signal-queries-migration/query_api_names.ts @@ -0,0 +1,23 @@ +/** + * @license + * Copyright Google LLC 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 {QueryFunctionName} from '../../../../compiler-cli/src/ngtsc/annotations'; + +/** Converts an initializer query API name to its decorator-equivalent. */ +export function queryFunctionNameToDecorator(name: QueryFunctionName): string { + if (name === 'viewChild') { + return 'ViewChild'; + } else if (name === 'viewChildren') { + return 'ViewChildren'; + } else if (name === 'contentChild') { + return 'ContentChild'; + } else if (name === 'contentChildren') { + return 'ContentChildren'; + } + throw new Error(`Unexpected query function name: ${name}`); +} diff --git a/packages/core/schematics/migrations/signal-queries-migration/query_list_type.ts b/packages/core/schematics/migrations/signal-queries-migration/query_list_type.ts new file mode 100644 index 00000000000..78cd13c366b --- /dev/null +++ b/packages/core/schematics/migrations/signal-queries-migration/query_list_type.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright Google LLC 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 ts from 'typescript'; + +/** + * Extracts the type `T` of expressions referencing `QueryList`. + */ +export function extractQueryListType(node: ts.TypeNode | ts.Expression): ts.TypeNode | undefined { + // Initializer variant of `new QueryList()`. + if ( + ts.isNewExpression(node) && + ts.isIdentifier(node.expression) && + node.expression.text === 'QueryList' + ) { + return node.typeArguments?.[0]; + } + + // Type variant of `: QueryList`. + if ( + ts.isTypeReferenceNode(node) && + ts.isIdentifier(node.typeName) && + node.typeName.text === 'QueryList' + ) { + return node.typeArguments?.[0]; + } + + return undefined; +} diff --git a/packages/core/schematics/utils/tsurge/BUILD.bazel b/packages/core/schematics/utils/tsurge/BUILD.bazel index f28e099b9aa..fbc667e0fec 100644 --- a/packages/core/schematics/utils/tsurge/BUILD.bazel +++ b/packages/core/schematics/utils/tsurge/BUILD.bazel @@ -13,6 +13,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", "//packages/compiler-cli/src/ngtsc/shims", + "//packages/compiler-cli/src/ngtsc/translator", "@npm//@types/diff", "@npm//@types/node", "@npm//chalk", diff --git a/packages/core/schematics/utils/tsurge/helpers/apply_import_manager.ts b/packages/core/schematics/utils/tsurge/helpers/apply_import_manager.ts new file mode 100644 index 00000000000..7448356c1e8 --- /dev/null +++ b/packages/core/schematics/utils/tsurge/helpers/apply_import_manager.ts @@ -0,0 +1,79 @@ +/** + * @license + * Copyright Google LLC 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 ts from 'typescript'; +import {ImportManager} from '../../../../../compiler-cli/src/ngtsc/translator'; +import { + absoluteFrom, + absoluteFromSourceFile, +} from '../../../../../compiler-cli/src/ngtsc/file_system'; +import {Replacement, TextUpdate} from '../replacement'; + +/** + * Applies import manager changes, and writes them as replacements the + * given result array. + */ +export function applyImportManagerChanges( + importManager: ImportManager, + replacements: Replacement[], + sourceFiles: readonly ts.SourceFile[], +) { + const {newImports, updatedImports, deletedImports} = importManager.finalize(); + const printer = ts.createPrinter({}); + const pathToFile = new Map(sourceFiles.map((s) => [s.fileName, s])); + + // Capture new imports + newImports.forEach((newImports, fileName) => { + newImports.forEach((newImport) => { + const printedImport = printer.printNode( + ts.EmitHint.Unspecified, + newImport, + pathToFile.get(fileName)!, + ); + replacements.push( + new Replacement( + absoluteFrom(fileName), + new TextUpdate({position: 0, end: 0, toInsert: `${printedImport}\n`}), + ), + ); + }); + }); + + // Capture updated imports + for (const [oldBindings, newBindings] of updatedImports.entries()) { + const printedBindings = printer.printNode( + ts.EmitHint.Unspecified, + newBindings, + oldBindings.getSourceFile(), + ); + replacements.push( + new Replacement( + absoluteFromSourceFile(oldBindings.getSourceFile()), + new TextUpdate({ + position: oldBindings.getStart(), + end: oldBindings.getEnd(), + toInsert: printedBindings, + }), + ), + ); + } + + // Update removed imports + for (const removedImport of deletedImports) { + replacements.push( + new Replacement( + absoluteFromSourceFile(removedImport.getSourceFile()), + new TextUpdate({ + position: removedImport.getStart(), + end: removedImport.getEnd(), + toInsert: '', + }), + ), + ); + } +} diff --git a/packages/core/schematics/utils/tsurge/testing/diff.ts b/packages/core/schematics/utils/tsurge/testing/diff.ts index 782e3ef5727..1452a6f6421 100644 --- a/packages/core/schematics/utils/tsurge/testing/diff.ts +++ b/packages/core/schematics/utils/tsurge/testing/diff.ts @@ -25,7 +25,7 @@ export function diffText(expected: string, actual: string): string { ? chalk.green(valueForColor) : part.removed ? chalk.red(valueForColor) - : part.value; + : chalk.reset(part.value); result += text; }