Skip to content

Commit 0cd7d3b

Browse files
Ryan RussellthePunderWoman
authored andcommitted
fix(core): preserve comments in internal inject migration (angular#60588)
The internal-only combineMemberInitializers option for the inject migration sometimes dropped the doc comments from the members. PR Close angular#60588
1 parent 76c60a6 commit 0cd7d3b

File tree

2 files changed

+61
-7
lines changed

2 files changed

+61
-7
lines changed

packages/core/schematics/ng-generate/inject-migration/migration.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -773,24 +773,36 @@ function applyInternalOnlyChanges(
773773

774774
result.toCombine.forEach(({declaration, initializer}) => {
775775
const initializerStatement = closestNode(initializer, ts.isStatement);
776-
const newProperty = ts.factory.createPropertyDeclaration(
777-
cloneModifiers(declaration.modifiers),
778-
cloneName(declaration.name),
779-
declaration.questionToken,
780-
declaration.type,
781-
initializer,
782-
);
783776

784777
// If the initialization order is being preserved, we have to remove the original
785778
// declaration and re-declare it. Otherwise we can do the replacement in-place.
786779
if (preserveInitOrder) {
780+
// Preserve comment in the new property since we are removing the entire node.
781+
const newProperty = ts.factory.createPropertyDeclaration(
782+
declaration.modifiers,
783+
declaration.name,
784+
declaration.questionToken,
785+
declaration.type,
786+
initializer,
787+
);
788+
787789
tracker.removeNode(declaration, true);
788790
removedMembers.add(declaration);
789791
afterInjectCalls.push(
790792
memberIndentation +
791793
printer.printNode(ts.EmitHint.Unspecified, newProperty, declaration.getSourceFile()),
792794
);
793795
} else {
796+
// Strip comments from the declaration since we are replacing just
797+
// the node, not the leading comment.
798+
const newProperty = ts.factory.createPropertyDeclaration(
799+
cloneModifiers(declaration.modifiers),
800+
cloneName(declaration.name),
801+
declaration.questionToken,
802+
declaration.type,
803+
initializer,
804+
);
805+
794806
tracker.replaceNode(declaration, newProperty);
795807
}
796808

packages/core/schematics/test/inject_migration_spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2221,6 +2221,48 @@ describe('inject migration', () => {
22212221
]);
22222222
});
22232223

2224+
it('should account for doc strings when inlining initializers and combining in initialization order', async () => {
2225+
writeFile(
2226+
'/dir.ts',
2227+
[
2228+
`import { Directive } from '@angular/core';`,
2229+
`import { Foo } from 'foo';`,
2230+
``,
2231+
`@Directive()`,
2232+
`class MyDir {`,
2233+
` /** Value of Foo */`,
2234+
` private readonly value: number;`,
2235+
``,
2236+
` /** ID of Foo */`,
2237+
` id: string;`,
2238+
``,
2239+
` constructor(private foo: Foo) {`,
2240+
` this.value = this.foo.getValue();`,
2241+
` this.id = this.value.toString();`,
2242+
` }`,
2243+
`}`,
2244+
].join('\n'),
2245+
);
2246+
2247+
await runInternalMigration();
2248+
2249+
expect(tree.readContent('/dir.ts').split('\n')).toEqual([
2250+
`import { Directive, inject } from '@angular/core';`,
2251+
`import { Foo } from 'foo';`,
2252+
``,
2253+
`@Directive()`,
2254+
`class MyDir {`,
2255+
` private foo = inject(Foo);`,
2256+
// The indentation of the members here is slightly off,
2257+
// but it's not a problem because this code is internal-only.
2258+
` /** Value of Foo */`,
2259+
`private readonly value: number = this.foo.getValue();`,
2260+
` /** ID of Foo */`,
2261+
`id: string = this.value.toString();`,
2262+
`}`,
2263+
]);
2264+
});
2265+
22242266
it('should hoist property declarations that were not combined above the inject() calls', async () => {
22252267
writeFile(
22262268
'/dir.ts',

0 commit comments

Comments
 (0)