From 4253662231f8e3f6de137ab511ff7fce4871a168 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 14 Feb 2020 13:30:43 -0800 Subject: [PATCH] fix(ivy): add strictLiteralTypes to align Ivy + VE checking of literals (#35462) Under View Engine's default (non-fullTemplateTypeCheck) checking, object and array literals which appear in templates are treated as having type `any`. This allows a number of patterns which would not otherwise compile, such as indexing an object literal by a string: ```html {{ {'a': 1, 'b': 2}[value] }} ``` (where `value` is `string`) Ivy, meanwhile, has always inferred strong types for object literals, even in its compatibility mode. This commit fixes the bug, and adds the `strictLiteralTypes` flag to specifically control this inference. When the flag is `false` (in compatibility mode), object and array literals receive the `any` type. PR Close #35462 --- aio/content/guide/template-typecheck.md | 1 + packages/compiler-cli/src/ngtsc/core/api.ts | 8 ++++++++ packages/compiler-cli/src/ngtsc/core/src/compiler.ts | 5 +++++ packages/compiler-cli/src/ngtsc/typecheck/src/api.ts | 9 +++++++++ .../compiler-cli/src/ngtsc/typecheck/src/expression.ts | 9 +++++++-- .../compiler-cli/src/ngtsc/typecheck/test/test_utils.ts | 2 ++ .../src/ngtsc/typecheck/test/type_check_block_spec.ts | 1 + 7 files changed, 33 insertions(+), 2 deletions(-) diff --git a/aio/content/guide/template-typecheck.md b/aio/content/guide/template-typecheck.md index 0a5b35fc9e..9eb02023c0 100644 --- a/aio/content/guide/template-typecheck.md +++ b/aio/content/guide/template-typecheck.md @@ -121,6 +121,7 @@ In case of a false positive like these, there are a few options: |`strictOutputEventTypes`|Whether `$event` will have the correct type for event bindings to component/directive an `@Output()`, or to animation events. If disabled, it will be `any`.| |`strictDomEventTypes`|Whether `$event` will have the correct type for event bindings to DOM events. If disabled, it will be `any`.| |`strictContextGenerics`|Whether the type parameters of generic components will be inferred correctly (including any generic bounds). If disabled, any type parameters will be `any`.| +|`strictLiteralTypes`|Whether object and array literals declared in the template will have their type inferred. If disabled, the type of such literals will be `any`.| If you still have issues after troubleshooting with these flags, you can fall back to full mode by disabling `strictTemplates`. diff --git a/packages/compiler-cli/src/ngtsc/core/api.ts b/packages/compiler-cli/src/ngtsc/core/api.ts index e8d035aa83..595d9a51c6 100644 --- a/packages/compiler-cli/src/ngtsc/core/api.ts +++ b/packages/compiler-cli/src/ngtsc/core/api.ts @@ -278,6 +278,14 @@ export interface StrictTemplateOptions { * Defaults to `false`, even if "fullTemplateTypeCheck" is set. */ strictContextGenerics?: boolean; + + /** + * Whether object or array literals defined in templates use their inferred type, or are + * interpreted as `any`. + * + * Defaults to `false` unless `fullTemplateTypeCheck` or `strictTemplates` are set. + */ + strictLiteralTypes?: boolean; } /** diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 1a292ade54..9f315059b9 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -424,6 +424,7 @@ export class NgCompiler { checkTypeOfPipes: true, strictSafeNavigationTypes: strictTemplates, useContextGenericType: strictTemplates, + strictLiteralTypes: true, }; } else { typeCheckingConfig = { @@ -442,6 +443,7 @@ export class NgCompiler { checkTypeOfPipes: false, strictSafeNavigationTypes: false, useContextGenericType: false, + strictLiteralTypes: false, }; } @@ -473,6 +475,9 @@ export class NgCompiler { if (this.options.strictContextGenerics !== undefined) { typeCheckingConfig.useContextGenericType = this.options.strictContextGenerics; } + if (this.options.strictLiteralTypes !== undefined) { + typeCheckingConfig.strictLiteralTypes = this.options.strictLiteralTypes; + } // Execute the typeCheck phase of each decorator in the program. const prepSpan = this.perfRecorder.start('typeCheckPrep'); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index 1032c4b2a5..3134a3db8a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -220,6 +220,15 @@ export interface TypeCheckingConfig { * component will be set to `any` during type-checking. */ useContextGenericType: boolean; + + /** + * Whether or not to infer types for object and array literals in the template. + * + * If this is `true`, then the type of an object or an array literal in the template will be the + * same type that TypeScript would infer if the literal appeared in code. If `false`, then such + * literals are cast to `any` when declared. + */ + strictLiteralTypes: boolean; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index bbdd7d7774..fd1fa179c7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -11,6 +11,7 @@ import * as ts from 'typescript'; import {TypeCheckingConfig} from './api'; import {addParseSpanInfo, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics'; +import {tsCastToAny} from './ts_util'; export const NULL_AS_ANY = ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); @@ -143,7 +144,9 @@ class AstTranslator implements AstVisitor { visitLiteralArray(ast: LiteralArray): ts.Expression { const elements = ast.expressions.map(expr => this.translate(expr)); - const node = ts.createArrayLiteral(elements); + const literal = ts.createArrayLiteral(elements); + // If strictLiteralTypes is disabled, array literals are cast to `any`. + const node = this.config.strictLiteralTypes ? literal : tsCastToAny(literal); addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -153,7 +156,9 @@ class AstTranslator implements AstVisitor { const value = this.translate(ast.values[idx]); return ts.createPropertyAssignment(ts.createStringLiteral(key), value); }); - const node = ts.createObjectLiteral(properties, true); + const literal = ts.createObjectLiteral(properties, true); + // If strictLiteralTypes is disabled, object literals are cast to `any`. + const node = this.config.strictLiteralTypes ? literal : tsCastToAny(literal); addParseSpanInfo(node, ast.sourceSpan); return node; } 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 aecbb168cc..c5dcb906d5 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -164,6 +164,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { checkTypeOfPipes: true, strictSafeNavigationTypes: true, useContextGenericType: true, + strictLiteralTypes: true, }; // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead. @@ -219,6 +220,7 @@ export function tcb( checkTemplateBodies: true, strictSafeNavigationTypes: true, useContextGenericType: true, + strictLiteralTypes: true, }; options = options || { emitSpans: false, 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 4286685a9a..e1955bd55f 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 @@ -330,6 +330,7 @@ describe('type check blocks', () => { checkTypeOfPipes: true, strictSafeNavigationTypes: true, useContextGenericType: true, + strictLiteralTypes: true, }; describe('config.applyTemplateContextGuards', () => {