fix(ivy): use any for generic context checks when !strictTemplates (#34649)

Previously, the template type-checker would always construct a generic
template context type with correct bounds, even when strictTemplates was
disabled. This meant that type-checking of expressions involving that type
was stricter than View Engine.

This commit introduces a 'strictContextGenerics' flag which behaves
similarly to other 'strictTemplates' flags, and switches the inference of
generic type parameters on the component context based on the value of this
flag.

PR Close #34649
This commit is contained in:
Alex Rickabaugh 2019-12-19 17:09:56 -08:00 committed by Andrew Kushnir
parent cb11380515
commit 0c8d085666
9 changed files with 61 additions and 8 deletions

View File

@ -120,6 +120,7 @@ In case of a false positive like these, there are a few options:
|`strictDomLocalRefTypes`|Whether local references to DOM elements will have the correct type. If disabled `ref` will be of type `any` for `<input #ref>`.|
|`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`.|
If you still have issues after troubleshooting with these flags, you can fall back to full mode by disabling `strictTemplates`.

View File

@ -9,7 +9,7 @@
export {AliasStrategy, AliasingHost, FileToModuleAliasingHost, PrivateExportAliasingHost} from './src/alias';
export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core';
export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default';
export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy} from './src/emitter';
export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy} from './src/emitter';
export {Reexport} from './src/reexport';
export {ImportFlags, OwningModule, Reference} from './src/references';
export {OwningModule, Reference} from './src/references';
export {ModuleResolver} from './src/resolver';

View File

@ -504,6 +504,7 @@ export class NgtscProgram implements api.Program {
// Pipes are checked in View Engine so there is no strictness flag.
checkTypeOfPipes: true,
strictSafeNavigationTypes: strictTemplates,
useContextGenericType: strictTemplates,
};
} else {
typeCheckingConfig = {
@ -521,6 +522,7 @@ export class NgtscProgram implements api.Program {
checkTypeOfNonDomReferences: false,
checkTypeOfPipes: false,
strictSafeNavigationTypes: false,
useContextGenericType: false,
};
}
@ -549,6 +551,9 @@ export class NgtscProgram implements api.Program {
if (this.options.strictAttributeTypes !== undefined) {
typeCheckingConfig.checkTypeOfAttributes = this.options.strictAttributeTypes;
}
if (this.options.strictContextGenerics !== undefined) {
typeCheckingConfig.useContextGenericType = this.options.strictContextGenerics;
}
// Execute the typeCheck phase of each decorator in the program.
const prepSpan = this.perfRecorder.start('typeCheckPrep');

View File

@ -211,6 +211,15 @@ export interface TypeCheckingConfig {
* This is currently an unsupported feature.
*/
checkQueries: false;
/**
* Whether to use any generic types of the context component.
*
* If this is `true`, then if the context component has generic types, those will be mirrored in
* the template type-checking context. If `false`, any generic type parameters of the context
* component will be set to `any` during type-checking.
*/
useContextGenericType: boolean;
}

View File

@ -57,7 +57,7 @@ export function generateTypeCheckBlock(
throw new Error(
`Expected TypeReferenceNode when referencing the ctx param for ${ref.debugName}`);
}
const paramList = [tcbCtxParam(ref.node, ctxRawType.typeName)];
const paramList = [tcbCtxParam(ref.node, ctxRawType.typeName, env.config.useContextGenericType)];
const scopeStatements = scope.render();
const innerBody = ts.createBlock([
@ -73,7 +73,7 @@ export function generateTypeCheckBlock(
/* modifiers */ undefined,
/* asteriskToken */ undefined,
/* name */ name,
/* typeParameters */ ref.node.typeParameters,
/* typeParameters */ env.config.useContextGenericType ? ref.node.typeParameters : undefined,
/* parameters */ paramList,
/* type */ undefined,
/* body */ body);
@ -925,12 +925,18 @@ class Scope {
* parameters listed (without their generic bounds).
*/
function tcbCtxParam(
node: ClassDeclaration<ts.ClassDeclaration>, name: ts.EntityName): ts.ParameterDeclaration {
node: ClassDeclaration<ts.ClassDeclaration>, name: ts.EntityName,
useGenericType: boolean): ts.ParameterDeclaration {
let typeArguments: ts.TypeNode[]|undefined = undefined;
// Check if the component is generic, and pass generic type parameters if so.
if (node.typeParameters !== undefined) {
typeArguments =
node.typeParameters.map(param => ts.createTypeReferenceNode(param.name, undefined));
if (useGenericType) {
typeArguments =
node.typeParameters.map(param => ts.createTypeReferenceNode(param.name, undefined));
} else {
typeArguments =
node.typeParameters.map(() => ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
}
}
const type = ts.createTypeReferenceNode(name, typeArguments);
return ts.createParameter(

View File

@ -163,6 +163,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true,
strictSafeNavigationTypes: true,
useContextGenericType: true,
};
// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
@ -217,6 +218,7 @@ export function tcb(
checkTypeOfPipes: true,
checkTemplateBodies: true,
strictSafeNavigationTypes: true,
useContextGenericType: true,
};
options = options || {
emitSpans: false,

View File

@ -326,6 +326,7 @@ describe('type check blocks', () => {
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true,
strictSafeNavigationTypes: true,
useContextGenericType: true,
};
describe('config.applyTemplateContextGuards', () => {
@ -567,5 +568,20 @@ describe('type check blocks', () => {
expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.b : null as any)');
});
});
describe('config.strictContextGenerics', () => {
const TEMPLATE = `Test`;
it('should use the generic type of the context when enabled', () => {
const block = tcb(TEMPLATE);
expect(block).toContain('function Test_TCB<T extends string>(ctx: Test<T>)');
});
it('should use any for the context generic type when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, useContextGenericType: false};
const block = tcb(TEMPLATE, undefined, DISABLED_CONFIG);
expect(block).toContain('function Test_TCB(ctx: Test<any>)');
});
});
});
});

View File

@ -215,6 +215,19 @@ export interface CompilerOptions extends ts.CompilerOptions {
*/
strictDomEventTypes?: boolean;
/**
* Whether to include the generic type of components when type-checking the template.
*
* If no component has generic type parameters, this setting has no effect.
*
* If a component has generic type parameters and this setting is `true`, those generic parameters
* will be included in the context type for the template. If `false`, any generic parameters will
* be set to `any` in the template context type.
*
* Defaults to `false`, even if "fullTemplateTypeCheck" is set.
*/
strictContextGenerics?: 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

View File

@ -972,7 +972,8 @@ export declare class AnimationEvent {
});
it('should constrain types using type parameter bounds', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true});
env.tsconfig(
{fullTemplateTypeCheck: true, strictInputTypes: true, strictContextGenerics: true});
env.write('test.ts', `
import {CommonModule} from '@angular/common';
import {Component, Input, NgModule} from '@angular/core';