mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
refactor: support arbitrary stats/metrics in tsurge (#61272)
Supports arbitrary stats/metrics in Tsurge. This will make complex analysis easier as we aren't bound to just `Record<string, number>` counters. PR Close #61272
This commit is contained in:
committed by
Kristiyan Kostadinov
parent
ef87a56c99
commit
d7a1c19a5e
@@ -91,6 +91,6 @@ export class DocumentCoreMigration extends TsurgeFunnelMigration<
|
||||
}
|
||||
|
||||
override async stats() {
|
||||
return {counters: {}};
|
||||
return confirmAsSerializable({});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -153,7 +153,7 @@ export class InjectFlagsMigration extends TsurgeFunnelMigration<
|
||||
}
|
||||
|
||||
override async stats() {
|
||||
return {counters: {}};
|
||||
return confirmAsSerializable({});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -678,9 +678,9 @@ describe('outputs', () => {
|
||||
]);
|
||||
|
||||
const stats = await runResults.getStatistics();
|
||||
expect(stats.counters['detectedOutputs']).toBe(4);
|
||||
expect(stats.counters['problematicOutputs']).toBe(2);
|
||||
expect(stats.counters['successRate']).toBe(0.5);
|
||||
expect(stats['detectedOutputs']).toBe(4);
|
||||
expect(stats['problematicOutputs']).toBe(2);
|
||||
expect(stats['successRate']).toBe(0.5);
|
||||
});
|
||||
|
||||
it('should capture migration statistics without problematic usages', async () => {
|
||||
@@ -696,9 +696,9 @@ describe('outputs', () => {
|
||||
]);
|
||||
|
||||
const stats = await runResults.getStatistics();
|
||||
expect(stats.counters['detectedOutputs']).toBe(2);
|
||||
expect(stats.counters['problematicOutputs']).toBe(0);
|
||||
expect(stats.counters['successRate']).toBe(1);
|
||||
expect(stats['detectedOutputs']).toBe(2);
|
||||
expect(stats['problematicOutputs']).toBe(0);
|
||||
expect(stats['successRate']).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -9,7 +9,6 @@
|
||||
import ts from 'typescript';
|
||||
import {
|
||||
confirmAsSerializable,
|
||||
MigrationStats,
|
||||
ProgramInfo,
|
||||
projectFile,
|
||||
ProjectFile,
|
||||
@@ -384,7 +383,7 @@ export class OutputMigration extends TsurgeFunnelMigration<
|
||||
return confirmAsSerializable(combinedData);
|
||||
}
|
||||
|
||||
override async stats(globalMetadata: CompilationUnitData): Promise<MigrationStats> {
|
||||
override async stats(globalMetadata: CompilationUnitData) {
|
||||
const detectedOutputs =
|
||||
new Set(Object.keys(globalMetadata.outputFields)).size +
|
||||
globalMetadata.problematicDeclarationCount;
|
||||
@@ -395,13 +394,11 @@ export class OutputMigration extends TsurgeFunnelMigration<
|
||||
const successRate =
|
||||
detectedOutputs > 0 ? (detectedOutputs - problematicOutputs) / detectedOutputs : 1;
|
||||
|
||||
return {
|
||||
counters: {
|
||||
detectedOutputs,
|
||||
problematicOutputs,
|
||||
successRate,
|
||||
},
|
||||
};
|
||||
return confirmAsSerializable({
|
||||
detectedOutputs,
|
||||
problematicOutputs,
|
||||
successRate,
|
||||
});
|
||||
}
|
||||
|
||||
override async migrate(globalData: CompilationUnitData) {
|
||||
|
||||
+5
-10
@@ -9,7 +9,6 @@
|
||||
import ts from 'typescript';
|
||||
import {
|
||||
confirmAsSerializable,
|
||||
MigrationStats,
|
||||
ProgramInfo,
|
||||
projectFile,
|
||||
ProjectFile,
|
||||
@@ -128,21 +127,17 @@ export class SelfClosingTagsMigration extends TsurgeFunnelMigration<
|
||||
return confirmAsSerializable(globalMeta);
|
||||
}
|
||||
|
||||
override async stats(
|
||||
globalMetadata: SelfClosingTagsCompilationUnitData,
|
||||
): Promise<MigrationStats> {
|
||||
override async stats(globalMetadata: SelfClosingTagsCompilationUnitData) {
|
||||
const touchedFilesCount = globalMetadata.tagReplacements.length;
|
||||
const replacementCount = globalMetadata.tagReplacements.reduce(
|
||||
(acc, cur) => acc + cur.replacementCount,
|
||||
0,
|
||||
);
|
||||
|
||||
return {
|
||||
counters: {
|
||||
touchedFilesCount,
|
||||
replacementCount,
|
||||
},
|
||||
};
|
||||
return confirmAsSerializable({
|
||||
touchedFilesCount,
|
||||
replacementCount,
|
||||
});
|
||||
}
|
||||
|
||||
override async migrate(globalData: SelfClosingTagsCompilationUnitData) {
|
||||
|
||||
@@ -231,15 +231,13 @@ export class SignalInputMigration extends TsurgeComplexMigration<
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
counters: {
|
||||
fullCompilationInputs,
|
||||
sourceInputs,
|
||||
incompatibleInputs,
|
||||
...fieldIncompatibleCounts,
|
||||
...classIncompatibleCounts,
|
||||
},
|
||||
};
|
||||
return confirmAsSerializable({
|
||||
fullCompilationInputs,
|
||||
sourceInputs,
|
||||
incompatibleInputs,
|
||||
...fieldIncompatibleCounts,
|
||||
...classIncompatibleCounts,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1586,14 +1586,14 @@ describe('signal queries migration', () => {
|
||||
],
|
||||
);
|
||||
|
||||
expect(await getStatistics()).toEqual({
|
||||
counters: {
|
||||
queriesCount: 3,
|
||||
multiQueries: 2,
|
||||
incompatibleQueries: 2,
|
||||
'incompat-field-Accessor': 1,
|
||||
'incompat-field-WriteAssignment': 1,
|
||||
},
|
||||
// Cast as we dynamically add fields to the stats. This can be improved in follow-ups
|
||||
// when stats for this migration are leveraging more complex data structures.
|
||||
expect((await getStatistics()) as object).toEqual({
|
||||
queriesCount: 3,
|
||||
multiQueries: 2,
|
||||
incompatibleQueries: 2,
|
||||
'incompat-field-Accessor': 1,
|
||||
'incompat-field-WriteAssignment': 1,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -644,15 +644,13 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
counters: {
|
||||
queriesCount,
|
||||
multiQueries,
|
||||
incompatibleQueries,
|
||||
...fieldIncompatibleCounts,
|
||||
...classIncompatibleCounts,
|
||||
},
|
||||
};
|
||||
return confirmAsSerializable({
|
||||
queriesCount,
|
||||
multiQueries,
|
||||
incompatibleQueries,
|
||||
...fieldIncompatibleCounts,
|
||||
...classIncompatibleCounts,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -110,6 +110,6 @@ export class TestBedGetMigration extends TsurgeFunnelMigration<
|
||||
}
|
||||
|
||||
override async stats() {
|
||||
return {counters: {}};
|
||||
return confirmAsSerializable({});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,8 +28,7 @@ export function migrate(): Rule {
|
||||
afterAnalysisFailure: () => {
|
||||
context.logger.error('Schematic failed unexpectedly with no analysis data');
|
||||
},
|
||||
whenDone: (stats) => {
|
||||
const {removedImports, changedFiles} = stats.counters;
|
||||
whenDone: ({removedImports, changedFiles}) => {
|
||||
let statsMessage: string;
|
||||
|
||||
if (removedImports === 0) {
|
||||
|
||||
+5
-8
@@ -10,7 +10,6 @@ import ts from 'typescript';
|
||||
import {
|
||||
BaseProgramInfo,
|
||||
confirmAsSerializable,
|
||||
MigrationStats,
|
||||
ProgramInfo,
|
||||
projectFile,
|
||||
ProjectFileID,
|
||||
@@ -163,13 +162,11 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
|
||||
return confirmAsSerializable(combinedData);
|
||||
}
|
||||
|
||||
override async stats(globalMetadata: CompilationUnitData): Promise<MigrationStats> {
|
||||
return {
|
||||
counters: {
|
||||
removedImports: globalMetadata.removedIdentifiers.length,
|
||||
changedFiles: globalMetadata.changedFiles,
|
||||
},
|
||||
};
|
||||
override async stats(globalMetadata: CompilationUnitData) {
|
||||
return confirmAsSerializable({
|
||||
removedImports: globalMetadata.removedIdentifiers.length,
|
||||
changedFiles: globalMetadata.changedFiles,
|
||||
});
|
||||
}
|
||||
|
||||
/** Gets an ID that can be used to look up a node based on its location. */
|
||||
|
||||
@@ -57,8 +57,7 @@ export function migrate(options: Options): Rule {
|
||||
afterAnalysisFailure: () => {
|
||||
context.logger.error('Migration failed unexpectedly with no analysis data');
|
||||
},
|
||||
whenDone: ({counters}) => {
|
||||
const {detectedOutputs, problematicOutputs, successRate} = counters;
|
||||
whenDone: ({detectedOutputs, problematicOutputs, successRate}) => {
|
||||
const migratedOutputs = detectedOutputs - problematicOutputs;
|
||||
const successRatePercent = (successRate * 100).toFixed(2);
|
||||
|
||||
|
||||
@@ -46,8 +46,7 @@ export function migrate(options: Options): Rule {
|
||||
afterAnalysisFailure: () => {
|
||||
context.logger.error('Migration failed unexpectedly with no analysis data');
|
||||
},
|
||||
whenDone: ({counters}) => {
|
||||
const {touchedFilesCount, replacementCount} = counters;
|
||||
whenDone: ({touchedFilesCount, replacementCount}) => {
|
||||
context.logger.info('');
|
||||
context.logger.info(`Successfully migrated to self-closing tags 🎉`);
|
||||
context.logger.info(
|
||||
|
||||
@@ -61,8 +61,7 @@ export function migrate(options: Options): Rule {
|
||||
afterAnalysisFailure: () => {
|
||||
context.logger.error('Migration failed unexpectedly with no analysis data');
|
||||
},
|
||||
whenDone: ({counters}) => {
|
||||
const {sourceInputs, incompatibleInputs} = counters;
|
||||
whenDone: ({sourceInputs, incompatibleInputs}) => {
|
||||
const migratedInputs = sourceInputs - incompatibleInputs;
|
||||
|
||||
context.logger.info('');
|
||||
|
||||
@@ -61,11 +61,10 @@ export function migrate(options: Options): Rule {
|
||||
context.logger.info(`Processing analysis data between targets...`);
|
||||
context.logger.info(``);
|
||||
},
|
||||
whenDone: ({counters}) => {
|
||||
whenDone: ({queriesCount, incompatibleQueries}) => {
|
||||
context.logger.info('');
|
||||
context.logger.info(`Successfully migrated to signal queries 🎉`);
|
||||
|
||||
const {queriesCount, incompatibleQueries} = counters;
|
||||
const migratedQueries = queriesCount - incompatibleQueries;
|
||||
|
||||
context.logger.info('');
|
||||
|
||||
@@ -32,6 +32,7 @@ jasmine_node_test(
|
||||
"//packages/core/schematics/ng-generate/signals:static_files",
|
||||
"//packages/core/schematics/ng-generate/standalone-migration:static_files",
|
||||
],
|
||||
shard_count = 4,
|
||||
deps = [
|
||||
":test_lib",
|
||||
"@npm//shelljs",
|
||||
|
||||
@@ -14,15 +14,9 @@ import {BaseProgramInfo, ProgramInfo} from './program_info';
|
||||
import {Serializable} from './helpers/serializable';
|
||||
import {createBaseProgramInfo} from './helpers/create_program';
|
||||
|
||||
/**
|
||||
* Type describing statistics that could be tracked
|
||||
* by migrations.
|
||||
*
|
||||
* Statistics may be tracked depending on the runner.
|
||||
*/
|
||||
export interface MigrationStats {
|
||||
counters: Record<string, number>;
|
||||
}
|
||||
/** Type helper extracting the stats type of a migration. */
|
||||
export type MigrationStats<T> =
|
||||
T extends TsurgeBaseMigration<unknown, unknown, infer Stats> ? Stats : never;
|
||||
|
||||
/**
|
||||
* @private
|
||||
@@ -32,7 +26,12 @@ export interface MigrationStats {
|
||||
* For example, this class exposes methods to conveniently create
|
||||
* TypeScript programs, while also allowing migration authors to override.
|
||||
*/
|
||||
export abstract class TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMetadata> {
|
||||
export abstract class TsurgeBaseMigration<
|
||||
UnitAnalysisMetadata,
|
||||
CombinedGlobalMetadata,
|
||||
// Note: Even when optional, they can be inferred from implementations.
|
||||
Stats = unknown,
|
||||
> {
|
||||
/**
|
||||
* Advanced Tsurge users can override this method, but most of the time,
|
||||
* overriding {@link prepareProgram} is more desirable.
|
||||
@@ -103,5 +102,5 @@ export abstract class TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMe
|
||||
): Promise<Serializable<CombinedGlobalMetadata>>;
|
||||
|
||||
/** Extract statistics based on the global metadata. */
|
||||
abstract stats(globalMetadata: CombinedGlobalMetadata): Promise<MigrationStats>;
|
||||
abstract stats(globalMetadata: CombinedGlobalMetadata): Promise<Serializable<Stats>>;
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ import {Serializable} from '../helpers/serializable';
|
||||
* @returns the serializable migration unit data.
|
||||
*/
|
||||
export async function executeAnalyzePhase<UnitData, GlobalData>(
|
||||
migration: TsurgeMigration<UnitData, GlobalData>,
|
||||
migration: TsurgeMigration<UnitData, GlobalData, unknown>,
|
||||
tsconfigAbsolutePath: string,
|
||||
): Promise<Serializable<UnitData>> {
|
||||
const baseInfo = migration.createProgram(tsconfigAbsolutePath);
|
||||
|
||||
@@ -16,7 +16,7 @@ import {TsurgeMigration} from '../migration';
|
||||
* @returns the serializable combined unit data.
|
||||
*/
|
||||
export async function executeCombinePhase<UnitData, GlobalData>(
|
||||
migration: TsurgeMigration<UnitData, GlobalData>,
|
||||
migration: TsurgeMigration<UnitData, GlobalData, unknown>,
|
||||
unitA: UnitData,
|
||||
unitB: UnitData,
|
||||
): Promise<Serializable<UnitData>> {
|
||||
|
||||
@@ -16,7 +16,7 @@ import {TsurgeMigration} from '../migration';
|
||||
* @returns the serializable global meta.
|
||||
*/
|
||||
export async function executeGlobalMetaPhase<UnitData, GlobalData>(
|
||||
migration: TsurgeMigration<UnitData, GlobalData>,
|
||||
migration: TsurgeMigration<UnitData, GlobalData, unknown>,
|
||||
combinedUnitData: UnitData,
|
||||
): Promise<Serializable<GlobalData>> {
|
||||
return await migration.globalMeta(combinedUnitData);
|
||||
|
||||
@@ -21,7 +21,7 @@ import {Replacement} from '../replacement';
|
||||
* absolute project directory path (to allow for applying).
|
||||
*/
|
||||
export async function executeMigratePhase<UnitData, GlobalData>(
|
||||
migration: TsurgeMigration<UnitData, GlobalData>,
|
||||
migration: TsurgeMigration<UnitData, GlobalData, unknown>,
|
||||
globalMetadata: GlobalData,
|
||||
tsconfigAbsolutePath: string,
|
||||
): Promise<{replacements: Replacement[]; projectRoot: AbsoluteFsPath}> {
|
||||
|
||||
@@ -27,9 +27,9 @@ export enum MigrationStage {
|
||||
}
|
||||
|
||||
/** Information necessary to run a Tsurge migration in the devkit. */
|
||||
export interface TsurgeDevkitMigration {
|
||||
export interface TsurgeDevkitMigration<Stats> {
|
||||
/** Instantiates the migration. */
|
||||
getMigration: (fs: FileSystem) => TsurgeMigration<unknown, unknown>;
|
||||
getMigration: (fs: FileSystem) => TsurgeMigration<unknown, unknown, Stats>;
|
||||
|
||||
/** File tree of the schematic. */
|
||||
tree: Tree;
|
||||
@@ -53,11 +53,13 @@ export interface TsurgeDevkitMigration {
|
||||
afterAnalysisFailure?: () => void;
|
||||
|
||||
/** Called when the migration is done running and stats are available. Useful for logging. */
|
||||
whenDone?: (stats: MigrationStats) => void;
|
||||
whenDone?: (stats: Stats) => void;
|
||||
}
|
||||
|
||||
/** Runs a Tsurge within an Angular Devkit context. */
|
||||
export async function runMigrationInDevkit(config: TsurgeDevkitMigration): Promise<void> {
|
||||
export async function runMigrationInDevkit<Stats>(
|
||||
config: TsurgeDevkitMigration<Stats>,
|
||||
): Promise<void> {
|
||||
const {buildPaths, testPaths} = await getProjectTsConfigPaths(config.tree);
|
||||
|
||||
if (!buildPaths.length && !testPaths.length) {
|
||||
|
||||
@@ -16,7 +16,7 @@ import {TsurgeMigration} from '../migration';
|
||||
* prefer parallel execution of combining via e.g. Beam combiners.
|
||||
*/
|
||||
export async function synchronouslyCombineUnitData<UnitData>(
|
||||
migration: TsurgeMigration<UnitData, unknown>,
|
||||
migration: TsurgeMigration<UnitData, unknown, unknown>,
|
||||
unitDatas: UnitData[],
|
||||
): Promise<UnitData | null> {
|
||||
if (unitDatas.length === 0) {
|
||||
|
||||
@@ -39,9 +39,9 @@ interface MigrateResult {
|
||||
*
|
||||
* TODO: Link design doc
|
||||
*/
|
||||
export type TsurgeMigration<UnitAnalysisMetadata, CombinedGlobalMetadata> =
|
||||
| TsurgeComplexMigration<UnitAnalysisMetadata, CombinedGlobalMetadata>
|
||||
| TsurgeFunnelMigration<UnitAnalysisMetadata, CombinedGlobalMetadata>;
|
||||
export type TsurgeMigration<UnitAnalysisMetadata, CombinedGlobalMetadata, Stats> =
|
||||
| TsurgeComplexMigration<UnitAnalysisMetadata, CombinedGlobalMetadata, Stats>
|
||||
| TsurgeFunnelMigration<UnitAnalysisMetadata, CombinedGlobalMetadata, Stats>;
|
||||
|
||||
/**
|
||||
* A simpler variant of a {@link TsurgeComplexMigration} that does not
|
||||
@@ -58,7 +58,8 @@ export type TsurgeMigration<UnitAnalysisMetadata, CombinedGlobalMetadata> =
|
||||
export abstract class TsurgeFunnelMigration<
|
||||
UnitAnalysisMetadata,
|
||||
CombinedGlobalMetadata,
|
||||
> extends TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMetadata> {
|
||||
Stats = unknown,
|
||||
> extends TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMetadata, Stats> {
|
||||
/**
|
||||
* Finalizes the migration result.
|
||||
*
|
||||
@@ -83,7 +84,8 @@ export abstract class TsurgeFunnelMigration<
|
||||
export abstract class TsurgeComplexMigration<
|
||||
UnitAnalysisMetadata,
|
||||
CombinedGlobalMetadata,
|
||||
> extends TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMetadata> {
|
||||
Stats = unknown,
|
||||
> extends TsurgeBaseMigration<UnitAnalysisMetadata, CombinedGlobalMetadata, Stats> {
|
||||
/**
|
||||
* Migration phase. Workers will be started for every compilation unit again,
|
||||
* instantiating a new program for every unit to compute the final migration
|
||||
|
||||
@@ -97,10 +97,8 @@ describe('output migration', () => {
|
||||
]);
|
||||
|
||||
expect(await getStatistics()).toEqual({
|
||||
counters: {
|
||||
allOutputs: 2,
|
||||
migratedOutputs: 1,
|
||||
},
|
||||
allOutputs: 2,
|
||||
migratedOutputs: 1,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,7 +14,6 @@ import {ProgramInfo} from '../program_info';
|
||||
import {Replacement, TextUpdate} from '../replacement';
|
||||
import {findOutputDeclarationsAndReferences, OutputID} from './output_helpers';
|
||||
import {projectFile} from '../project_paths';
|
||||
import {MigrationStats} from '../base_migration';
|
||||
|
||||
type AnalysisUnit = {[id: OutputID]: {seenProblematicUsage: boolean}};
|
||||
type GlobalMetadata = {[id: OutputID]: {canBeMigrated: boolean}};
|
||||
@@ -120,7 +119,7 @@ export class OutputMigration extends TsurgeComplexMigration<AnalysisUnit, Global
|
||||
return {replacements};
|
||||
}
|
||||
|
||||
override async stats(globalMetadata: GlobalMetadata): Promise<MigrationStats> {
|
||||
override async stats(globalMetadata: GlobalMetadata) {
|
||||
let allOutputs = 0;
|
||||
let migratedOutputs = 0;
|
||||
|
||||
@@ -131,11 +130,9 @@ export class OutputMigration extends TsurgeComplexMigration<AnalysisUnit, Global
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
counters: {
|
||||
allOutputs,
|
||||
migratedOutputs,
|
||||
},
|
||||
};
|
||||
return confirmAsSerializable({
|
||||
allOutputs,
|
||||
migratedOutputs,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,11 +29,11 @@ import {TestRun} from './test_run';
|
||||
*
|
||||
* @returns a mock file system with the applied replacements of the migration.
|
||||
*/
|
||||
export async function runTsurgeMigration<UnitData, GlobalData>(
|
||||
migration: TsurgeMigration<UnitData, GlobalData>,
|
||||
export async function runTsurgeMigration<Stats>(
|
||||
migration: TsurgeMigration<unknown, unknown, Stats>,
|
||||
files: {name: AbsoluteFsPath; contents: string; isProgramRootFile?: boolean}[],
|
||||
compilerOptions: ts.CompilerOptions = {},
|
||||
): Promise<TestRun> {
|
||||
): Promise<TestRun<Stats>> {
|
||||
const mockFs = getFileSystem();
|
||||
if (!(mockFs instanceof MockFileSystem)) {
|
||||
throw new Error('Expected a mock file system for `runTsurgeMigration`.');
|
||||
|
||||
@@ -7,12 +7,11 @@
|
||||
*/
|
||||
|
||||
import {MockFileSystem} from '../../../../../compiler-cli/src/ngtsc/file_system/testing';
|
||||
import {MigrationStats} from '../base_migration';
|
||||
|
||||
/** Type describing results of a Tsurge migration test run. */
|
||||
export interface TestRun {
|
||||
export interface TestRun<Stats> {
|
||||
/** File system that can be used to read migrated file contents. */
|
||||
fs: MockFileSystem;
|
||||
/** Function that can be invoked to compute migration statistics. */
|
||||
getStatistics: () => Promise<MigrationStats>;
|
||||
getStatistics: () => Promise<Stats>;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user