From 113411c9b0e29d24f55588b7fbe02c36157e5d34 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 23 Oct 2019 17:47:47 -0700 Subject: [PATCH] fix(ivy): split checkTypeOfReferences into DOM and non-DOM flags. (#33365) View Engine correctly infers the type of local refs to directives or to s, just not to DOM nodes. This commit splits the checkTypeOfReferences flag into two separate halves, allowing the compiler to align with this behavior. PR Close #33365 --- packages/compiler-cli/src/ngtsc/program.ts | 6 ++- .../src/ngtsc/typecheck/src/api.ts | 18 +++++++-- .../ngtsc/typecheck/src/type_check_block.ts | 20 +++++++--- .../src/ngtsc/typecheck/test/test_utils.ts | 6 ++- .../typecheck/test/type_check_block_spec.ts | 39 +++++++++++++++++-- 5 files changed, 73 insertions(+), 16 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 50f3cc06fd..55f1d4871d 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -441,7 +441,8 @@ export class NgtscProgram implements api.Program { // - error TS2531: Object is possibly 'null'. // - error TS2339: Property 'value' does not exist on type 'EventTarget'. checkTypeOfDomEvents: false, - checkTypeOfReferences: true, + checkTypeOfDomReferences: true, + checkTypeOfNonDomReferences: true, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -457,7 +458,8 @@ export class NgtscProgram implements api.Program { checkTypeOfOutputEvents: false, checkTypeOfAnimationEvents: false, checkTypeOfDomEvents: false, - checkTypeOfReferences: false, + checkTypeOfDomReferences: false, + checkTypeOfNonDomReferences: 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 77e2464792..8ef1f71bc8 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -154,14 +154,24 @@ export interface TypeCheckingConfig { */ checkTypeOfDomEvents: boolean; + /** + * Whether to infer the type of local references to DOM elements. + * + * If this is `true`, the type of a `#ref` variable on a DOM node in the template will be + * determined by the type of `document.createElement` for the given DOM node type. If set to + * `false`, the type of `ref` for DOM nodes will be `any`. + */ + checkTypeOfDomReferences: boolean; + + /** * Whether to infer the type of local references. * - * If this is `true`, the type of any `#ref` variable in the template will be determined by the - * referenced entity (either a directive or a DOM element). If set to `false`, the type of `ref` - * will be `any`. + * If this is `true`, the type of a `#ref` variable that points to a directive or `TemplateRef` in + * the template will be inferred correctly. If set to `false`, the type of `ref` for will be + * `any`. */ - checkTypeOfReferences: boolean; + checkTypeOfNonDomReferences: boolean; /** * Whether to include type information from pipes in the type-checking operation. 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 e28e93d2c0..384dfa1d8f 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 @@ -1023,11 +1023,6 @@ class TcbExpressionTranslator { addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); return expr; } else if (binding instanceof TmplAstReference) { - if (!this.tcb.env.config.checkTypeOfReferences) { - // References are pinned to 'any'. - return NULL_AS_ANY; - } - const target = this.tcb.boundTarget.getReferenceTarget(binding); if (target === null) { throw new Error(`Unbound reference? ${binding.name}`); @@ -1037,10 +1032,20 @@ class TcbExpressionTranslator { // element or template. if (target instanceof TmplAstElement) { + if (!this.tcb.env.config.checkTypeOfDomReferences) { + // References to DOM nodes are pinned to 'any'. + return NULL_AS_ANY; + } + const expr = ts.getMutableClone(this.scope.resolve(target)); addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); return expr; } else if (target instanceof TmplAstTemplate) { + if (!this.tcb.env.config.checkTypeOfNonDomReferences) { + // References to `TemplateRef`s pinned to 'any'. + return NULL_AS_ANY; + } + // Direct references to an node simply require a value of type // `TemplateRef`. To get this, an expression of the form // `(null as any as TemplateRef)` is constructed. @@ -1053,6 +1058,11 @@ class TcbExpressionTranslator { addParseSpanInfo(value, toAbsoluteSpan(ast.span, this.sourceSpan)); return value; } else { + if (!this.tcb.env.config.checkTypeOfNonDomReferences) { + // References to directives are pinned to 'any'. + return NULL_AS_ANY; + } + const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive)); addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); return expr; 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 7d9eb95728..3945b02d1d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -156,7 +156,8 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { checkTypeOfOutputEvents: true, checkTypeOfAnimationEvents: true, checkTypeOfDomEvents: true, - checkTypeOfReferences: true, + checkTypeOfDomReferences: true, + checkTypeOfNonDomReferences: true, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -207,7 +208,8 @@ export function tcb( checkTypeOfOutputEvents: true, checkTypeOfAnimationEvents: true, checkTypeOfDomEvents: true, - checkTypeOfReferences: true, + checkTypeOfDomReferences: true, + checkTypeOfNonDomReferences: true, checkTypeOfPipes: true, checkTemplateBodies: true, strictSafeNavigationTypes: 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 5e13b1b145..9343450425 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 @@ -298,7 +298,8 @@ describe('type check blocks', () => { checkTypeOfOutputEvents: true, checkTypeOfAnimationEvents: true, checkTypeOfDomEvents: true, - checkTypeOfReferences: true, + checkTypeOfDomReferences: true, + checkTypeOfNonDomReferences: true, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -424,7 +425,7 @@ describe('type check blocks', () => { }); }); - describe('config.checkTypeOfReferences', () => { + describe('config.checkTypeOfDomReferences', () => { const TEMPLATE = `{{ref.value}}`; it('should trace references when enabled', () => { @@ -433,12 +434,44 @@ describe('type check blocks', () => { }); it('should use any for reference types when disabled', () => { - const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfReferences: false}; + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfDomReferences: false}; const block = tcb(TEMPLATE, [], DISABLED_CONFIG); expect(block).toContain('(null as any).value'); }); }); + describe('config.checkTypeOfNonDomReferences', () => { + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + exportAs: ['dir'], + inputs: {'dirInput': 'dirInput'}, + outputs: {'outputField': 'dirOutput'}, + hasNgTemplateContextGuard: true, + }]; + const TEMPLATE = + `
{{ref.value}}
{{ref2.value2}}`; + + it('should trace references to a directive when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('(_t2).value'); + }); + + it('should trace references to an when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('((null as any as core.TemplateRef)).value2'); + }); + + it('should use any for reference types when disabled', () => { + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfNonDomReferences: false}; + const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); + expect(block).toContain('(null as any).value'); + }); + }); + describe('config.checkTypeOfAttributes', () => { const TEMPLATE = ``; const DIRECTIVES: TestDeclaration[] = [{