Skip to content

Commit 39a4e00

Browse files
aparziatscott
authored andcommitted
fix(core): fix ng generate @angular/core:output-migration. Fixes angular#58650 (angular#60763)
Fixes angular#58650 - Insert a TODO comment for empty emit (without parameter). PR Close angular#60763
1 parent 9bba182 commit 39a4e00

File tree

2 files changed

+136
-0
lines changed

2 files changed

+136
-0
lines changed

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,65 @@ describe('outputs', () => {
169169
});
170170
});
171171

172+
it('should not insert a TODO comment for emit function with no type', async () => {
173+
await verify({
174+
before: `
175+
import {Directive, Output, EventEmitter} from '@angular/core';
176+
177+
@Directive()
178+
export class TestDir {
179+
@Output() someChange = new EventEmitter();
180+
181+
someMethod(): void {
182+
this.someChange.emit();
183+
}
184+
}
185+
`,
186+
after: `
187+
import {Directive, output} from '@angular/core';
188+
189+
@Directive()
190+
export class TestDir {
191+
readonly someChange = output();
192+
193+
someMethod(): void {
194+
this.someChange.emit();
195+
}
196+
}
197+
`,
198+
});
199+
});
200+
201+
it('should insert a TODO comment for emit function with type', async () => {
202+
await verify({
203+
before: `
204+
import {Directive, Output, EventEmitter} from '@angular/core';
205+
206+
@Directive()
207+
export class TestDir {
208+
@Output() someChange = new EventEmitter<string>();
209+
210+
someMethod(): void {
211+
this.someChange.emit();
212+
}
213+
}
214+
`,
215+
after: `
216+
import {Directive, output} from '@angular/core';
217+
218+
@Directive()
219+
export class TestDir {
220+
readonly someChange = output<string>();
221+
222+
someMethod(): void {
223+
// TODO: The 'emit' function requires a mandatory string argument
224+
this.someChange.emit();
225+
}
226+
}
227+
`,
228+
});
229+
});
230+
172231
it('should migrate multiple outputs', async () => {
173232
await verifyDeclaration({
174233
before:

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
ProjectFileID,
1717
Replacement,
1818
Serializable,
19+
TextUpdate,
1920
TsurgeFunnelMigration,
2021
} from '../../utils/tsurge';
2122

@@ -216,6 +217,8 @@ export class OutputMigration extends TsurgeFunnelMigration<
216217
}
217218
}
218219

220+
addCommentForEmptyEmit(node, info, checker, reflector, dtsReader, outputFieldReplacements);
221+
219222
// detect imports of test runners
220223
if (isTestRunnerImport(node)) {
221224
isTestFile = true;
@@ -449,3 +452,77 @@ function addOutputReplacement(
449452
}
450453
existingReplacements.replacements.push(...replacements);
451454
}
455+
456+
function addCommentForEmptyEmit(
457+
node: ts.Node,
458+
info: ProgramInfo,
459+
checker: ts.TypeChecker,
460+
reflector: TypeScriptReflectionHost,
461+
dtsReader: DtsMetadataReader,
462+
outputFieldReplacements: Record<ClassFieldUniqueKey, OutputMigrationData>,
463+
): void {
464+
if (!isEmptyEmitCall(node)) return;
465+
466+
const propertyAccess = getPropertyAccess(node);
467+
if (!propertyAccess) return;
468+
469+
const symbol = checker.getSymbolAtLocation(propertyAccess.name);
470+
if (!symbol || !symbol.declarations?.length) return;
471+
472+
const propertyDeclaration = isTargetOutputDeclaration(
473+
propertyAccess,
474+
checker,
475+
reflector,
476+
dtsReader,
477+
);
478+
if (!propertyDeclaration) return;
479+
480+
const eventEmitterType = getEventEmitterArgumentType(propertyDeclaration);
481+
if (!eventEmitterType) return;
482+
483+
const id = getUniqueIdForProperty(info, propertyDeclaration);
484+
const file = projectFile(node.getSourceFile(), info);
485+
const formatter = getFormatterText(node);
486+
const todoReplacement: TextUpdate = new TextUpdate({
487+
toInsert: `${formatter.indent}// TODO: The 'emit' function requires a mandatory ${eventEmitterType} argument\n`,
488+
end: formatter.lineStartPos,
489+
position: formatter.lineStartPos,
490+
});
491+
492+
addOutputReplacement(outputFieldReplacements, id, file, new Replacement(file, todoReplacement));
493+
}
494+
495+
function isEmptyEmitCall(node: ts.Node): node is ts.CallExpression {
496+
return (
497+
ts.isCallExpression(node) &&
498+
ts.isPropertyAccessExpression(node.expression) &&
499+
node.expression.name.text === 'emit' &&
500+
node.arguments.length === 0
501+
);
502+
}
503+
504+
function getPropertyAccess(node: ts.CallExpression): ts.PropertyAccessExpression | null {
505+
const propertyAccessExpression = (node.expression as ts.PropertyAccessExpression).expression;
506+
return ts.isPropertyAccessExpression(propertyAccessExpression) ? propertyAccessExpression : null;
507+
}
508+
509+
function getEventEmitterArgumentType(propertyDeclaration: ts.PropertyDeclaration): string | null {
510+
const initializer = propertyDeclaration.initializer;
511+
if (!initializer || !ts.isNewExpression(initializer)) return null;
512+
513+
const isEventEmitter =
514+
ts.isIdentifier(initializer.expression) && initializer.expression.getText() === 'EventEmitter';
515+
516+
if (!isEventEmitter) return null;
517+
518+
const [typeArg] = initializer.typeArguments ?? [];
519+
return typeArg ? typeArg.getText() : null;
520+
}
521+
522+
function getFormatterText(node: ts.Node): {indent: string; lineStartPos: number} {
523+
const sourceFile = node.getSourceFile();
524+
const {line} = sourceFile.getLineAndCharacterOfPosition(node.getStart());
525+
const lineStartPos = sourceFile.getPositionOfLineAndCharacter(line, 0);
526+
const indent = sourceFile.text.slice(lineStartPos, node.getStart());
527+
return {indent, lineStartPos};
528+
}

0 commit comments

Comments
 (0)