From a141ee8fb296ee7ae4436bfa498f5ed9c2b5a08d Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 11 Oct 2024 12:11:42 +0000 Subject: [PATCH] refactor(migrations): add `best-effort-mode` for queries migration (#58168) Allows users to ignore detected incompatibilities and migrate as much as possible. Similar to the input migration. PR Close #58168 --- .../incompatibility.ts | 17 ++++- .../signal-queries-migration/known_queries.ts | 19 ++++- .../migration.spec.ts | 74 +++++++++++++++++++ .../signal-queries-migration/migration.ts | 28 +++---- .../migration_config.ts | 6 ++ 5 files changed, 127 insertions(+), 17 deletions(-) diff --git a/packages/core/schematics/migrations/signal-queries-migration/incompatibility.ts b/packages/core/schematics/migrations/signal-queries-migration/incompatibility.ts index 4fa739d899e..ead0811498f 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/incompatibility.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/incompatibility.ts @@ -7,8 +7,12 @@ */ import {FieldIncompatibilityReason} from '../signal-migration/src'; -import {pickFieldIncompatibility} from '../signal-migration/src/passes/problematic_patterns/incompatibility'; +import { + nonIgnorableFieldIncompatibilities, + pickFieldIncompatibility, +} from '../signal-migration/src/passes/problematic_patterns/incompatibility'; import {ClassFieldUniqueKey} from '../signal-migration/src/passes/reference_resolution/known_fields'; +import type {KnownQueries} from './known_queries'; import type {GlobalUnitData} from './migration'; export function markFieldIncompatibleInMetadata( @@ -31,3 +35,14 @@ export function markFieldIncompatibleInMetadata( ).reason; } } + +export function filterBestEffortIncompatibilities(knownQueries: KnownQueries) { + for (const query of Object.values(knownQueries.globalMetadata.problematicQueries)) { + if ( + query.fieldReason !== null && + !nonIgnorableFieldIncompatibilities.includes(query.fieldReason) + ) { + query.fieldReason = null; + } + } +} diff --git a/packages/core/schematics/migrations/signal-queries-migration/known_queries.ts b/packages/core/schematics/migrations/signal-queries-migration/known_queries.ts index 7e7cb75285d..f07911d6c96 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/known_queries.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/known_queries.ts @@ -7,7 +7,7 @@ */ import ts from 'typescript'; -import {ProgramInfo} from '../../utils/tsurge'; +import {ProgramInfo, projectFile} from '../../utils/tsurge'; import {ProblematicFieldRegistry} from '../signal-migration/src/passes/problematic_patterns/problematic_field_registry'; import { ClassFieldDescriptor, @@ -28,6 +28,7 @@ import { } from '../signal-migration/src/passes/problematic_patterns/incompatibility'; import {markFieldIncompatibleInMetadata} from './incompatibility'; import {ExtractedQuery} from './identify_queries'; +import {MigrationConfig} from './migration_config'; export class KnownQueries implements @@ -41,7 +42,8 @@ export class KnownQueries constructor( private readonly info: ProgramInfo, - private globalMetadata: GlobalUnitData, + private readonly config: MigrationConfig, + public globalMetadata: GlobalUnitData, ) {} isFieldIncompatible(descriptor: ClassFieldDescriptor): boolean { @@ -69,6 +71,19 @@ export class KnownQueries node: queryField, }); this.knownQueryIDs.set(id, {key: id, node: queryField}); + + const descriptor: ClassFieldDescriptor = {key: id, node: queryField}; + const file = projectFile(queryField.getSourceFile(), this.info); + + if ( + this.config.shouldMigrateQuery !== undefined && + !this.config.shouldMigrateQuery(descriptor, file) + ) { + this.markFieldIncompatible(descriptor, { + context: null, + reason: FieldIncompatibilityReason.SkippedViaConfigFilter, + }); + } } attemptRetrieveDescriptorFromSymbol(symbol: ts.Symbol): ClassFieldDescriptor | null { 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 d437b8e894e..6db6e1f7c1a 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts @@ -1239,6 +1239,80 @@ describe('signal queries migration', () => { } `); }); + + describe('--best-effort-mode', () => { + it('should be possible to forcibly migrate even with a detected `.changes` access', async () => { + const {fs} = await runTsurgeMigration(new SignalQueriesMigration({bestEffortMode: true}), [ + { + name: absoluteFrom('/app.component.ts'), + isProgramRootFile: true, + contents: dedent` + import {ViewChildren, QueryList, ElementRef, Component} from '@angular/core'; + + @Component({ + template: '', + }) + class MyComp { + @ViewChildren('label') labels = new QueryList(); + + click() { + this.labels.changes.subscribe(); + } + } + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app.component.ts')); + expect(actual).toMatchWithDiff(` + import {ElementRef, Component, viewChildren} from '@angular/core'; + + @Component({ + template: '', + }) + class MyComp { + readonly labels = viewChildren('label'); + + click() { + this.labels().changes.subscribe(); + } + } + `); + }); + + it(`should not forcibly migrate if it's an accessor field`, async () => { + const {fs} = await runTsurgeMigration(new SignalQueriesMigration({bestEffortMode: true}), [ + { + name: absoluteFrom('/app.component.ts'), + isProgramRootFile: true, + contents: dedent` + import {ViewChildren, QueryList, ElementRef, Component} from '@angular/core'; + + @Component({ + template: '', + }) + class MyComp { + @ViewChildren('label') + set labels(list: QueryList) {} + } + `, + }, + ]); + + const actual = fs.readFile(absoluteFrom('/app.component.ts')); + expect(actual).toMatchWithDiff(` + import {ViewChildren, QueryList, ElementRef, Component} from '@angular/core'; + + @Component({ + template: '', + }) + class MyComp { + @ViewChildren('label') + set labels(list: QueryList) {} + } + `); + }); + }); }); function populateDeclarationTestCaseComponent(declaration: string): string { diff --git a/packages/core/schematics/migrations/signal-queries-migration/migration.ts b/packages/core/schematics/migrations/signal-queries-migration/migration.ts index 4b25e288a47..56198a45ed7 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration.ts @@ -54,7 +54,10 @@ import {replaceQueryListGetCall} from './fn_get_replacement'; import {checkForIncompatibleQueryListAccesses} from './incompatible_query_list_fns'; import {replaceQueryListFirstAndLastReferences} from './fn_first_last_replacement'; import {MigrationConfig} from './migration_config'; -import {markFieldIncompatibleInMetadata} from './incompatibility'; +import { + filterBestEffortIncompatibilities, + markFieldIncompatibleInMetadata, +} from './incompatibility'; export interface CompilationUnitData { knownQueryFields: Record; @@ -285,19 +288,10 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< const filesWithIncompleteMigration = new Map>(); const filesWithQueryListOutsideOfDeclarations = new WeakSet(); - const knownQueries = new KnownQueries(info, globalMetadata); + const knownQueries = new KnownQueries(info, this.config, globalMetadata); const referenceResult: ReferenceResult = {references: []}; const sourceQueries: ExtractedQuery[] = []; - const isMigratedQuery = (descriptor: ClassFieldDescriptor) => - globalMetadata.knownQueryFields[descriptor.key] !== undefined && - globalMetadata.problematicQueries[descriptor.key] === undefined && - (this.config.shouldMigrateQuery === undefined || - this.config.shouldMigrateQuery( - descriptor, - projectFile(descriptor.node.getSourceFile(), info), - )); - // Detect all queries in this unit. const queryWholeProgramVisitor = (node: ts.Node) => { // Detect all SOURCE queries and migrate them, if possible. @@ -388,13 +382,17 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< this.config.reportProgressFn?.(80, 'Migrating queries..'); + if (this.config.bestEffortMode) { + filterBestEffortIncompatibilities(knownQueries); + } + // Migrate declarations. for (const extractedQuery of sourceQueries) { const node = extractedQuery.node; const sf = node.getSourceFile(); const descriptor = {key: extractedQuery.id, node: extractedQuery.node}; - if (!isMigratedQuery(descriptor)) { + if (knownQueries.isFieldIncompatible(descriptor)) { updateFileState(filesWithSourceQueries, sf, extractedQuery.kind); updateFileState(filesWithIncompleteMigration, sf, extractedQuery.kind); continue; @@ -416,9 +414,11 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< const referenceMigrationHost: ReferenceMigrationHost = { printer, replacements, - shouldMigrateReferencesToField: (field) => isMigratedQuery(field), + shouldMigrateReferencesToField: (field) => !knownQueries.isFieldIncompatible(field), shouldMigrateReferencesToClass: (clazz) => - !!knownQueries.getQueryFieldsOfClass(clazz)?.some((q) => isMigratedQuery(q)), + !!knownQueries + .getQueryFieldsOfClass(clazz) + ?.some((q) => !knownQueries.isFieldIncompatible(q)), }; migrateTypeScriptReferences(referenceMigrationHost, referenceResult.references, checker, info); migrateTemplateReferences(referenceMigrationHost, referenceResult.references); diff --git a/packages/core/schematics/migrations/signal-queries-migration/migration_config.ts b/packages/core/schematics/migrations/signal-queries-migration/migration_config.ts index a4888257495..133afb41d71 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration_config.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration_config.ts @@ -10,6 +10,12 @@ import {ProjectFile} from '../../utils/tsurge'; import {ClassFieldDescriptor} from '../signal-migration/src/passes/reference_resolution/known_fields'; export interface MigrationConfig { + /** + * Whether to migrate as much as possible, even if certain + * queries would otherwise be marked as incompatible for migration. + */ + bestEffortMode?: boolean; + /** * Whether the given query should be migrated. With batch execution, this * callback fires for foreign queries from other compilation units too.