Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(ivy): strictness flags for template type checking (#33365)
The template type checking abilities of the Ivy compiler are far more
advanced than the level of template type checking that was previously
done for Angular templates. Up until now, a single compiler option
called "fullTemplateTypeCheck" was available to configure the level
of template type checking. However, now that more advanced type checking
is being done, new errors may surface that were previously not reported,
in which case it may not be feasible to fix all new errors at once.

Having only a single option to disable a large number of template type
checking capabilities does not allow for incrementally addressing newly
reported types of errors. As a solution, this commit introduces some new
compiler options to be able to enable/disable certain kinds of template
type checks on a fine-grained basis.

PR Close #33365
  • Loading branch information
JoostK authored and AndrewKushnir committed Oct 24, 2019
1 parent 113411c commit 0d9be22
Show file tree
Hide file tree
Showing 4 changed files with 604 additions and 15 deletions.
44 changes: 36 additions & 8 deletions packages/compiler-cli/src/ngtsc/program.ts
Expand Up @@ -425,26 +425,29 @@ export class NgtscProgram implements api.Program {
// requested.
let typeCheckingConfig: TypeCheckingConfig;
if (this.options.fullTemplateTypeCheck) {
const strictTemplates = !!this.options.strictTemplates;
typeCheckingConfig = {
applyTemplateContextGuards: true,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfInputBindings: true,
strictNullInputBindings: true,
checkTypeOfAttributes: true,
checkTypeOfInputBindings: strictTemplates,
strictNullInputBindings: strictTemplates,
checkTypeOfAttributes: strictTemplates,
// Even in full template type-checking mode, DOM binding checks are not quite ready yet.
checkTypeOfDomBindings: false,
checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true,
checkTypeOfOutputEvents: strictTemplates,
checkTypeOfAnimationEvents: strictTemplates,
// Checking of DOM events currently has an adverse effect on developer experience,
// e.g. for `<input (blur)="update($event.target.value)">` enabling this check results in:
// - error TS2531: Object is possibly 'null'.
// - error TS2339: Property 'value' does not exist on type 'EventTarget'.
checkTypeOfDomEvents: false,
checkTypeOfDomReferences: true,
checkTypeOfDomEvents: strictTemplates,
checkTypeOfDomReferences: strictTemplates,
// Non-DOM references have the correct type in View Engine so there is no strictness flag.
checkTypeOfNonDomReferences: true,
// Pipes are checked in View Engine so there is no strictness flag.
checkTypeOfPipes: true,
strictSafeNavigationTypes: true,
strictSafeNavigationTypes: strictTemplates,
};
} else {
typeCheckingConfig = {
Expand All @@ -465,6 +468,31 @@ export class NgtscProgram implements api.Program {
};
}

// Apply explicitly configured strictness flags on top of the default configuration
// based on "fullTemplateTypeCheck".
if (this.options.strictInputTypes !== undefined) {
typeCheckingConfig.checkTypeOfInputBindings = this.options.strictInputTypes;
}
if (this.options.strictNullInputTypes !== undefined) {
typeCheckingConfig.strictNullInputBindings = this.options.strictNullInputTypes;
}
if (this.options.strictOutputEventTypes !== undefined) {
typeCheckingConfig.checkTypeOfOutputEvents = this.options.strictOutputEventTypes;
typeCheckingConfig.checkTypeOfAnimationEvents = this.options.strictOutputEventTypes;
}
if (this.options.strictDomEventTypes !== undefined) {
typeCheckingConfig.checkTypeOfDomEvents = this.options.strictDomEventTypes;
}
if (this.options.strictSafeNavigationTypes !== undefined) {
typeCheckingConfig.strictSafeNavigationTypes = this.options.strictSafeNavigationTypes;
}
if (this.options.strictDomLocalRefTypes !== undefined) {
typeCheckingConfig.checkTypeOfDomReferences = this.options.strictDomLocalRefTypes;
}
if (this.options.strictAttributeTypes !== undefined) {
typeCheckingConfig.checkTypeOfAttributes = this.options.strictAttributeTypes;
}

// Execute the typeCheck phase of each decorator in the program.
const prepSpan = this.perfRecorder.start('typeCheckPrep');
const ctx = new TypeCheckContext(typeCheckingConfig, this.refEmitter !, this.typeCheckFilePath);
Expand Down
11 changes: 6 additions & 5 deletions packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
Expand Up @@ -107,10 +107,10 @@ export interface TypeCheckingConfig {
* Whether to check text attributes that happen to be consumed by a directive or component.
*
* For example, in a template containing `<input matInput disabled>` 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.
* 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 an 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.
*/
Expand All @@ -129,7 +129,8 @@ export interface TypeCheckingConfig {
checkTypeOfDomBindings: boolean;

/**
* Whether to infer the type of the `$event` variable in event bindings for directive outputs.
* Whether to infer the type of the `$event` variable in event bindings for directive outputs or
* animation events.
*
* If this is `true`, the type of `$event` will be inferred based on the generic type of
* `EventEmitter`/`Subject` of the output. If set to `false`, the `$event` variable will be of
Expand Down
106 changes: 104 additions & 2 deletions packages/compiler-cli/src/transformers/api.ts
Expand Up @@ -108,10 +108,112 @@ export interface CompilerOptions extends ts.CompilerOptions {
// Default is true.
generateCodeForLibraries?: boolean;

// Whether to enable all type checks for templates.
// This will be true be default in Angular 6.
/**
* Whether to type check the entire template.
*
* This flag currently controls a couple aspects of template type-checking, including
* whether embedded views are checked.
*
* For maximum type-checking, set this to `true`, and set `strictTemplates` to `true`.
*/
fullTemplateTypeCheck?: boolean;

/**
* If `true`, implies all template strictness flags below (unless individually disabled).
*
* Has no effect unless `fullTemplateTypeCheck` is also enabled.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictTemplates?: boolean;


/**
* Whether to check the type of a binding to a directive/component input against the type of the
* field on the directive/component.
*
* For example, if this is `false` then the expression `[input]="expr"` will have `expr` type-
* checked, but not the assignment of the resulting type to the `input` property of whichever
* directive or component is receiving the binding. If set to `true`, both sides of the assignment
* are checked.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictInputTypes?: 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.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set. Note that if `strictInputTypes` is
* not set, or set to `false`, this flag has no effect.
*/
strictNullInputTypes?: boolean;

/**
* Whether to check text attributes that happen to be consumed by a directive or component.
*
* For example, in a template containing `<input matInput disabled>` 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 an 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.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set. Note that if `strictInputTypes` is
* not set, or set to `false`, this flag has no effect.
*/
strictAttributeTypes?: boolean;

/**
* Whether to use a strict type for null-safe navigation operations.
*
* If this is `false`, then the return type of `a?.b` or `a?()` will be `any`. If set to `true`,
* then the return type of `a?.b` for example will be the same as the type of the ternary
* expression `a != null ? a.b : a`.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictSafeNavigationTypes?: boolean;

/**
* Whether to infer the type of local references.
*
* 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. If set to `false`,
* the type of `ref` for DOM nodes will be `any`.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictDomLocalRefTypes?: boolean;

/**
* Whether to infer the type of the `$event` variable in event bindings for directive outputs or
* animation events.
*
* If this is `true`, the type of `$event` will be inferred based on the generic type of
* `EventEmitter`/`Subject` of the output. If set to `false`, the `$event` variable will be of
* type `any`.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictOutputEventTypes?: boolean;

/**
* Whether to infer the type of the `$event` variable in event bindings to DOM events.
*
* If this is `true`, the type of `$event` will be inferred based on TypeScript's
* `HTMLElementEventMap`, with a fallback to the native `Event` type. If set to `false`, the
* `$event` variable will be of type `any`.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictDomEventTypes?: boolean;

// Whether to use the CompilerHost's fileNameToModuleName utility (if available) to generate
// import module specifiers. This is false by default, and exists to support running ngtsc
// within Google. This option is internal and is used by the ng_module.bzl rule to switch
Expand Down

0 comments on commit 0d9be22

Please sign in to comment.