Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View file
Open in desktop
Expand Up @@ -1276,7 +1276,7 @@ function tcbCreateEventHandler(
/* modifier */ undefined,
/* typeParameters */ undefined,
/* parameters */[eventParam],
/* type */ undefined,
/* type */ ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword),
/* equalsGreaterThanToken*/ undefined,
/* body */ handler);
}
Expand Down
24 changes: 21 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
View file
Open in desktop
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
View file
Open in desktop
Expand Up @@ -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(),
Expand All @@ -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));
Expand All @@ -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);
Expand Down
28 changes: 14 additions & 14 deletions packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
View file
Open in desktop
Expand Up @@ -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)));');
});

});
Expand Down Expand Up @@ -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));');
});
});

Expand All @@ -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);');
});
});

Expand All @@ -410,18 +410,18 @@ 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};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
// 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);');
});
});

Expand Down