fix(migrations): preserve optional parameters (#57367)

Makes it so the inject migration preserves the optional token when declaring a parameter. This came up in some testing as something that can be potentially breaking for classes that implement interfaces.

PR Close #57367
This commit is contained in:
Kristiyan Kostadinov
2024-08-13 08:05:40 +02:00
committed by Andrew Kushnir
parent 84827d5958
commit d6b24833ca
2 changed files with 76 additions and 5 deletions
@@ -321,12 +321,17 @@ function migrateParameter(
// If the parameter declares a property, we need to declare it (e.g. `private foo: Foo`).
if (declaresProp) {
const prop = ts.factory.createPropertyDeclaration(
node.modifiers?.filter((modifier) => {
// Strip out the DI decorators, as well as `public` which is redundant.
return !ts.isDecorator(modifier) && modifier.kind !== ts.SyntaxKind.PublicKeyword;
}),
cloneModifiers(
node.modifiers?.filter((modifier) => {
// Strip out the DI decorators, as well as `public` which is redundant.
return !ts.isDecorator(modifier) && modifier.kind !== ts.SyntaxKind.PublicKeyword;
}),
),
name,
undefined,
// Don't add the question token to private properties since it won't affect interface implementation.
node.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.PrivateKeyword)
? undefined
: node.questionToken,
// We can't initialize the property if it's referenced within a `super` call.
// See the logic further below for the initialization.
usedInSuper ? node.type : undefined,
@@ -610,3 +615,15 @@ function replaceNodePlaceholder(
const result = printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);
return result.replace(PLACEHOLDER, replacement);
}
/**
* Clones an optional array of modifiers. Can be useful to
* strip the comments from a node with modifiers.
*/
function cloneModifiers(modifiers: ts.ModifierLike[] | ts.NodeArray<ts.ModifierLike> | undefined) {
return modifiers?.map((modifier) => {
return ts.isDecorator(modifier)
? ts.factory.createDecorator(modifier.expression)
: ts.factory.createModifier(modifier.kind);
});
}
@@ -1341,6 +1341,60 @@ describe('inject migration', () => {
]);
});
it('should mark optional members if they correspond to optional parameters', async () => {
writeFile(
'/dir.ts',
[
`import { Directive, Optional } from '@angular/core';`,
`import { Foo } from 'foo';`,
``,
`@Directive()`,
`class MyDir {`,
` constructor(@Optional() public foo?: Foo) {}`,
`}`,
].join('\n'),
);
await runMigration();
expect(tree.readContent('/dir.ts').split('\n')).toEqual([
`import { Directive, inject } from '@angular/core';`,
`import { Foo } from 'foo';`,
``,
`@Directive()`,
`class MyDir {`,
` foo? = inject(Foo, { optional: true });`,
`}`,
]);
});
it('should not mark private members as optional', async () => {
writeFile(
'/dir.ts',
[
`import { Directive, Optional } from '@angular/core';`,
`import { Foo } from 'foo';`,
``,
`@Directive()`,
`class MyDir {`,
` constructor(@Optional() private foo?: Foo) {}`,
`}`,
].join('\n'),
);
await runMigration();
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, { optional: true });`,
`}`,
]);
});
describe('internal-only behavior', () => {
function runInternalMigration() {
return runMigration({_internalCombineMemberInitializers: true});