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
This commit is contained in:
Paul Gschwendtner 2024-10-11 12:11:42 +00:00
parent 19369d78ba
commit a141ee8fb2
5 changed files with 127 additions and 17 deletions

View file

@ -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;
}
}
}

View file

@ -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 {

View file

@ -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<ElementRef>();
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<ElementRef>('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<ElementRef>) {}
}
`,
},
]);
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<ElementRef>) {}
}
`);
});
});
});
function populateDeclarationTestCaseComponent(declaration: string): string {

View file

@ -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<ClassFieldUniqueKey, {fieldName: string; isMulti: boolean}>;
@ -285,19 +288,10 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
const filesWithIncompleteMigration = new Map<ts.SourceFile, Set<QueryFunctionName>>();
const filesWithQueryListOutsideOfDeclarations = new WeakSet<ts.SourceFile>();
const knownQueries = new KnownQueries(info, globalMetadata);
const knownQueries = new KnownQueries(info, this.config, globalMetadata);
const referenceResult: ReferenceResult<ClassFieldDescriptor> = {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<ClassFieldDescriptor> = {
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);

View file

@ -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.