Skip to content

Commit 9da21f7

Browse files
pkozlowski-opensourcethePunderWoman
pkozlowski-opensource
authored and
thePunderWoman
committed
feat(migrations): replace .next usage on outputs (#57654)
This commits extends the decorator-based output migration to replace .next usage (not supported with the output function) with .emit (supported in both decorator-based and function based outputs). PR Close #57654
1 parent 71f5ef2 commit 9da21f7

File tree

4 files changed

+163
-39
lines changed

4 files changed

+163
-39
lines changed

‎packages/core/schematics/migrations/output-migration/output-migration.spec.ts‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,32 +106,69 @@ describe('outputs', () => {
106106
});
107107
});
108108

109-
describe('declarations _with_ problematic access patterns', () => {
110-
it('should _not_ migrate outputs that are used with .pipe', () => {
111-
verifyNoChange(`
109+
describe('.next migration', () => {
110+
it('should migrate .next usages that should have been .emit', () => {
111+
verify({
112+
before: `
112113
import {Directive, Output, EventEmitter} from '@angular/core';
113114
114115
@Directive()
115116
export class TestDir {
116-
@Output() someChange = new EventEmitter();
117+
@Output() someChange = new EventEmitter<string>();
117118
118-
someMethod() {
119-
this.someChange.pipe();
119+
onClick() {
120+
this.someChange.next('clicked');
120121
}
121122
}
122-
`);
123+
`,
124+
after: `
125+
import { Directive, output } from '@angular/core';
126+
127+
@Directive()
128+
export class TestDir {
129+
readonly someChange = output<string>();
130+
131+
onClick() {
132+
this.someChange.emit('clicked');
133+
}
134+
}
135+
`,
136+
});
123137
});
124138

125-
it('should _not_ migrate outputs that are used with .next', () => {
126-
verifyNoChange(`
139+
it('should _not_ migrate .next usages when problematic output usage is detected', () => {
140+
verifyNoChange(
141+
`
127142
import {Directive, Output, EventEmitter} from '@angular/core';
128143
129144
@Directive()
130145
export class TestDir {
131146
@Output() someChange = new EventEmitter<string>();
132147
148+
onClick() {
149+
this.someChange.next('clicked');
150+
}
151+
152+
ngOnDestroy() {
153+
this.someChange.complete();
154+
}
155+
}
156+
`,
157+
);
158+
});
159+
});
160+
161+
describe('declarations _with_ problematic access patterns', () => {
162+
it('should _not_ migrate outputs that are used with .pipe', () => {
163+
verifyNoChange(`
164+
import {Directive, Output, EventEmitter} from '@angular/core';
165+
166+
@Directive()
167+
export class TestDir {
168+
@Output() someChange = new EventEmitter();
169+
133170
someMethod() {
134-
this.someChange.next('payload');
171+
this.someChange.pipe();
135172
}
136173
}
137174
`);

‎packages/core/schematics/migrations/output-migration/output-migration.ts‎

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,18 @@ import {
2020
import {DtsMetadataReader} from '../../../../compiler-cli/src/ngtsc/metadata';
2121
import {TypeScriptReflectionHost} from '../../../../compiler-cli/src/ngtsc/reflection';
2222
import {
23-
isOutputDeclaration,
2423
OutputID,
2524
getUniqueIdForProperty,
26-
getTargetPropertyDeclaration,
25+
isTargetOutputDeclaration,
2726
extractSourceOutputDefinition,
28-
isProblematicEventEmitterUsage,
27+
isPotentialProblematicEventEmitterUsage,
28+
isPotentialNextCallUsage,
2929
} from './output_helpers';
30-
import {calculateImportReplacements, calculateDeclarationReplacements} from './output-replacements';
30+
import {
31+
calculateImportReplacements,
32+
calculateDeclarationReplacements,
33+
calculateNextFnReplacement,
34+
} from './output-replacements';
3135

3236
interface OutputMigrationData {
3337
path: ProjectRelativePath;
@@ -52,7 +56,7 @@ export class OutputMigration extends TsurgeFunnelMigration<
5256
program,
5357
projectDirAbsPath,
5458
}: ProgramInfo): Promise<Serializable<CompilationUnitData>> {
55-
const outputFields: Record<OutputID, OutputMigrationData> = {};
59+
const outputFieldReplacements: Record<OutputID, OutputMigrationData> = {};
5660
const problematicUsages: Record<OutputID, true> = {};
5761

5862
const filesWithOutputDeclarations = new Set<ProjectRelativePath>();
@@ -69,29 +73,43 @@ export class OutputMigration extends TsurgeFunnelMigration<
6973
const relativePath = projectRelativePath(node.getSourceFile(), projectDirAbsPath);
7074

7175
filesWithOutputDeclarations.add(relativePath);
72-
outputFields[outputDef.id] = {
73-
path: relativePath,
74-
replacements: calculateDeclarationReplacements(
75-
projectDirAbsPath,
76-
node,
77-
outputDef.aliasParam,
78-
),
79-
};
76+
addOutputReplacements(
77+
outputFieldReplacements,
78+
outputDef.id,
79+
relativePath,
80+
calculateDeclarationReplacements(projectDirAbsPath, node, outputDef.aliasParam),
81+
);
82+
}
83+
}
84+
85+
// detect .next usages that should be migrated to .emit
86+
if (isPotentialNextCallUsage(node) && ts.isPropertyAccessExpression(node.expression)) {
87+
const propertyDeclaration = isTargetOutputDeclaration(
88+
node.expression.expression,
89+
checker,
90+
reflector,
91+
dtsReader,
92+
);
93+
if (propertyDeclaration !== null) {
94+
const id = getUniqueIdForProperty(projectDirAbsPath, propertyDeclaration);
95+
const relativePath = projectRelativePath(node.getSourceFile(), projectDirAbsPath);
96+
addOutputReplacements(outputFieldReplacements, id, relativePath, [
97+
calculateNextFnReplacement(projectDirAbsPath, node.expression.name),
98+
]);
8099
}
81100
}
82101

83102
// detect unsafe access of the output property
84-
if (isProblematicEventEmitterUsage(node)) {
85-
const targetSymbol = checker.getSymbolAtLocation(node.expression);
86-
if (targetSymbol !== undefined) {
87-
const propertyDeclaration = getTargetPropertyDeclaration(targetSymbol);
88-
if (
89-
propertyDeclaration !== null &&
90-
isOutputDeclaration(propertyDeclaration, reflector, dtsReader)
91-
) {
92-
const id = getUniqueIdForProperty(projectDirAbsPath, propertyDeclaration);
93-
problematicUsages[id] = true;
94-
}
103+
if (isPotentialProblematicEventEmitterUsage(node)) {
104+
const propertyDeclaration = isTargetOutputDeclaration(
105+
node.expression,
106+
checker,
107+
reflector,
108+
dtsReader,
109+
);
110+
if (propertyDeclaration !== null) {
111+
const id = getUniqueIdForProperty(projectDirAbsPath, propertyDeclaration);
112+
problematicUsages[id] = true;
95113
}
96114
}
97115

@@ -112,7 +130,7 @@ export class OutputMigration extends TsurgeFunnelMigration<
112130
);
113131

114132
return confirmAsSerializable({
115-
outputFields,
133+
outputFields: outputFieldReplacements,
116134
importReplacements,
117135
problematicUsages,
118136
});
@@ -183,3 +201,20 @@ export class OutputMigration extends TsurgeFunnelMigration<
183201
return replacements;
184202
}
185203
}
204+
205+
function addOutputReplacements(
206+
outputFieldReplacements: Record<OutputID, OutputMigrationData>,
207+
outputId: OutputID,
208+
relativePath: ProjectRelativePath,
209+
replacements: Replacement[],
210+
): void {
211+
const existingReplacements = outputFieldReplacements[outputId];
212+
if (existingReplacements !== undefined) {
213+
existingReplacements.replacements.push(...replacements);
214+
} else {
215+
outputFieldReplacements[outputId] = {
216+
path: relativePath,
217+
replacements: replacements,
218+
};
219+
}
220+
}

‎packages/core/schematics/migrations/output-migration/output-replacements.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,18 @@ export function calculateImportReplacements(
9999

100100
return importReplacements;
101101
}
102+
103+
export function calculateNextFnReplacement(
104+
projectDirAbsPath: AbsoluteFsPath,
105+
node: ts.MemberName,
106+
): Replacement {
107+
const sf = node.getSourceFile();
108+
return new Replacement(
109+
projectRelativePath(sf, projectDirAbsPath),
110+
new TextUpdate({
111+
position: node.getStart(),
112+
end: node.getEnd(),
113+
toInsert: 'emit',
114+
}),
115+
);
116+
}

‎packages/core/schematics/migrations/output-migration/output_helpers.ts‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export interface ExtractedOutput {
2828
aliasParam?: ts.Expression;
2929
}
3030

31+
const PROBLEMATIC_OUTPUT_USAGES = new Set(['complete', 'pipe']);
32+
3133
/**
3234
* Determines if the given node refers to a decorator-based output, and
3335
* returns its resolved metadata if possible.
@@ -58,17 +60,52 @@ function isOutputDeclarationEligibleForMigration(node: ts.PropertyDeclaration) {
5860
);
5961
}
6062

61-
const problematicEventEmitterUsages = new Set(['pipe', 'next', 'complete']);
62-
export function isProblematicEventEmitterUsage(node: ts.Node): node is ts.PropertyAccessExpression {
63+
export function isPotentialProblematicEventEmitterUsage(
64+
node: ts.Node,
65+
): node is ts.PropertyAccessExpression {
6366
return (
6467
ts.isPropertyAccessExpression(node) &&
6568
ts.isIdentifier(node.name) &&
66-
problematicEventEmitterUsages.has(node.name.text)
69+
PROBLEMATIC_OUTPUT_USAGES.has(node.name.text)
6770
);
6871
}
6972

73+
export function isPotentialNextCallUsage(node: ts.Node): node is ts.CallExpression {
74+
if (
75+
ts.isCallExpression(node) &&
76+
ts.isPropertyAccessExpression(node.expression) &&
77+
ts.isIdentifier(node.expression.name)
78+
) {
79+
const methodName = node.expression.name.text;
80+
if (methodName === 'next') {
81+
return true;
82+
}
83+
}
84+
85+
return false;
86+
}
87+
88+
export function isTargetOutputDeclaration(
89+
node: ts.Node,
90+
checker: ts.TypeChecker,
91+
reflector: ReflectionHost,
92+
dtsReader: DtsMetadataReader,
93+
): ts.PropertyDeclaration | null {
94+
const targetSymbol = checker.getSymbolAtLocation(node);
95+
if (targetSymbol !== undefined) {
96+
const propertyDeclaration = getTargetPropertyDeclaration(targetSymbol);
97+
if (
98+
propertyDeclaration !== null &&
99+
isOutputDeclaration(propertyDeclaration, reflector, dtsReader)
100+
) {
101+
return propertyDeclaration;
102+
}
103+
}
104+
return null;
105+
}
106+
70107
/** Gets whether the given property is an Angular `@Output`. */
71-
export function isOutputDeclaration(
108+
function isOutputDeclaration(
72109
node: ts.PropertyDeclaration,
73110
reflector: ReflectionHost,
74111
dtsReader: DtsMetadataReader,

0 commit comments

Comments
 (0)