From cd7b1992193be85f3317a4ef793b13c03ac2b215 Mon Sep 17 00:00:00 2001 From: JoostK Date: Thu, 10 Oct 2019 20:22:38 +0200 Subject: [PATCH] feat(ivy): check regular attributes that correspond with directive inputs (#33066) Prior to this change, a static attribute that corresponds with a directive's input would not be type-checked against the type of the input. This is unfortunate, as a static value always has type `string`, whereas the directive's input type might be something different. This typically occurs when a developer forgets to enclose the attribute name in brackets to make it a property binding. This commit lets static attributes be considered as bindings with string values, so that they will be properly type-checked. PR Close #33066 --- .../code/code-tabs.component.ts | 2 +- .../ngtsc/typecheck/src/type_check_block.ts | 38 ++++++++++++------- .../typecheck/test/type_check_block_spec.ts | 11 ++++++ .../test/ngtsc/template_typecheck_spec.ts | 26 +++++++++++++ 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/aio/src/app/custom-elements/code/code-tabs.component.ts b/aio/src/app/custom-elements/code/code-tabs.component.ts index 3a507f2b64..8f50df5643 100644 --- a/aio/src/app/custom-elements/code/code-tabs.component.ts +++ b/aio/src/app/custom-elements/code/code-tabs.component.ts @@ -27,7 +27,7 @@ export interface TabInfo {
- + {{ tab.header }} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 4c330b8131..c8627b3823 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -840,6 +840,7 @@ function tcbGetDirectiveInputs( const unsetFields = new Set(propMatch.values()); el.inputs.forEach(processAttribute); + el.attributes.forEach(processAttribute); if (el instanceof TmplAstTemplate) { el.templateAttrs.forEach(processAttribute); } @@ -856,21 +857,30 @@ function tcbGetDirectiveInputs( * a matching binding. */ function processAttribute(attr: TmplAstBoundAttribute | TmplAstTextAttribute): void { - if (attr instanceof TmplAstBoundAttribute && propMatch.has(attr.name)) { - const field = propMatch.get(attr.name) !; - - // Remove the field from the set of unseen fields, now that it's been assigned to. - unsetFields.delete(field); - - // Produce an expression representing the value of the binding. - const expr = tcbExpression(attr.value, tcb, scope, attr.valueSpan || attr.sourceSpan); - directiveInputs.push({ - type: 'binding', - field: field, - expression: expr, - sourceSpan: attr.sourceSpan, - }); + // Skip the attribute if the directive does not have an input for it. + if (!propMatch.has(attr.name)) { + return; } + const field = propMatch.get(attr.name) !; + + // Remove the field from the set of unseen fields, now that it's been assigned to. + unsetFields.delete(field); + + let expr: ts.Expression; + if (attr instanceof TmplAstBoundAttribute) { + // Produce an expression representing the value of the binding. + expr = tcbExpression(attr.value, tcb, scope, attr.valueSpan || attr.sourceSpan); + } else { + // For regular attributes with a static string value, use the represented string literal. + expr = ts.createStringLiteral(attr.value); + } + + directiveInputs.push({ + type: 'binding', + field: field, + expression: expr, + sourceSpan: attr.sourceSpan, + }); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 248176a88c..cdad5d93de 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -35,6 +35,17 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE)).toContain('((ctx).a)[(ctx).b];'); }); + it('should handle attribute values for directive inputs', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'DirA', + selector: '[dir]', + inputs: {inputA: 'inputA'}, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: ("value")'); + }); + it('should handle empty bindings', () => { const TEMPLATE = `
`; const DIRECTIVES: TestDeclaration[] = [{ diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 410655dc49..035be9e2d1 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -80,6 +80,32 @@ export declare class CommonModule { env.driveMain(); }); + it('should check regular attributes that are directive inputs', () => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp {} + + @Directive({selector: '[dir]'}) + class TestDir { + @Input() foo: number; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); + }); + it('should check basic usage of NgIf', () => { env.write('test.ts', ` import {CommonModule} from '@angular/common';