fix(compiler): support event bindings in `fullTemplateTypeCheck` (#20490)

The type-check block now disables type checking event access instead
of generating a reference to an undefined variable.

PR Close #20490
This commit is contained in:
Chuck Jazdzewski 2017-11-16 10:16:04 -08:00 committed by Miško Hevery
parent 23ab83b504
commit 4ed04392d3
2 changed files with 57 additions and 7 deletions

View File

@ -14,13 +14,16 @@ import * as ts from 'typescript';
import {TestSupport, expectNoDiagnostics, setup} from '../test_support'; import {TestSupport, expectNoDiagnostics, setup} from '../test_support';
type MockFiles = {
[fileName: string]: string
};
describe('ng type checker', () => { describe('ng type checker', () => {
let errorSpy: jasmine.Spy&((s: string) => void); let errorSpy: jasmine.Spy&((s: string) => void);
let testSupport: TestSupport; let testSupport: TestSupport;
function compileAndCheck( function compileAndCheck(
mockDirs: {[fileName: string]: string}[], mockDirs: MockFiles[], overrideOptions: ng.CompilerOptions = {}): ng.Diagnostics {
overrideOptions: ng.CompilerOptions = {}): ng.Diagnostics {
testSupport.writeFiles(...mockDirs); testSupport.writeFiles(...mockDirs);
const fileNames: string[] = []; const fileNames: string[] = [];
mockDirs.forEach((dir) => { mockDirs.forEach((dir) => {
@ -40,13 +43,12 @@ describe('ng type checker', () => {
testSupport = setup(); testSupport = setup();
}); });
function accept( function accept(files: MockFiles = {}, overrideOptions: ng.CompilerOptions = {}) {
files: {[fileName: string]: string} = {}, overrideOptions: ng.CompilerOptions = {}) {
expectNoDiagnostics({}, compileAndCheck([QUICKSTART, files], overrideOptions)); expectNoDiagnostics({}, compileAndCheck([QUICKSTART, files], overrideOptions));
} }
function reject( function reject(
message: string | RegExp, location: RegExp, files: {[fileName: string]: string}, message: string | RegExp, location: RegExp, files: MockFiles,
overrideOptions: ng.CompilerOptions = {}) { overrideOptions: ng.CompilerOptions = {}) {
const diagnostics = compileAndCheck([QUICKSTART, files], overrideOptions); const diagnostics = compileAndCheck([QUICKSTART, files], overrideOptions);
if (!diagnostics || !diagnostics.length) { if (!diagnostics || !diagnostics.length) {
@ -79,6 +81,41 @@ describe('ng type checker', () => {
}); });
}); });
describe('regressions ', () => {
const a = (files: MockFiles, options: object = {}) => {
accept(files, {fullTemplateTypeCheck: true, ...options});
};
// #19905
it('should accept an event binding', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener} from '@angular/core';
@Component({
selector: 'comp',
template: '<div someDir></div>'
})
export class MainComp {}
@Directive({
selector: '[someDir]'
})
export class SomeDirective {
@HostListener('click', ['$event'])
onClick(event: any) {}
}
@NgModule({
declarations: [MainComp, SomeDirective],
})
export class MainModule {}`
});
});
});
describe('with modified quickstart (fullTemplateTypeCheck: false)', () => { describe('with modified quickstart (fullTemplateTypeCheck: false)', () => {
addTests({fullTemplateTypeCheck: false}); addTests({fullTemplateTypeCheck: false});
}); });

View File

@ -69,6 +69,19 @@ interface Expression {
const DYNAMIC_VAR_NAME = '_any'; const DYNAMIC_VAR_NAME = '_any';
class TypeCheckLocalResolver implements LocalResolver {
getLocal(name: string): o.Expression|null {
if (name === EventHandlerVars.event.name) {
// References to the event should not be type-checked.
// TODO(chuckj): determine a better type for the event.
return o.variable(DYNAMIC_VAR_NAME);
}
return null;
}
}
const defaultResolver = new TypeCheckLocalResolver();
class ViewBuilder implements TemplateAstVisitor, LocalResolver { class ViewBuilder implements TemplateAstVisitor, LocalResolver {
private refOutputVars = new Map<string, OutputVarType>(); private refOutputVars = new Map<string, OutputVarType>();
private variables: VariableAst[] = []; private variables: VariableAst[] = [];
@ -112,7 +125,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
this.updates.forEach((expression) => { this.updates.forEach((expression) => {
const {sourceSpan, context, value} = this.preprocessUpdateExpression(expression); const {sourceSpan, context, value} = this.preprocessUpdateExpression(expression);
const bindingId = `${bindingCount++}`; const bindingId = `${bindingCount++}`;
const nameResolver = context === this.component ? this : null; const nameResolver = context === this.component ? this : defaultResolver;
const {stmts, currValExpr} = convertPropertyBinding( const {stmts, currValExpr} = convertPropertyBinding(
nameResolver, o.variable(this.getOutputVar(context)), value, bindingId); nameResolver, o.variable(this.getOutputVar(context)), value, bindingId);
stmts.push(new o.ExpressionStatement(currValExpr)); stmts.push(new o.ExpressionStatement(currValExpr));
@ -122,7 +135,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
this.actions.forEach(({sourceSpan, context, value}) => { this.actions.forEach(({sourceSpan, context, value}) => {
const bindingId = `${bindingCount++}`; const bindingId = `${bindingCount++}`;
const nameResolver = context === this.component ? this : null; const nameResolver = context === this.component ? this : defaultResolver;
const {stmts} = convertActionBinding( const {stmts} = convertActionBinding(
nameResolver, o.variable(this.getOutputVar(context)), value, bindingId); nameResolver, o.variable(this.getOutputVar(context)), value, bindingId);
viewStmts.push(...stmts.map( viewStmts.push(...stmts.map(