From 0cd7d3bdf0a9c0aeb514998a248e913fe49b5410 Mon Sep 17 00:00:00 2001 From: Ryan Russell Date: Thu, 27 Mar 2025 15:49:08 +0000 Subject: [PATCH] fix(core): preserve comments in internal inject migration (#60588) The internal-only combineMemberInitializers option for the inject migration sometimes dropped the doc comments from the members. PR Close #60588 --- .../ng-generate/inject-migration/migration.ts | 26 ++++++++---- .../schematics/test/inject_migration_spec.ts | 42 +++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/packages/core/schematics/ng-generate/inject-migration/migration.ts b/packages/core/schematics/ng-generate/inject-migration/migration.ts index e0b5d8a23e0..e1cf82f570f 100644 --- a/packages/core/schematics/ng-generate/inject-migration/migration.ts +++ b/packages/core/schematics/ng-generate/inject-migration/migration.ts @@ -773,17 +773,19 @@ function applyInternalOnlyChanges( result.toCombine.forEach(({declaration, initializer}) => { const initializerStatement = closestNode(initializer, ts.isStatement); - const newProperty = ts.factory.createPropertyDeclaration( - cloneModifiers(declaration.modifiers), - cloneName(declaration.name), - declaration.questionToken, - declaration.type, - initializer, - ); // If the initialization order is being preserved, we have to remove the original // declaration and re-declare it. Otherwise we can do the replacement in-place. if (preserveInitOrder) { + // Preserve comment in the new property since we are removing the entire node. + const newProperty = ts.factory.createPropertyDeclaration( + declaration.modifiers, + declaration.name, + declaration.questionToken, + declaration.type, + initializer, + ); + tracker.removeNode(declaration, true); removedMembers.add(declaration); afterInjectCalls.push( @@ -791,6 +793,16 @@ function applyInternalOnlyChanges( printer.printNode(ts.EmitHint.Unspecified, newProperty, declaration.getSourceFile()), ); } else { + // Strip comments from the declaration since we are replacing just + // the node, not the leading comment. + const newProperty = ts.factory.createPropertyDeclaration( + cloneModifiers(declaration.modifiers), + cloneName(declaration.name), + declaration.questionToken, + declaration.type, + initializer, + ); + tracker.replaceNode(declaration, newProperty); } diff --git a/packages/core/schematics/test/inject_migration_spec.ts b/packages/core/schematics/test/inject_migration_spec.ts index b75b53659f9..dbe58e99b81 100644 --- a/packages/core/schematics/test/inject_migration_spec.ts +++ b/packages/core/schematics/test/inject_migration_spec.ts @@ -2221,6 +2221,48 @@ describe('inject migration', () => { ]); }); + it('should account for doc strings when inlining initializers and combining in initialization order', async () => { + writeFile( + '/dir.ts', + [ + `import { Directive } from '@angular/core';`, + `import { Foo } from 'foo';`, + ``, + `@Directive()`, + `class MyDir {`, + ` /** Value of Foo */`, + ` private readonly value: number;`, + ``, + ` /** ID of Foo */`, + ` id: string;`, + ``, + ` constructor(private foo: Foo) {`, + ` this.value = this.foo.getValue();`, + ` this.id = this.value.toString();`, + ` }`, + `}`, + ].join('\n'), + ); + + await runInternalMigration(); + + expect(tree.readContent('/dir.ts').split('\n')).toEqual([ + `import { Directive, inject } from '@angular/core';`, + `import { Foo } from 'foo';`, + ``, + `@Directive()`, + `class MyDir {`, + ` private foo = inject(Foo);`, + // The indentation of the members here is slightly off, + // but it's not a problem because this code is internal-only. + ` /** Value of Foo */`, + `private readonly value: number = this.foo.getValue();`, + ` /** ID of Foo */`, + `id: string = this.value.toString();`, + `}`, + ]); + }); + it('should hoist property declarations that were not combined above the inject() calls', async () => { writeFile( '/dir.ts',