From ece0b2d7ce91ce76e407a1f047d1ed42053f9c1d Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 11 Oct 2019 19:41:05 +0200 Subject: [PATCH] feat(ivy): disable strict null checks for input bindings (#33066) This commit introduces an internal config option of the template type checker that allows to disable strict null checks of input bindings to directives. This may be particularly useful when a directive is from a library that is not compiled with `strictNullChecks` enabled. Right now, strict null checks are enabled when `fullTemplateTypeCheck` is turned on, and disabled when it's off. In the near future, several of the internal configuration options will be added as public Angular compiler options so that users can have fine-grained control over which areas of the template type checker to enable, allowing for a more incremental migration strategy. PR Close #33066 --- packages/compiler-cli/src/ngtsc/program.ts | 2 ++ .../src/ngtsc/typecheck/src/api.ts | 12 ++++++++++ .../ngtsc/typecheck/src/type_check_block.ts | 22 ++++++++++++----- .../src/ngtsc/typecheck/test/test_utils.ts | 2 ++ .../typecheck/test/type_check_block_spec.ts | 24 ++++++++++++++++--- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 672a7a1fdc..041f1b8247 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -401,6 +401,7 @@ export class NgtscProgram implements api.Program { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: true, + strictNullInputBindings: true, // Even in full template type-checking mode, DOM binding checks are not quite ready yet. checkTypeOfDomBindings: false, checkTypeOfPipes: true, @@ -412,6 +413,7 @@ export class NgtscProgram implements api.Program { checkQueries: false, checkTemplateBodies: false, checkTypeOfInputBindings: false, + strictNullInputBindings: false, checkTypeOfDomBindings: false, checkTypeOfPipes: false, strictSafeNavigationTypes: false, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index 1658237e0d..9653d492c1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -83,6 +83,18 @@ export interface TypeCheckingConfig { */ checkTypeOfInputBindings: boolean; + /** + * Whether to use strict null types for input bindings for directives. + * + * If this is `true`, applications that are compiled with TypeScript's `strictNullChecks` enabled + * will produce type errors for bindings which can evaluate to `undefined` or `null` where the + * inputs's type does not include `undefined` or `null` in its type. If set to `false`, all + * 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. + */ + strictNullInputBindings: 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 58def5ef89..4c330b8131 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 @@ -388,11 +388,14 @@ class TcbUnclaimedInputsOp extends TcbOp { let expr = tcbExpression( binding.value, this.tcb, this.scope, binding.valueSpan || binding.sourceSpan); - - // If checking the type of bindings is disabled, cast the resulting expression to 'any' before - // the assignment. if (!this.tcb.env.config.checkTypeOfInputBindings) { + // If checking the type of bindings is disabled, cast the resulting expression to 'any' + // before the assignment. expr = tsCastToAny(expr); + } else if (!this.tcb.env.config.strictNullInputBindings) { + // If strict null checks are disabled, erase `null` and `undefined` from the type by + // wrapping the expression in a non-null assertion. + expr = ts.createNonNullExpression(expr); } if (this.tcb.env.config.checkTypeOfDomBindings && binding.type === BindingType.Property) { @@ -781,11 +784,18 @@ function tcbCallTypeCtor( const members = inputs.map(input => { if (input.type === 'binding') { // For bound inputs, the property is assigned the binding expression. - let expression = input.expression; + let expr = input.expression; if (!tcb.env.config.checkTypeOfInputBindings) { - expression = tsCastToAny(expression); + // If checking the type of bindings is disabled, cast the resulting expression to 'any' + // before the assignment. + expr = tsCastToAny(expr); + } else if (!tcb.env.config.strictNullInputBindings) { + // If strict null checks are disabled, erase `null` and `undefined` from the type by + // wrapping the expression in a non-null assertion. + expr = ts.createNonNullExpression(expr); } - const assignment = ts.createPropertyAssignment(input.field, wrapForDiagnostics(expression)); + + const assignment = ts.createPropertyAssignment(input.field, wrapForDiagnostics(expr)); addParseSpanInfo(assignment, input.sourceSpan); return assignment; } else { 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 c0d05b661f..d3ce287d7c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -119,6 +119,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: true, + strictNullInputBindings: true, // Feature is still in development. // TODO(alxhub): enable when DOM checking via lib.dom.d.ts is further along. checkTypeOfDomBindings: false, @@ -160,6 +161,7 @@ export function tcb( applyTemplateContextGuards: true, checkQueries: false, checkTypeOfInputBindings: true, + strictNullInputBindings: true, checkTypeOfDomBindings: false, checkTypeOfPipes: true, checkTemplateBodies: 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 ee0e013a71..248176a88c 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 @@ -222,6 +222,7 @@ describe('type check blocks', () => { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: true, + strictNullInputBindings: true, checkTypeOfDomBindings: false, checkTypeOfPipes: true, strictSafeNavigationTypes: true, @@ -257,20 +258,37 @@ describe('type check blocks', () => { }); }); + describe('config.strictNullInputBindings', () => { + const TEMPLATE = `
`; + + it('should include null and undefined when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })'); + expect(block).toContain('(ctx).b;'); + }); + it('should use the non-null assertion operator when disabled', () => { + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false}; + const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); + expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a!) })'); + expect(block).toContain('(ctx).b!;'); + }); + }); + describe('config.checkTypeOfBindings', () => { - const TEMPLATE = `
`; + const TEMPLATE = `
`; it('should check types of bindings when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })'); - expect(block).toContain('(ctx).a;'); + expect(block).toContain('(ctx).b;'); }); it('should not check types of bindings when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('Dir.ngTypeCtor({ dirInput: (((ctx).a as any)) })'); - expect(block).toContain('((ctx).a as any);'); + expect(block).toContain('((ctx).b as any);'); }); });