From 30525e004e0b98bff6f0d1a748cfa4e13499da35 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 22 Oct 2024 12:01:41 -0700 Subject: [PATCH] Revert "refactor(migrations): update migrations to combine analysis data in parallel (#58280)" This reverts commit 8735543d06322863b5de2e2c050db90e3f42989b. --- .../output-migration/output-migration.ts | 14 +---- .../src/batch/merge_unit_data.ts | 55 ++++++----------- .../signal-migration/src/batch/test_bin.ts | 28 ++++----- .../signal-migration/src/migration.ts | 21 +++---- .../signal-queries-migration/migration.ts | 60 +++++-------------- .../signal-input-migration/index.ts | 13 +--- .../signal-queries-migration/index.ts | 13 +--- .../apply_query_refactoring.ts | 4 +- 8 files changed, 61 insertions(+), 147 deletions(-) diff --git a/packages/core/schematics/migrations/output-migration/output-migration.ts b/packages/core/schematics/migrations/output-migration/output-migration.ts index 50b4c827b9519..18c8bf21dea7a 100644 --- a/packages/core/schematics/migrations/output-migration/output-migration.ts +++ b/packages/core/schematics/migrations/output-migration/output-migration.ts @@ -291,10 +291,7 @@ export class OutputMigration extends TsurgeFunnelMigration< }); } - override async combine( - unitA: CompilationUnitData, - unitB: CompilationUnitData, - ): Promise> { + override async merge(units: CompilationUnitData[]): Promise> { const outputFields: Record = {}; const importReplacements: Record< ProjectFileID, @@ -302,7 +299,7 @@ export class OutputMigration extends TsurgeFunnelMigration< > = {}; const problematicUsages: Record = {}; - for (const unit of [unitA, unitB]) { + for (const unit of units) { for (const declIdStr of Object.keys(unit.outputFields)) { const declId = declIdStr as ClassFieldUniqueKey; // THINK: detect clash? Should we have an utility to merge data based on unique IDs? @@ -327,13 +324,6 @@ export class OutputMigration extends TsurgeFunnelMigration< }); } - override async globalMeta( - combinedData: CompilationUnitData, - ): Promise> { - // Noop here as we don't have any form of special global metadata. - return confirmAsSerializable(combinedData); - } - override async stats(globalMetadata: CompilationUnitData): Promise { // TODO: Add statistics. return {counters: {}}; diff --git a/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts b/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts index 24e67b4f02914..f2b5da47f5dec 100644 --- a/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts +++ b/packages/core/schematics/migrations/signal-migration/src/batch/merge_unit_data.ts @@ -16,19 +16,30 @@ import {CompilationUnitData} from './unit_data'; type InputData = {key: string; info: CompilationUnitData['knownInputs'][string]}; /** Merges a list of compilation units into a combined unit. */ -export function combineCompilationUnitData( - unitA: CompilationUnitData, - unitB: CompilationUnitData, +export function mergeCompilationUnitData( + metadataFiles: CompilationUnitData[], ): CompilationUnitData { const result: CompilationUnitData = { knownInputs: {}, }; - for (const file of [unitA, unitB]) { + const idToGraphNode = new Map>(); + const inheritanceGraph: GraphNode[] = []; + const isNodeIncompatible = (node: InputData) => + node.info.memberIncompatibility !== null || node.info.owningClassIncompatibility !== null; + + for (const file of metadataFiles) { for (const [key, info] of Object.entries(file.knownInputs)) { const existing = result.knownInputs[key]; if (existing === undefined) { result.knownInputs[key] = info; + const node: GraphNode = { + incoming: new Set(), + outgoing: new Set(), + data: {info, key}, + }; + inheritanceGraph.push(node); + idToGraphNode.set(key, node); continue; } @@ -66,37 +77,7 @@ export function combineCompilationUnitData( } } - return result; -} - -export function convertToGlobalMeta(combinedData: CompilationUnitData): CompilationUnitData { - const globalMeta: CompilationUnitData = { - knownInputs: {}, - }; - - const idToGraphNode = new Map>(); - const inheritanceGraph: GraphNode[] = []; - const isNodeIncompatible = (node: InputData) => - node.info.memberIncompatibility !== null || node.info.owningClassIncompatibility !== null; - - for (const [key, info] of Object.entries(combinedData.knownInputs)) { - const existing = globalMeta.knownInputs[key]; - if (existing !== undefined) { - continue; - } - - const node: GraphNode = { - incoming: new Set(), - outgoing: new Set(), - data: {info, key}, - }; - inheritanceGraph.push(node); - idToGraphNode.set(key, node); - - globalMeta.knownInputs[key] = info; - } - - for (const [key, info] of Object.entries(globalMeta.knownInputs)) { + for (const [key, info] of Object.entries(result.knownInputs)) { if (info.extendsFrom !== null) { const from = idToGraphNode.get(key)!; const target = idToGraphNode.get(info.extendsFrom)!; @@ -128,7 +109,7 @@ export function convertToGlobalMeta(combinedData: CompilationUnitData): Compilat } } - for (const info of Object.values(combinedData.knownInputs)) { + for (const info of Object.values(result.knownInputs)) { // We never saw a source file for this input, globally. Try marking it as incompatible, // so that all references and inheritance checks can propagate accordingly. if (!info.seenAsSourceInput) { @@ -144,5 +125,5 @@ export function convertToGlobalMeta(combinedData: CompilationUnitData): Compilat } } - return globalMeta; + return result; } diff --git a/packages/core/schematics/migrations/signal-migration/src/batch/test_bin.ts b/packages/core/schematics/migrations/signal-migration/src/batch/test_bin.ts index 28ad0bb8bee2c..b8037a4cefc3c 100644 --- a/packages/core/schematics/migrations/signal-migration/src/batch/test_bin.ts +++ b/packages/core/schematics/migrations/signal-migration/src/batch/test_bin.ts @@ -9,12 +9,11 @@ import fs from 'fs'; import path from 'path'; import {executeAnalyzePhase} from '../../../../utils/tsurge/executors/analyze_exec'; -import {executeCombinePhase} from '../../../../utils/tsurge/executors/combine_exec'; +import {executeMergePhase} from '../../../../utils/tsurge/executors/merge_exec'; import {executeMigratePhase} from '../../../../utils/tsurge/executors/migrate_exec'; import {SignalInputMigration} from '../migration'; import {writeMigrationReplacements} from '../write_replacements'; import {CompilationUnitData} from './unit_data'; -import {executeGlobalMetaPhase} from '../../../../utils/tsurge/executors/global_meta_exec'; main().catch((e) => { console.error(e); @@ -28,16 +27,19 @@ async function main() { if (mode === 'extract') { const analyzeResult = await executeAnalyzePhase(migration, path.resolve(args[0])); process.stdout.write(JSON.stringify(analyzeResult)); - } else if (mode === 'combine') { - const unitAPromise = readUnitMeta(path.resolve(args[0])); - const unitBPromise = readUnitMeta(path.resolve(args[1])); - - const [unitA, unitB] = await Promise.all([unitAPromise, unitBPromise]); - const mergedResult = await executeCombinePhase(migration, unitA, unitB); + } else if (mode === 'merge') { + const mergedResult = await executeMergePhase( + migration, + await Promise.all( + args.map((p) => + fs.promises + .readFile(path.resolve(p), 'utf8') + .then((data) => JSON.parse(data) as CompilationUnitData), + ), + ), + ); process.stdout.write(JSON.stringify(mergedResult)); - } else if (mode === 'globalMeta') { - await executeGlobalMetaPhase(migration, await readUnitMeta(args[0])); } else if (mode === 'migrate') { const {replacements, projectRoot} = await executeMigratePhase( migration, @@ -48,9 +50,3 @@ async function main() { writeMigrationReplacements(replacements, projectRoot); } } - -async function readUnitMeta(filePath: string): Promise { - return fs.promises - .readFile(filePath, 'utf8') - .then((data) => JSON.parse(data) as CompilationUnitData); -} diff --git a/packages/core/schematics/migrations/signal-migration/src/migration.ts b/packages/core/schematics/migrations/signal-migration/src/migration.ts index c392c04cf96e4..35433afadebe3 100644 --- a/packages/core/schematics/migrations/signal-migration/src/migration.ts +++ b/packages/core/schematics/migrations/signal-migration/src/migration.ts @@ -18,7 +18,7 @@ import {MigrationHost} from './migration_host'; import {executeAnalysisPhase} from './phase_analysis'; import {pass4__checkInheritanceOfInputs} from './passes/4_check_inheritance'; import {getCompilationUnitMetadata} from './batch/extract'; -import {convertToGlobalMeta, combineCompilationUnitData} from './batch/merge_unit_data'; +import {mergeCompilationUnitData} from './batch/merge_unit_data'; import {Replacement} from '../../../utils/tsurge/replacement'; import {populateKnownInputsFromGlobalData} from './batch/populate_global_data'; import {executeMigrationPhase} from './phase_migrate'; @@ -28,8 +28,10 @@ import { ClassIncompatibilityReason, FieldIncompatibilityReason, } from './passes/problematic_patterns/incompatibility'; +import {isInputDescriptor} from './utils/input_id'; import {MigrationConfig} from './migration_config'; import {ClassFieldUniqueKey} from './passes/reference_resolution/known_fields'; +import {MigrationStats} from '../../../utils/tsurge'; import {createNgtscProgram} from '../../../utils/tsurge/helpers/ngtsc_program'; /** @@ -122,8 +124,8 @@ export class SignalInputMigration extends TsurgeComplexMigration< // Non-batch mode! if (this.config.upgradeAnalysisPhaseToAvoidBatch) { - const globalMeta = await this.globalMeta(unitData); - const {replacements} = await this.migrate(globalMeta, info, { + const merged = await this.merge([unitData]); + const {replacements} = await this.migrate(merged, info, { knownInputs, result, host, @@ -141,17 +143,8 @@ export class SignalInputMigration extends TsurgeComplexMigration< return confirmAsSerializable(unitData); } - override async combine( - unitA: CompilationUnitData, - unitB: CompilationUnitData, - ): Promise> { - return confirmAsSerializable(combineCompilationUnitData(unitA, unitB)); - } - - override async globalMeta( - combinedData: CompilationUnitData, - ): Promise> { - return confirmAsSerializable(convertToGlobalMeta(combinedData)); + override async merge(units: CompilationUnitData[]): Promise> { + return confirmAsSerializable(mergeCompilationUnitData(units)); } override async migrate( diff --git a/packages/core/schematics/migrations/signal-queries-migration/migration.ts b/packages/core/schematics/migrations/signal-queries-migration/migration.ts index e73bdcb852f21..558e9406097a2 100644 --- a/packages/core/schematics/migrations/signal-queries-migration/migration.ts +++ b/packages/core/schematics/migrations/signal-queries-migration/migration.ts @@ -309,54 +309,44 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< return confirmAsSerializable(res); } - override async combine( - unitA: CompilationUnitData, - unitB: CompilationUnitData, - ): Promise> { - const combined: CompilationUnitData = { + override async merge(units: CompilationUnitData[]): Promise> { + const merged: GlobalUnitData = { knownQueryFields: {}, - potentialProblematicQueries: {}, - potentialProblematicReferenceForMultiQueries: {}, + problematicQueries: {}, reusableAnalysisReferences: null, }; - for (const unit of [unitA, unitB]) { + for (const unit of units) { for (const [id, value] of Object.entries(unit.knownQueryFields)) { - combined.knownQueryFields[id as ClassFieldUniqueKey] = value; + merged.knownQueryFields[id as ClassFieldUniqueKey] = value; } - for (const [id, info] of Object.entries(unit.potentialProblematicQueries)) { if (info.fieldReason !== null) { markFieldIncompatibleInMetadata( - combined.potentialProblematicQueries, + merged.problematicQueries, id as ClassFieldUniqueKey, info.fieldReason, ); } if (info.classReason !== null) { - combined.potentialProblematicQueries[id as ClassFieldUniqueKey] ??= { + merged.problematicQueries[id as ClassFieldUniqueKey] ??= { classReason: null, fieldReason: null, }; - combined.potentialProblematicQueries[id as ClassFieldUniqueKey].classReason = - info.classReason; + merged.problematicQueries[id as ClassFieldUniqueKey].classReason = info.classReason; } } - - for (const id of Object.keys(unit.potentialProblematicReferenceForMultiQueries)) { - combined.potentialProblematicReferenceForMultiQueries[id as ClassFieldUniqueKey] = true; - } - if (unit.reusableAnalysisReferences !== null) { - combined.reusableAnalysisReferences = unit.reusableAnalysisReferences; + assert(units.length === 1, 'Expected migration to not run in batch mode'); + merged.reusableAnalysisReferences = unit.reusableAnalysisReferences; } } - for (const unit of [unitA, unitB]) { + for (const unit of units) { for (const id of Object.keys(unit.potentialProblematicReferenceForMultiQueries)) { - if (combined.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) { + if (merged.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) { markFieldIncompatibleInMetadata( - combined.potentialProblematicQueries, + merged.problematicQueries, id as ClassFieldUniqueKey, FieldIncompatibilityReason.SignalQueries__QueryListProblematicFieldAccessed, ); @@ -364,29 +354,7 @@ export class SignalQueriesMigration extends TsurgeComplexMigration< } } - return confirmAsSerializable(combined); - } - - override async globalMeta( - combinedData: CompilationUnitData, - ): Promise> { - const globalUnitData: GlobalUnitData = { - knownQueryFields: combinedData.knownQueryFields, - problematicQueries: combinedData.potentialProblematicQueries, - reusableAnalysisReferences: combinedData.reusableAnalysisReferences, - }; - - for (const id of Object.keys(combinedData.potentialProblematicReferenceForMultiQueries)) { - if (combinedData.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) { - markFieldIncompatibleInMetadata( - globalUnitData.problematicQueries, - id as ClassFieldUniqueKey, - FieldIncompatibilityReason.SignalQueries__QueryListProblematicFieldAccessed, - ); - } - } - - return confirmAsSerializable(globalUnitData); + return confirmAsSerializable(merged); } override async migrate(globalMetadata: GlobalUnitData, info: ProgramInfo) { diff --git a/packages/core/schematics/ng-generate/signal-input-migration/index.ts b/packages/core/schematics/ng-generate/signal-input-migration/index.ts index f97d225c01d44..46c8174bf194a 100644 --- a/packages/core/schematics/ng-generate/signal-input-migration/index.ts +++ b/packages/core/schematics/ng-generate/signal-input-migration/index.ts @@ -15,7 +15,6 @@ import {groupReplacementsByFile} from '../../utils/tsurge/helpers/group_replacem import {setFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system'; import {CompilationUnitData} from '../../migrations/signal-migration/src/batch/unit_data'; import {ProjectRootRelativePath, TextUpdate} from '../../utils/tsurge'; -import {synchronouslyCombineUnitData} from '../../utils/tsurge/helpers/combine_units'; interface Options { path: string; @@ -78,19 +77,13 @@ export function migrate(options: Options): Rule { context.logger.info(`Processing analysis data between targets..`); context.logger.info(``); - const combined = await synchronouslyCombineUnitData(migration, unitResults); - if (combined === null) { - context.logger.error('Migration failed unexpectedly with no analysis data'); - return; - } - - const globalMeta = await migration.globalMeta(combined); + const merged = await migration.merge(unitResults); const replacementsPerFile: Map = new Map(); for (const {info, tsconfigPath} of programInfos) { context.logger.info(`Migrating: ${tsconfigPath}..`); - const {replacements} = await migration.migrate(globalMeta, info); + const {replacements} = await migration.migrate(merged, info); const changesPerFile = groupReplacementsByFile(replacements); for (const [file, changes] of changesPerFile) { @@ -111,7 +104,7 @@ export function migrate(options: Options): Rule { tree.commitUpdate(recorder); } - const {counters} = await migration.stats(globalMeta); + const {counters} = await migration.stats(merged); const migratedInputs = counters.sourceInputs - counters.incompatibleInputs; context.logger.info(''); diff --git a/packages/core/schematics/ng-generate/signal-queries-migration/index.ts b/packages/core/schematics/ng-generate/signal-queries-migration/index.ts index 2d3514b236302..7f69daafebe51 100644 --- a/packages/core/schematics/ng-generate/signal-queries-migration/index.ts +++ b/packages/core/schematics/ng-generate/signal-queries-migration/index.ts @@ -17,7 +17,6 @@ import { CompilationUnitData, SignalQueriesMigration, } from '../../migrations/signal-queries-migration/migration'; -import {synchronouslyCombineUnitData} from '../../utils/tsurge/helpers/combine_units'; interface Options { path: string; @@ -80,19 +79,13 @@ export function migrate(options: Options): Rule { context.logger.info(`Processing analysis data between targets..`); context.logger.info(``); - const combined = await synchronouslyCombineUnitData(migration, unitResults); - if (combined === null) { - context.logger.error('Migration failed unexpectedly with no analysis data'); - return; - } - - const globalMeta = await migration.globalMeta(combined); + const merged = await migration.merge(unitResults); const replacementsPerFile: Map = new Map(); for (const {info, tsconfigPath} of programInfos) { context.logger.info(`Migrating: ${tsconfigPath}..`); - const {replacements} = await migration.migrate(globalMeta, info); + const {replacements} = await migration.migrate(merged, info); const changesPerFile = groupReplacementsByFile(replacements); for (const [file, changes] of changesPerFile) { @@ -118,7 +111,7 @@ export function migrate(options: Options): Rule { const { counters: {queriesCount, incompatibleQueries, multiQueries}, - } = await migration.stats(globalMeta); + } = await migration.stats(merged); const migratedQueries = queriesCount - incompatibleQueries; context.logger.info(''); diff --git a/packages/language-service/src/refactorings/convert_to_signal_queries/apply_query_refactoring.ts b/packages/language-service/src/refactorings/convert_to_signal_queries/apply_query_refactoring.ts index ff95599f388c0..3eeae93da4f3c 100644 --- a/packages/language-service/src/refactorings/convert_to_signal_queries/apply_query_refactoring.ts +++ b/packages/language-service/src/refactorings/convert_to_signal_queries/apply_query_refactoring.ts @@ -55,8 +55,8 @@ export async function applySignalQueriesRefactoring( }); const unitData = await migration.analyze(programInfo); - const globalMeta = await migration.globalMeta(unitData); - const {replacements, knownQueries} = await migration.migrate(globalMeta, programInfo); + const merged = await migration.merge([unitData]); + const {replacements, knownQueries} = await migration.migrate(merged, programInfo); const targetQueries = Array.from(knownQueries.knownQueryIDs.values()).filter((descriptor) => shouldMigrateQuery(descriptor, projectFile(descriptor.node.getSourceFile(), programInfo)),