From d8ce2129d50510864a768499248a198f960829ca Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 19 Oct 2019 22:16:03 +0200 Subject: [PATCH] feat(ivy): add flag to disable checking of text attributes (#33365) For elements that have a text attribute, it may happen that the element is matched by a directive that consumes the attribute as an input. In that case, the template type checker will validate the correctness of the attribute with respect to the directive's declared type of the input, which would typically be `boolean` for the `disabled` input. Since empty attributes are assigned the empty string at runtime, the template type checker would report an error for this template. This commit introduces a strictness flag to help alleviate this particular situation, effectively ignoring text attributes that happen to be consumed by a directive. PR Close #33365 --- packages/compiler-cli/src/ngtsc/program.ts | 2 ++ .../src/ngtsc/typecheck/src/api.ts | 15 +++++++++++ .../ngtsc/typecheck/src/type_check_block.ts | 5 ++++ .../ngtsc/typecheck/test/diagnostics_spec.ts | 19 ++++++++++++++ .../src/ngtsc/typecheck/test/test_utils.ts | 2 ++ .../typecheck/test/type_check_block_spec.ts | 26 +++++++++++++++++++ 6 files changed, 69 insertions(+) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 963d4c448c..50f3cc06fd 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -431,6 +431,7 @@ export class NgtscProgram implements api.Program { checkTemplateBodies: true, checkTypeOfInputBindings: true, strictNullInputBindings: true, + checkTypeOfAttributes: true, // Even in full template type-checking mode, DOM binding checks are not quite ready yet. checkTypeOfDomBindings: false, checkTypeOfOutputEvents: true, @@ -451,6 +452,7 @@ export class NgtscProgram implements api.Program { checkTemplateBodies: false, checkTypeOfInputBindings: false, strictNullInputBindings: false, + checkTypeOfAttributes: false, checkTypeOfDomBindings: false, checkTypeOfOutputEvents: false, checkTypeOfAnimationEvents: false, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index c2ccc6bab3..77e2464792 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -98,9 +98,24 @@ export interface TypeCheckingConfig { * binding expressions are wrapped in a non-null assertion operator to effectively disable strict * null checks. This may be particularly useful when the directive is from a library that is not * compiled with `strictNullChecks` enabled. + * + * If `checkTypeOfInputBindings` is set to `false`, this flag has no effect. */ strictNullInputBindings: boolean; + /** + * Whether to check text attributes that happen to be consumed by a directive or component. + * + * For example, in a template containing `` the `disabled` attribute ends + * up being consumed as an input with type `boolean` by the `matInput` directive. At runtime the + * input will be set to the attribute's string value, which is the empty string for attributes + * without a value, so with this flag set to `true` an error would be reported. If set to `false`, + * text attributes will never report an error. + * + * Note that if `checkTypeOfInputBindings` is set to `false`, this flag has no effect. + */ + checkTypeOfAttributes: boolean; + /** * Whether to check the left-hand side type of binding operations to DOM properties. * 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 cd8cb476d4..e28e93d2c0 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 @@ -1153,6 +1153,11 @@ function tcbGetDirectiveInputs( return; } + // Skip text attributes if configured to do so. + if (!tcb.env.config.checkTypeOfAttributes && attr instanceof TmplAstTextAttribute) { + return; + } + // Skip the attribute if the directive does not have an input for it. if (!propMatch.has(attr.name)) { return; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 8d0fddd32a..8cedeb585e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -154,6 +154,25 @@ runInEachFileSystem(() => { ]); }); + it('checks text attributes that are consumed by bindings with literal string types', () => { + const messages = diagnose( + `
`, ` + class Dir { + mode: 'dark'|'light'; + } + class TestComponent {}`, + [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'mode': 'mode'}, + }]); + + expect(messages).toEqual([ + `synthetic.html(1, 10): Type '"drak"' is not assignable to type '"dark" | "light"'.`, + ]); + }); + it('produces diagnostics for pipes', () => { const messages = diagnose( `
{{ person.name | pipe:person.age:1 }}
`, ` diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 6305d3b81d..7d9eb95728 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -149,6 +149,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { checkTemplateBodies: true, checkTypeOfInputBindings: true, strictNullInputBindings: true, + checkTypeOfAttributes: true, // Feature is still in development. // TODO(alxhub): enable when DOM checking via lib.dom.d.ts is further along. checkTypeOfDomBindings: false, @@ -201,6 +202,7 @@ export function tcb( checkQueries: false, checkTypeOfInputBindings: true, strictNullInputBindings: true, + checkTypeOfAttributes: true, checkTypeOfDomBindings: false, checkTypeOfOutputEvents: true, checkTypeOfAnimationEvents: true, 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 99e0fa40f5..5e13b1b145 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 @@ -293,6 +293,7 @@ describe('type check blocks', () => { checkTemplateBodies: true, checkTypeOfInputBindings: true, strictNullInputBindings: true, + checkTypeOfAttributes: true, checkTypeOfDomBindings: false, checkTypeOfOutputEvents: true, checkTypeOfAnimationEvents: true, @@ -438,6 +439,31 @@ describe('type check blocks', () => { }); }); + describe('config.checkTypeOfAttributes', () => { + const TEMPLATE = ``; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'disabled': 'disabled', 'cols': 'cols', 'rows': 'rows'}, + }]; + + it('should assign string value to the input when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('disabled: ("")'); + expect(block).toContain('cols: ("3")'); + expect(block).toContain('rows: (2)'); + }); + + it('should use any for attributes but still check bound attributes when disabled', () => { + const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfAttributes: false}; + const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); + expect(block).toContain('disabled: (null as any)'); + expect(block).toContain('cols: (null as any)'); + expect(block).toContain('rows: (2)'); + }); + }); + describe('config.checkTypeOfPipes', () => { const TEMPLATE = `{{a | test:b:c}}`; const PIPES: TestDeclaration[] = [{