Skip to content

Commit 0162ceb

Browse files
Ryan Russellpkozlowski-opensource
authored andcommitted
fix(core): inject migration should treat @Attribute as optional (angular#60916)
The @Attribute decorator will inject null if a host attribute is missing, but `inject(new HostAttributeToken(...))` will throw a no provider error. We should set {optional: true} when migrating an @Attribute decorator. Also allow nonNullableOptional to add `!` to those declarations. PR Close angular#60916
1 parent 037dede commit 0162ceb

File tree

2 files changed

+42
-8
lines changed

2 files changed

+42
-8
lines changed

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ function createInjectReplacementCall(
409409
const moduleName = '@angular/core';
410410
const sourceFile = param.getSourceFile();
411411
const decorators = getAngularDecorators(localTypeChecker, ts.getDecorators(param) || []);
412-
const literalProps: ts.ObjectLiteralElementLike[] = [];
412+
const literalProps = new Set<string>();
413413
const type = param.type;
414414
let injectedType = '';
415415
let typeArguments = type && hasGenerics(type) ? [type] : undefined;
@@ -451,24 +451,27 @@ function createInjectReplacementCall(
451451
const expression = ts.factory.createNewExpression(constructorRef, undefined, [firstArg]);
452452
injectedType = printer.printNode(ts.EmitHint.Unspecified, expression, sourceFile);
453453
typeArguments = undefined;
454+
// @Attribute is implicitly optional.
455+
hasOptionalDecorator = true;
456+
literalProps.add('optional');
454457
}
455458
break;
456459

457460
case 'Optional':
458461
hasOptionalDecorator = true;
459-
literalProps.push(ts.factory.createPropertyAssignment('optional', ts.factory.createTrue()));
462+
literalProps.add('optional');
460463
break;
461464

462465
case 'SkipSelf':
463-
literalProps.push(ts.factory.createPropertyAssignment('skipSelf', ts.factory.createTrue()));
466+
literalProps.add('skipSelf');
464467
break;
465468

466469
case 'Self':
467-
literalProps.push(ts.factory.createPropertyAssignment('self', ts.factory.createTrue()));
470+
literalProps.add('self');
468471
break;
469472

470473
case 'Host':
471-
literalProps.push(ts.factory.createPropertyAssignment('host', ts.factory.createTrue()));
474+
literalProps.add('host');
472475
break;
473476
}
474477
}
@@ -479,8 +482,14 @@ function createInjectReplacementCall(
479482
const injectRef = tracker.addImport(param.getSourceFile(), 'inject', moduleName);
480483
const args: ts.Expression[] = [ts.factory.createIdentifier(PLACEHOLDER)];
481484

482-
if (literalProps.length > 0) {
483-
args.push(ts.factory.createObjectLiteralExpression(literalProps));
485+
if (literalProps.size > 0) {
486+
args.push(
487+
ts.factory.createObjectLiteralExpression(
488+
Array.from(literalProps, (prop) =>
489+
ts.factory.createPropertyAssignment(prop, ts.factory.createTrue()),
490+
),
491+
),
492+
);
484493
}
485494

486495
let expression: ts.Expression = ts.factory.createCallExpression(injectRef, typeArguments, args);

packages/core/schematics/test/inject_migration_spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ describe('inject migration', () => {
194194
``,
195195
`@Directive()`,
196196
`class MyDir {`,
197-
` private foo = inject(new HostAttributeToken('foo'));`,
197+
` private foo = inject(new HostAttributeToken('foo'), { optional: true });`,
198198
`}`,
199199
]);
200200
});
@@ -1256,6 +1256,31 @@ describe('inject migration', () => {
12561256
]);
12571257
});
12581258

1259+
it('should add non-null assertion for @Attribute injections when enabled', async () => {
1260+
writeFile(
1261+
'/dir.ts',
1262+
[
1263+
`import { Attribute, Directive } from '@angular/core';`,
1264+
``,
1265+
`@Directive()`,
1266+
`class MyDir {`,
1267+
` constructor(@Attribute('tabindex') private foo: string) {}`,
1268+
`}`,
1269+
].join('\n'),
1270+
);
1271+
1272+
await runMigration({nonNullableOptional: true});
1273+
1274+
expect(tree.readContent('/dir.ts').split('\n')).toEqual([
1275+
`import { Directive, HostAttributeToken, inject } from '@angular/core';`,
1276+
``,
1277+
`@Directive()`,
1278+
`class MyDir {`,
1279+
` private foo = inject(new HostAttributeToken('tabindex'), { optional: true })!;`,
1280+
`}`,
1281+
]);
1282+
});
1283+
12591284
it('should pick up the first non-literal type if a parameter has a union type', async () => {
12601285
writeFile(
12611286
'/dir.ts',

0 commit comments

Comments
 (0)