fix(ivy): avoid implicit any errors in event handlers (#33550)

When template type checking is configured with `strictDomEventTypes` or
`strictOutputEventTypes` disabled, in compilation units that have
`noImplicitAny` enabled but `strictNullChecks` disabled, a template type
checking error could be produced for certain event handlers.

The error is avoided by letting an event handler in the generated TCB
always have an explicit `any` return type.

Fixes #33528

PR Close #33550
This commit is contained in:
JoostK 2019-11-02 21:42:25 +01:00 committed by atscott
parent d749dd3ea1
commit e2d7b25e0d
4 changed files with 40 additions and 21 deletions

View File

@ -1276,7 +1276,7 @@ function tcbCreateEventHandler(
/* modifier */ undefined,
/* typeParameters */ undefined,
/* parameters */[eventParam],
/* type */ undefined,
/* type */ ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword),
/* equalsGreaterThanToken*/ undefined,
/* body */ handler);
}

View File

@ -6,9 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/
import {TestFile, runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import * as ts from 'typescript';
import {TestFile, runInEachFileSystem} from '../../file_system/testing';
import {TypeCheckingConfig} from '../src/api';
import {TestDeclaration, ngForDeclaration, ngForDts, typecheck} from './test_utils';
runInEachFileSystem(() => {
@ -258,6 +260,21 @@ runInEachFileSystem(() => {
expect(messages).toEqual([]);
});
// https://github.com/angular/angular/issues/33528
it('should not produce a diagnostic for implicit any return types', () => {
const messages = diagnose(
`<div (click)="state = null"></div>`, `
class TestComponent {
state: any;
}`,
// Disable strict DOM event checking and strict null checks, to infer an any return type
// that would be implicit if the handler function would not have an explicit return
// type.
[], [], {checkTypeOfDomEvents: false}, {strictNullChecks: false});
expect(messages).toEqual([]);
});
});
describe('strict null checks', () => {
@ -314,8 +331,9 @@ class TestComponent {
function diagnose(
template: string, source: string, declarations?: TestDeclaration[],
additionalSources: TestFile[] = []): string[] {
const diagnostics = typecheck(template, source, declarations, additionalSources);
additionalSources: TestFile[] = [], config?: Partial<TypeCheckingConfig>,
options?: ts.CompilerOptions): string[] {
const diagnostics = typecheck(template, source, declarations, additionalSources, config, options);
return diagnostics.map(diag => {
const text =
typeof diag.messageText === 'string' ? diag.messageText : diag.messageText.messageText;

View File

@ -230,7 +230,8 @@ export function tcb(
export function typecheck(
template: string, source: string, declarations: TestDeclaration[] = [],
additionalSources: {name: AbsoluteFsPath; contents: string}[] = []): ts.Diagnostic[] {
additionalSources: {name: AbsoluteFsPath; contents: string}[] = [],
config: Partial<TypeCheckingConfig> = {}, opts: ts.CompilerOptions = {}): ts.Diagnostic[] {
const typeCheckFilePath = absoluteFrom('/_typecheck_.ts');
const files = [
typescriptLibDts(),
@ -243,7 +244,7 @@ export function typecheck(
...additionalSources,
];
const {program, host, options} =
makeProgram(files, {strictNullChecks: true, noImplicitAny: true}, undefined, false);
makeProgram(files, {strictNullChecks: true, noImplicitAny: true, ...opts}, undefined, false);
const sf = program.getSourceFile(absoluteFrom('/main.ts')) !;
const checker = program.getTypeChecker();
const logicalFs = new LogicalFileSystem(getRootDirs(host, options));
@ -254,7 +255,7 @@ export function typecheck(
program, checker, options, host, new TypeScriptReflectionHost(checker)),
new LogicalProjectStrategy(reflectionHost, logicalFs),
]);
const ctx = new TypeCheckContext(ALL_ENABLED_CONFIG, emitter, typeCheckFilePath);
const ctx = new TypeCheckContext({...ALL_ENABLED_CONFIG, ...config}, emitter, typeCheckFilePath);
const templateUrl = 'synthetic.html';
const templateFile = new ParseSourceFile(template, templateUrl);

View File

@ -253,26 +253,26 @@ describe('type check blocks', () => {
const TEMPLATE = `<div dir (dirOutput)="foo($event)"></div>`;
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe($event => (ctx).foo($event));');
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
});
it('should emit a listener function with AnimationEvent for animation events', () => {
const TEMPLATE = `<div (@animation.done)="foo($event)"></div>`;
const block = tcb(TEMPLATE);
expect(block).toContain('($event: animations.AnimationEvent) => (ctx).foo($event);');
expect(block).toContain('($event: animations.AnimationEvent): any => (ctx).foo($event);');
});
it('should emit addEventListener calls for unclaimed outputs', () => {
const TEMPLATE = `<div (event)="foo($event)"></div>`;
const block = tcb(TEMPLATE);
expect(block).toContain('_t1.addEventListener("event", $event => (ctx).foo($event));');
expect(block).toContain('_t1.addEventListener("event", ($event): any => (ctx).foo($event));');
});
it('should allow to cast $event using $any', () => {
const TEMPLATE = `<div (event)="foo($any($event))"></div>`;
const block = tcb(TEMPLATE);
expect(block).toContain(
'_t1.addEventListener("event", $event => (ctx).foo(($event as any)));');
'_t1.addEventListener("event", ($event): any => (ctx).foo(($event as any)));');
});
});
@ -374,18 +374,18 @@ describe('type check blocks', () => {
it('should check types of directive outputs when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe($event => (ctx).foo($event));');
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", $event => (ctx).foo($event));');
'_t1.addEventListener("nonDirOutput", ($event): any => (ctx).foo($event));');
});
it('should not check types of directive outputs when disabled', () => {
const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfOutputEvents: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('($event: any) => (ctx).foo($event);');
expect(block).toContain('($event: any): any => (ctx).foo($event);');
// Note that DOM events are still checked, that is controlled by `checkTypeOfDomEvents`
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", $event => (ctx).foo($event));');
'_t1.addEventListener("nonDirOutput", ($event): any => (ctx).foo($event));');
});
});
@ -394,13 +394,13 @@ describe('type check blocks', () => {
it('should check types of animation events when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('($event: animations.AnimationEvent) => (ctx).foo($event);');
expect(block).toContain('($event: animations.AnimationEvent): any => (ctx).foo($event);');
});
it('should not check types of animation events when disabled', () => {
const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfAnimationEvents: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('($event: any) => (ctx).foo($event);');
expect(block).toContain('($event: any): any => (ctx).foo($event);');
});
});
@ -410,9 +410,9 @@ describe('type check blocks', () => {
it('should check types of DOM events when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe($event => (ctx).foo($event));');
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", $event => (ctx).foo($event));');
'_t1.addEventListener("nonDirOutput", ($event): any => (ctx).foo($event));');
});
it('should not check types of DOM events when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfDomEvents: false};
@ -420,8 +420,8 @@ describe('type check blocks', () => {
// Note that directive outputs are still checked, that is controlled by
// `checkTypeOfOutputEvents`
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe($event => (ctx).foo($event));');
expect(block).toContain('($event: any) => (ctx).foo($event);');
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain('($event: any): any => (ctx).foo($event);');
});
});