From 6958d11d958f5e6002c4de4ba95494c5f8f34aa6 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 12 Oct 2019 18:38:47 +0200 Subject: [PATCH] feat(ivy): type checking of event bindings (#33125) Until now, the template type checker has not checked any of the event bindings that could be present on an element, for example ``` ``` has two event bindings: the `change` event corresponding with an `@Output()` on the `my-cmp` component and the `click` DOM event. This commit adds functionality to the template type checker in order to type check both kind of event bindings. This means that the correctness of the bindings expressions, as well as the type of the `$event` variable will now be taken into account during template type checking. Resolves FW-1598 PR Close #33125 --- packages/compiler-cli/src/ngtsc/program.ts | 10 + .../src/ngtsc/typecheck/src/api.ts | 28 +- .../src/ngtsc/typecheck/src/diagnostics.ts | 2 + .../src/ngtsc/typecheck/src/environment.ts | 112 ++++- .../src/ngtsc/typecheck/src/expression.ts | 37 +- .../ngtsc/typecheck/src/type_check_block.ts | 437 ++++++++++++++---- .../ngtsc/typecheck/src/type_check_file.ts | 3 + .../ngtsc/typecheck/test/diagnostics_spec.ts | 56 +++ .../typecheck/test/span_comments_spec.ts | 25 + .../src/ngtsc/typecheck/test/test_utils.ts | 58 ++- .../typecheck/test/type_check_block_spec.ts | 97 ++++ .../test/ngtsc/fake_core/index.ts | 4 + .../test/ngtsc/template_typecheck_spec.ts | 36 ++ packages/core/test/bundling/todo/index.ts | 6 +- .../core/test/bundling/todo_i18n/index.ts | 6 +- 15 files changed, 787 insertions(+), 130 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index f4c59ae89f..c3d0aea40b 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -405,6 +405,13 @@ export class NgtscProgram implements api.Program { strictNullInputBindings: true, // Even in full template type-checking mode, DOM binding checks are not quite ready yet. checkTypeOfDomBindings: false, + checkTypeOfOutputEvents: true, + checkTypeOfAnimationEvents: true, + // Checking of DOM events currently has an adverse effect on developer experience, + // e.g. for `` enabling this check results in: + // - error TS2531: Object is possibly 'null'. + // - error TS2339: Property 'value' does not exist on type 'EventTarget'. + checkTypeOfDomEvents: false, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -416,6 +423,9 @@ export class NgtscProgram implements api.Program { checkTypeOfInputBindings: false, strictNullInputBindings: false, checkTypeOfDomBindings: false, + checkTypeOfOutputEvents: false, + checkTypeOfAnimationEvents: false, + checkTypeOfDomEvents: false, checkTypeOfPipes: false, strictSafeNavigationTypes: false, }; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index 9653d492c1..4c9138d4a3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -107,6 +107,32 @@ export interface TypeCheckingConfig { */ checkTypeOfDomBindings: boolean; + /** + * Whether to infer the type of the `$event` variable in event bindings for directive outputs. + * + * 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`. + */ + checkTypeOfOutputEvents: boolean; + + /** + * Whether to infer the type of the `$event` variable in event bindings for animations. + * + * If this is `true`, the type of `$event` will be `AnimationEvent` from `@angular/animations`. + * If set to `false`, the `$event` variable will be of type `any`. + */ + checkTypeOfAnimationEvents: 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`. + */ + checkTypeOfDomEvents: boolean; + /** * Whether to include type information from pipes in the type-checking operation. * @@ -186,4 +212,4 @@ export interface ExternalTemplateSourceMapping { node: ts.Expression; template: string; templateUrl: string; -} \ No newline at end of file +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 3f6c1a002e..dae298b33f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -110,6 +110,8 @@ export function shouldReportDiagnostic(diagnostic: ts.Diagnostic): boolean { return false; } else if (code === 2695 /* Left side of comma operator is unused and has no side effects. */) { return false; + } else if (code === 7006 /* Parameter '$event' implicitly has an 'any' type. */) { + return false; } return true; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index e14b8a777c..b6052c6f83 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {DYNAMIC_TYPE, ExpressionType, ExternalExpr, Type} from '@angular/compiler'; +import {ExpressionType, ExternalExpr, ReadVarExpr, Type} from '@angular/compiler'; import * as ts from 'typescript'; import {NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../imports'; @@ -40,6 +40,9 @@ export class Environment { private pipeInsts = new Map(); protected pipeInstStatements: ts.Statement[] = []; + private outputHelperIdent: ts.Identifier|null = null; + protected helperStatements: ts.Statement[] = []; + constructor( readonly config: TypeCheckingConfig, protected importManager: ImportManager, private refEmitter: ReferenceEmitter, protected contextFile: ts.SourceFile) {} @@ -105,6 +108,92 @@ export class Environment { return pipeInstId; } + /** + * Declares a helper function to be able to cast directive outputs of type `EventEmitter` to + * have an accurate `subscribe()` method that properly carries over the generic type `T` into the + * listener function passed as argument to `subscribe`. This is done to work around a typing + * deficiency in `EventEmitter.subscribe`, where the listener function is typed as any. + */ + declareOutputHelper(): ts.Expression { + if (this.outputHelperIdent !== null) { + return this.outputHelperIdent; + } + + const eventEmitter = this.referenceExternalType( + '@angular/core', 'EventEmitter', [new ExpressionType(new ReadVarExpr('T'))]); + + const outputHelperIdent = ts.createIdentifier('_outputHelper'); + const genericTypeDecl = ts.createTypeParameterDeclaration('T'); + const genericTypeRef = ts.createTypeReferenceNode('T', /* typeParameters */ undefined); + + // Declare a type that has a `subscribe` method that carries over type `T` as parameter + // into the callback. The below code generates the following type literal: + // `{subscribe(cb: (event: T) => any): void;}` + const observableLike = ts.createTypeLiteralNode([ts.createMethodSignature( + /* typeParameters */ undefined, + /* parameters */[ts.createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + /* name */ 'cb', + /* questionToken */ undefined, + /* type */ ts.createFunctionTypeNode( + /* typeParameters */ undefined, + /* parameters */[ts.createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + /* name */ 'event', + /* questionToken */ undefined, + /* type */ genericTypeRef)], + /* type */ ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)))], + /* type */ ts.createKeywordTypeNode(ts.SyntaxKind.VoidKeyword), + /* name */ 'subscribe', + /* questionToken */ undefined)]); + + // Declares the first signature of `_outputHelper` that matches arguments of type + // `EventEmitter`, to convert them into `observableLike` defined above. The following + // statement is generated: + // `declare function _outputHelper(output: EventEmitter): observableLike;` + this.helperStatements.push(ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */[ts.createModifier(ts.SyntaxKind.DeclareKeyword)], + /* asteriskToken */ undefined, + /* name */ outputHelperIdent, + /* typeParameters */[genericTypeDecl], + /* parameters */[ts.createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + /* name */ 'output', + /* questionToken */ undefined, + /* type */ eventEmitter)], + /* type */ observableLike, + /* body */ undefined)); + + // Declares the second signature of `_outputHelper` that matches all other argument types, + // i.e. ensures type identity for output types other than `EventEmitter`. This corresponds + // with the following statement: + // `declare function _outputHelper(output: T): T;` + this.helperStatements.push(ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */[ts.createModifier(ts.SyntaxKind.DeclareKeyword)], + /* asteriskToken */ undefined, + /* name */ outputHelperIdent, + /* typeParameters */[genericTypeDecl], + /* parameters */[ts.createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + /* name */ 'output', + /* questionToken */ undefined, + /* type */ genericTypeRef)], + /* type */ genericTypeRef, + /* body */ undefined)); + + return this.outputHelperIdent = outputHelperIdent; + } + /** * Generate a `ts.Expression` that references the given node. * @@ -131,28 +220,19 @@ export class Environment { } /** - * Generate a `ts.TypeNode` that references a given type from '@angular/core'. + * Generate a `ts.TypeNode` that references a given type from the provided module. * - * This will involve importing the type into the file, and will also add a number of generic type - * parameters (using `any`) as requested. + * This will involve importing the type into the file, and will also add type parameters if + * provided. */ - referenceCoreType(name: string, typeParamCount: number = 0): ts.TypeNode { - const external = new ExternalExpr({ - moduleName: '@angular/core', - name, - }); - let typeParams: Type[]|null = null; - if (typeParamCount > 0) { - typeParams = []; - for (let i = 0; i < typeParamCount; i++) { - typeParams.push(DYNAMIC_TYPE); - } - } + referenceExternalType(moduleName: string, name: string, typeParams?: Type[]): ts.TypeNode { + const external = new ExternalExpr({moduleName, name}); return translateType(new ExpressionType(external, null, typeParams), this.importManager); } getPreludeStatements(): ts.Statement[] { return [ + ...this.helperStatements, ...this.pipeInstStatements, ...this.typeCtorStatements, ]; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 1f98c37cd2..7b42cef1f3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -86,7 +86,12 @@ class AstTranslator implements AstVisitor { return node; } - visitChain(ast: Chain): never { throw new Error('Method not implemented.'); } + visitChain(ast: Chain): ts.Expression { + const elements = ast.expressions.map(expr => this.translate(expr)); + const node = wrapForDiagnostics(ts.createCommaList(elements)); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; + } visitConditional(ast: Conditional): ts.Expression { const condExpr = this.translate(ast.condition); @@ -97,7 +102,13 @@ class AstTranslator implements AstVisitor { return node; } - visitFunctionCall(ast: FunctionCall): never { throw new Error('Method not implemented.'); } + visitFunctionCall(ast: FunctionCall): ts.Expression { + const receiver = wrapForDiagnostics(this.translate(ast.target !)); + const args = ast.args.map(expr => this.translate(expr)); + const node = ts.createCall(receiver, undefined, args); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; + } visitImplicitReceiver(ast: ImplicitReceiver): never { throw new Error('Method not implemented.'); @@ -120,7 +131,16 @@ class AstTranslator implements AstVisitor { return node; } - visitKeyedWrite(ast: KeyedWrite): never { throw new Error('Method not implemented.'); } + visitKeyedWrite(ast: KeyedWrite): ts.Expression { + const receiver = wrapForDiagnostics(this.translate(ast.obj)); + const left = ts.createElementAccess(receiver, this.translate(ast.key)); + // TODO(joost): annotate `left` with the span of the element access, which is not currently + // available on `ast`. + const right = this.translate(ast.value); + const node = wrapForDiagnostics(ts.createBinary(left, ts.SyntaxKind.EqualsToken, right)); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; + } visitLiteralArray(ast: LiteralArray): ts.Expression { const elements = ast.expressions.map(expr => this.translate(expr)); @@ -186,7 +206,16 @@ class AstTranslator implements AstVisitor { return node; } - visitPropertyWrite(ast: PropertyWrite): never { throw new Error('Method not implemented.'); } + visitPropertyWrite(ast: PropertyWrite): ts.Expression { + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); + const left = ts.createPropertyAccess(receiver, ast.name); + // TODO(joost): annotate `left` with the span of the property access, which is not currently + // available on `ast`. + const right = this.translate(ast.value); + const node = wrapForDiagnostics(ts.createBinary(left, ts.SyntaxKind.EqualsToken, right)); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; + } visitQuote(ast: Quote): never { throw new Error('Method not implemented.'); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index ed37665032..49484e9a20 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, PropertyRead, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, ParsedEventType, PropertyRead, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; @@ -421,6 +421,127 @@ class TcbUnclaimedInputsOp extends TcbOp { } } +/** + * A `TcbOp` which generates code to check event bindings on an element that correspond with the + * outputs of a directive. + * + * Executing this operation returns nothing. + */ +class TcbDirectiveOutputsOp extends TcbOp { + constructor( + private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, + private dir: TypeCheckableDirectiveMeta) { + super(); + } + + execute(): null { + const dirId = this.scope.resolve(this.node, this.dir); + + // `dir.outputs` is an object map of field names on the directive class to event names. + // This is backwards from what's needed to match event handlers - a map of event names to field + // names is desired. Invert `dir.outputs` into `fieldByEventName` to create this map. + const fieldByEventName = new Map(); + const outputs = this.dir.outputs; + for (const key of Object.keys(outputs)) { + fieldByEventName.set(outputs[key], key); + } + + for (const output of this.node.outputs) { + if (output.type !== ParsedEventType.Regular || !fieldByEventName.has(output.name)) { + continue; + } + const field = fieldByEventName.get(output.name) !; + + if (this.tcb.env.config.checkTypeOfOutputEvents) { + // For strict checking of directive events, generate a call to the `subscribe` method + // on the directive's output field to let type information flow into the handler function's + // `$event` parameter. + // + // Note that the `EventEmitter` type from '@angular/core' that is typically used for + // outputs has a typings deficiency in its `subscribe` method. The generic type `T` is not + // carried into the handler function, which is vital for inference of the type of `$event`. + // As a workaround, the directive's field is passed into a helper function that has a + // specially crafted set of signatures, to effectively cast `EventEmitter` to something + // that has a `subscribe` method that properly carries the `T` into the handler function. + const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer); + + const outputField = ts.createPropertyAccess(dirId, field); + const outputHelper = + ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]); + const subscribeFn = ts.createPropertyAccess(outputHelper, 'subscribe'); + const call = ts.createCall(subscribeFn, /* typeArguments */ undefined, [handler]); + addParseSpanInfo(call, output.sourceSpan); + this.scope.addStatement(ts.createExpressionStatement(call)); + } else { + // If strict checking of directive events is disabled, emit a handler function where the + // `$event` parameter has an explicit `any` type. + const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any); + this.scope.addStatement(ts.createExpressionStatement(handler)); + } + } + + return null; + } +} + +/** + * A `TcbOp` which generates code to check "unclaimed outputs" - event bindings on an element which + * were not attributed to any directive or component, and are instead processed against the HTML + * element itself. + * + * Executing this operation returns nothing. + */ +class TcbUnclaimedOutputsOp extends TcbOp { + constructor( + private tcb: Context, private scope: Scope, private element: TmplAstElement, + private claimedOutputs: Set) { + super(); + } + + execute(): null { + const elId = this.scope.resolve(this.element); + + // TODO(alxhub): this could be more efficient. + for (const output of this.element.outputs) { + if (this.claimedOutputs.has(output.name)) { + // Skip this event handler as it was claimed by a directive. + continue; + } + + if (output.type === ParsedEventType.Animation) { + // Animation output bindings always have an `$event` parameter of type `AnimationEvent`. + const eventType = this.tcb.env.config.checkTypeOfAnimationEvents ? + this.tcb.env.referenceExternalType('@angular/animations', 'AnimationEvent') : + EventParamType.Any; + + const handler = tcbCreateEventHandler(output, this.tcb, this.scope, eventType); + this.scope.addStatement(ts.createExpressionStatement(handler)); + } else if (this.tcb.env.config.checkTypeOfDomEvents) { + // If strict checking of DOM events is enabled, generate a call to `addEventListener` on + // the element instance so that TypeScript's type inference for + // `HTMLElement.addEventListener` using `HTMLElementEventMap` to infer an accurate type for + // `$event` depending on the event name. For unknown event names, TypeScript resorts to the + // base `Event` type. + const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer); + + const call = ts.createCall( + /* expression */ ts.createPropertyAccess(elId, 'addEventListener'), + /* typeArguments */ undefined, + /* arguments */[ts.createStringLiteral(output.name), handler]); + addParseSpanInfo(call, output.sourceSpan); + this.scope.addStatement(ts.createExpressionStatement(call)); + } else { + // If strict checking of DOM inputs is disabled, emit a handler function where the `$event` + // parameter has an explicit `any` type. + const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any); + this.scope.addStatement(ts.createExpressionStatement(handler)); + } + } + + return null; + } +} + /** * Value used to break a circular reference between `TcbOp`s. * @@ -672,12 +793,14 @@ class Scope { const opIndex = this.opQueue.push(new TcbElementOp(this.tcb, this, node)) - 1; this.elementOpMap.set(node, opIndex); this.appendDirectivesAndInputsOfNode(node); + this.appendOutputsOfNode(node); for (const child of node.children) { this.appendNode(child); } } else if (node instanceof TmplAstTemplate) { // Template children are rendered in a child scope. this.appendDirectivesAndInputsOfNode(node); + this.appendOutputsOfNode(node); if (this.tcb.env.config.checkTemplateBodies) { const ctxIndex = this.opQueue.push(new TcbTemplateContextOp(this.tcb, this)) - 1; this.templateCtxOpMap.set(node, ctxIndex); @@ -730,6 +853,38 @@ class Scope { this.opQueue.push(new TcbDomSchemaCheckerOp(this.tcb, node, checkElement, claimedInputs)); } } + + private appendOutputsOfNode(node: TmplAstElement|TmplAstTemplate): void { + // Collect all the outputs on the element. + const claimedOutputs = new Set(); + const directives = this.tcb.boundTarget.getDirectivesOfNode(node); + if (directives === null || directives.length === 0) { + // If there are no directives, then all outputs are unclaimed outputs, so queue an operation + // to add them if needed. + if (node instanceof TmplAstElement) { + this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs)); + } + return; + } + + // Queue operations for all directives to check the relevant outputs for a directive. + for (const dir of directives) { + this.opQueue.push(new TcbDirectiveOutputsOp(this.tcb, this, node, dir)); + } + + // After expanding the directives, we might need to queue an operation to check any unclaimed + // outputs. + if (node instanceof TmplAstElement) { + // Go through the directives and register any outputs that it claims in `claimedOutputs`. + for (const dir of directives) { + for (const outputField of Object.keys(dir.outputs)) { + claimedOutputs.add(dir.outputs[outputField]); + } + } + + this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs)); + } + } } /** @@ -763,13 +918,122 @@ function tcbCtxParam( */ function tcbExpression( ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression { - const translateSpan = (span: ParseSpan) => toAbsoluteSpan(span, sourceSpan); + const translator = new TcbExpressionTranslator(tcb, scope, sourceSpan); + return translator.translate(ast); +} - // `astToTypescript` actually does the conversion. A special resolver `tcbResolve` is passed which - // interprets specific expression nodes that interact with the `ImplicitReceiver`. These nodes - // actually refer to identifiers within the current scope. - return astToTypescript( - ast, (ast) => tcbResolve(ast, tcb, scope, sourceSpan), tcb.env.config, translateSpan); +class TcbExpressionTranslator { + constructor( + protected tcb: Context, protected scope: Scope, protected sourceSpan: ParseSourceSpan) {} + + translate(ast: AST): ts.Expression { + // `astToTypescript` actually does the conversion. A special resolver `tcbResolve` is passed + // which interprets specific expression nodes that interact with the `ImplicitReceiver`. These + // nodes actually refer to identifiers within the current scope. + return astToTypescript( + ast, ast => this.resolve(ast), this.tcb.env.config, + (span: ParseSpan) => toAbsoluteSpan(span, this.sourceSpan)); + } + + /** + * Resolve an `AST` expression within the given scope. + * + * Some `AST` expressions refer to top-level concepts (references, variables, the component + * context). This method assists in resolving those. + */ + protected resolve(ast: AST): ts.Expression|null { + if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) { + // Check whether the template metadata has bound a target for this expression. If so, then + // resolve that target. If not, then the expression is referencing the top-level component + // context. + const binding = this.tcb.boundTarget.getExpressionTarget(ast); + if (binding !== null) { + // This expression has a binding to some variable or reference in the template. Resolve it. + if (binding instanceof TmplAstVariable) { + const expr = ts.getMutableClone(this.scope.resolve(binding)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + return expr; + } else if (binding instanceof TmplAstReference) { + const target = this.tcb.boundTarget.getReferenceTarget(binding); + if (target === null) { + throw new Error(`Unbound reference? ${binding.name}`); + } + + // The reference is either to an element, an node, or to a directive on an + // element or template. + + if (target instanceof TmplAstElement) { + const expr = ts.getMutableClone(this.scope.resolve(target)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + return expr; + } else if (target instanceof TmplAstTemplate) { + // Direct references to an node simply require a value of type + // `TemplateRef`. To get this, an expression of the form + // `(null as any as TemplateRef)` is constructed. + let value: ts.Expression = ts.createNull(); + value = + ts.createAsExpression(value, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + value = ts.createAsExpression( + value, + this.tcb.env.referenceExternalType('@angular/core', 'TemplateRef', [DYNAMIC_TYPE])); + value = ts.createParen(value); + addParseSpanInfo(value, toAbsoluteSpan(ast.span, this.sourceSpan)); + return value; + } else { + const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + return expr; + } + } else { + throw new Error(`Unreachable: ${binding}`); + } + } else { + // This is a PropertyRead(ImplicitReceiver) and probably refers to a property access on the + // component context. Let it fall through resolution here so it will be caught when the + // ImplicitReceiver is resolved in the branch below. + return null; + } + } else if (ast instanceof ImplicitReceiver) { + // AST instances representing variables and references look very similar to property reads + // from the component context: both have the shape + // PropertyRead(ImplicitReceiver, 'propertyName'). + // + // `tcbExpression` will first try to `tcbResolve` the outer PropertyRead. If this works, it's + // because the `BoundTarget` found an expression target for the whole expression, and + // therefore `tcbExpression` will never attempt to `tcbResolve` the ImplicitReceiver of that + // PropertyRead. + // + // Therefore if `tcbResolve` is called on an `ImplicitReceiver`, it's because no outer + // PropertyRead resolved to a variable or reference, and therefore this is a property read on + // the component context itself. + return ts.createIdentifier('ctx'); + } else if (ast instanceof BindingPipe) { + const expr = this.translate(ast.exp); + let pipe: ts.Expression; + if (this.tcb.env.config.checkTypeOfPipes) { + pipe = this.tcb.getPipeByName(ast.name); + } else { + pipe = ts.createParen(ts.createAsExpression( + ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword))); + } + const args = ast.args.map(arg => this.translate(arg)); + const result = tsCallMethod(pipe, 'transform', [expr, ...args]); + addParseSpanInfo(result, toAbsoluteSpan(ast.span, this.sourceSpan)); + return result; + } else if ( + ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver && + ast.name === '$any' && ast.args.length === 1) { + const expr = this.translate(ast.args[0]); + const exprAsAny = + ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + const result = ts.createParen(exprAsAny); + addParseSpanInfo(result, toAbsoluteSpan(ast.span, this.sourceSpan)); + return result; + } else { + // This AST isn't special after all. + return null; + } + } } /** @@ -889,99 +1153,82 @@ function tcbGetDirectiveInputs( } } +const EVENT_PARAMETER = '$event'; + +const enum EventParamType { + /* Generates code to infer the type of `$event` based on how the listener is registered. */ + Infer, + + /* Declares the type of the `$event` parameter as `any`. */ + Any, +} + /** - * Resolve an `AST` expression within the given scope. + * Creates an arrow function to be used as handler function for event bindings. The handler + * function has a single parameter `$event` and the bound event's handler `AST` represented as a + * TypeScript expression as its body. * - * Some `AST` expressions refer to top-level concepts (references, variables, the component - * context). This method assists in resolving those. + * When `eventType` is set to `Infer`, the `$event` parameter will not have an explicit type. This + * allows for the created handler function to have its `$event` parameter's type inferred based on + * how it's used, to enable strict type checking of event bindings. When set to `Any`, the `$event` + * parameter will have an explicit `any` type, effectively disabling strict type checking of event + * bindings. Alternatively, an explicit type can be passed for the `$event` parameter. */ -function tcbResolve( - ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression|null { - if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) { - // Check whether the template metadata has bound a target for this expression. If so, then - // resolve that target. If not, then the expression is referencing the top-level component - // context. - const binding = tcb.boundTarget.getExpressionTarget(ast); - if (binding !== null) { - // This expression has a binding to some variable or reference in the template. Resolve it. - if (binding instanceof TmplAstVariable) { - const expr = ts.getMutableClone(scope.resolve(binding)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); - return expr; - } else if (binding instanceof TmplAstReference) { - const target = tcb.boundTarget.getReferenceTarget(binding); - if (target === null) { - throw new Error(`Unbound reference? ${binding.name}`); - } +function tcbCreateEventHandler( + event: TmplAstBoundEvent, tcb: Context, scope: Scope, + eventType: EventParamType | ts.TypeNode): ts.ArrowFunction { + const handler = tcbEventHandlerExpression(event.handler, tcb, scope, event.handlerSpan); - // The reference is either to an element, an node, or to a directive on an - // element or template. - - if (target instanceof TmplAstElement) { - const expr = ts.getMutableClone(scope.resolve(target)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); - return expr; - } else if (target instanceof TmplAstTemplate) { - // Direct references to an node simply require a value of type - // `TemplateRef`. To get this, an expression of the form - // `(null as any as TemplateRef)` is constructed. - let value: ts.Expression = ts.createNull(); - value = ts.createAsExpression(value, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); - value = ts.createAsExpression(value, tcb.env.referenceCoreType('TemplateRef', 1)); - value = ts.createParen(value); - addParseSpanInfo(value, toAbsoluteSpan(ast.span, sourceSpan)); - return value; - } else { - const expr = ts.getMutableClone(scope.resolve(target.node, target.directive)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); - return expr; - } - } else { - throw new Error(`Unreachable: ${binding}`); - } - } else { - // This is a PropertyRead(ImplicitReceiver) and probably refers to a property access on the - // component context. Let it fall through resolution here so it will be caught when the - // ImplicitReceiver is resolved in the branch below. - return null; - } - } else if (ast instanceof ImplicitReceiver) { - // AST instances representing variables and references look very similar to property reads from - // the component context: both have the shape PropertyRead(ImplicitReceiver, 'propertyName'). - // - // `tcbExpression` will first try to `tcbResolve` the outer PropertyRead. If this works, it's - // because the `BoundTarget` found an expression target for the whole expression, and therefore - // `tcbExpression` will never attempt to `tcbResolve` the ImplicitReceiver of that PropertyRead. - // - // Therefore if `tcbResolve` is called on an `ImplicitReceiver`, it's because no outer - // PropertyRead resolved to a variable or reference, and therefore this is a property read on - // the component context itself. - return ts.createIdentifier('ctx'); - } else if (ast instanceof BindingPipe) { - const expr = tcbExpression(ast.exp, tcb, scope, sourceSpan); - let pipe: ts.Expression; - if (tcb.env.config.checkTypeOfPipes) { - pipe = tcb.getPipeByName(ast.name); - } else { - pipe = ts.createParen(ts.createAsExpression( - ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword))); - } - const args = ast.args.map(arg => tcbExpression(arg, tcb, scope, sourceSpan)); - const result = tsCallMethod(pipe, 'transform', [expr, ...args]); - addParseSpanInfo(result, toAbsoluteSpan(ast.span, sourceSpan)); - return result; - } else if ( - ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver && - ast.name === '$any' && ast.args.length === 1) { - const expr = tcbExpression(ast.args[0], tcb, scope, sourceSpan); - const exprAsAny = - ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); - const result = ts.createParen(exprAsAny); - addParseSpanInfo(result, toAbsoluteSpan(ast.span, sourceSpan)); - return result; + let eventParamType: ts.TypeNode|undefined; + if (eventType === EventParamType.Infer) { + eventParamType = undefined; + } else if (eventType === EventParamType.Any) { + eventParamType = ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword); } else { - // This AST isn't special after all. - return null; + eventParamType = eventType; + } + + const eventParam = ts.createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + /* name */ EVENT_PARAMETER, + /* questionToken */ undefined, + /* type */ eventParamType); + return ts.createArrowFunction( + /* modifier */ undefined, + /* typeParameters */ undefined, + /* parameters */[eventParam], + /* type */ undefined, + /* equalsGreaterThanToken*/ undefined, + /* body */ handler); +} + +/** + * Similar to `tcbExpression`, this function converts the provided `AST` expression into a + * `ts.Expression`, with special handling of the `$event` variable that can be used within event + * bindings. + */ +function tcbEventHandlerExpression( + ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression { + const translator = new TcbEventHandlerTranslator(tcb, scope, sourceSpan); + return translator.translate(ast); +} + +class TcbEventHandlerTranslator extends TcbExpressionTranslator { + protected resolve(ast: AST): ts.Expression|null { + // Recognize a property read on the implicit receiver corresponding with the event parameter + // that is available in event bindings. Since this variable is a parameter of the handler + // function that the converted expression becomes a child of, just create a reference to the + // parameter by its name. + if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver && + ast.name === EVENT_PARAMETER) { + const event = ts.createIdentifier(EVENT_PARAMETER); + addParseSpanInfo(event, toAbsoluteSpan(ast.span, this.sourceSpan)); + return event; + } + + return super.resolve(ast); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index a2ee6c787b..5875911fd2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -51,6 +51,9 @@ export class TypeCheckFile extends Environment { '\n\n'; const printer = ts.createPrinter(); source += '\n'; + for (const stmt of this.helperStatements) { + source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n'; + } for (const stmt of this.pipeInstStatements) { source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n'; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index cc3ae3584e..8d0fddd32a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -185,6 +185,62 @@ runInEachFileSystem(() => { expect(messages).toEqual([]); }); + describe('outputs', () => { + it('should produce a diagnostic for directive outputs', () => { + const messages = diagnose( + `
`, ` + import {EventEmitter} from '@angular/core'; + class Dir { + out = new EventEmitter(); + } + class TestComponent { + handleEvent(event: string): void {} + }`, + [{type: 'directive', name: 'Dir', selector: '[dir]', outputs: {'out': 'event'}}]); + + expect(messages).toEqual([ + `synthetic.html(1, 31): Argument of type 'number' is not assignable to parameter of type 'string'.`, + ]); + }); + + it('should produce a diagnostic for animation events', () => { + const messages = diagnose(`
`, ` + class TestComponent { + handleEvent(event: string): void {} + }`); + + expect(messages).toEqual([ + `synthetic.html(1, 41): Argument of type 'AnimationEvent' is not assignable to parameter of type 'string'.`, + ]); + }); + + it('should produce a diagnostic for element outputs', () => { + const messages = diagnose(`
`, ` + import {EventEmitter} from '@angular/core'; + class TestComponent { + handleEvent(event: string): void {} + }`); + + expect(messages).toEqual([ + `synthetic.html(1, 27): Argument of type 'MouseEvent' is not assignable to parameter of type 'string'.`, + ]); + }); + + it('should not produce a diagnostic when $event implicitly has an any type', () => { + const messages = diagnose( + `
`, ` + class Dir { + out: any; + } + class TestComponent { + handleEvent(event: string): void {} + }`, + [{type: 'directive', name: 'Dir', selector: '[dir]', outputs: {'out': 'event'}}]); + + expect(messages).toEqual([]); + }); + }); + describe('strict null checks', () => { it('produces diagnostic for unchecked property access', () => { const messages = diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts index 863f37d2e6..28d843dcce 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts @@ -59,16 +59,35 @@ describe('type check blocks diagnostics', () => { .toContain('(ctx).method((ctx).a /*10,11*/, (ctx).b /*13,14*/) /*3,16*/;'); }); + it('should annotate function calls', () => { + const TEMPLATE = `{{ method(a)(b, c) }}`; + expect(tcbWithSpans(TEMPLATE)) + .toContain( + '((ctx).method((ctx).a /*10,11*/) /*3,12*/)((ctx).b /*13,14*/, (ctx).c /*16,17*/) /*3,19*/;'); + }); + it('should annotate property access', () => { const TEMPLATE = `{{ a.b.c }}`; expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/).b /*3,6*/).c /*3,9*/;'); }); + it('should annotate property writes', () => { + const TEMPLATE = `
`; + expect(tcbWithSpans(TEMPLATE)) + .toContain('((((ctx).a /*14,15*/).b /*14,17*/).c = (ctx).d /*22,23*/) /*14,23*/'); + }); + it('should annotate keyed property access', () => { const TEMPLATE = `{{ a[b] }}`; expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*3,4*/)[(ctx).b /*5,6*/] /*3,8*/;'); }); + it('should annotate keyed property writes', () => { + const TEMPLATE = `
`; + expect(tcbWithSpans(TEMPLATE)) + .toContain('(((ctx).a /*14,15*/)[(ctx).b /*16,17*/] = (ctx).c /*21,22*/) /*14,22*/'); + }); + it('should annotate safe property access', () => { const TEMPLATE = `{{ a?.b }}`; expect(tcbWithSpans(TEMPLATE)) @@ -87,6 +106,12 @@ describe('type check blocks diagnostics', () => { expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*8,9*/ as any) /*3,11*/;'); }); + it('should annotate chained expressions', () => { + const TEMPLATE = `
`; + expect(tcbWithSpans(TEMPLATE)) + .toContain('((ctx).a /*14,15*/, (ctx).b /*17,18*/, (ctx).c /*20,21*/) /*14,21*/'); + }); + it('should annotate pipe usages', () => { const TEMPLATE = `{{ a | test:b }}`; const PIPES: TestDeclaration[] = [{ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 7d8281d5fd..9d443eb2bd 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CssSelector, ParseSourceFile, ParseSourceSpan, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, parseTemplate} from '@angular/compiler'; +import {CssSelector, ParseSourceFile, ParseSourceSpan, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, Type, parseTemplate} from '@angular/compiler'; import * as ts from 'typescript'; import {AbsoluteFsPath, LogicalFileSystem, absoluteFrom} from '../../file_system'; @@ -40,7 +40,21 @@ export function typescriptLibDts(): TestFile { length: number; } - declare interface HTMLElement {} + declare interface Event { + preventDefault(): void; + } + declare interface MouseEvent extends Event { + readonly x: number; + readonly y: number; + } + + declare interface HTMLElementEventMap { + "click": MouseEvent; + } + declare interface HTMLElement { + addEventListener(type: K, listener: (this: HTMLElement, ev: HTMLElementEventMap[K]) => any): void; + addEventListener(type: string, listener: (evt: Event): void;): void; + } declare interface HTMLDivElement extends HTMLElement {} declare interface HTMLImageElement extends HTMLElement { src: string; @@ -73,6 +87,21 @@ export function angularCoreDts(): TestFile { abstract readonly elementRef: unknown; abstract createEmbeddedView(context: C): unknown; } + + export declare class EventEmitter { + subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown; + } + ` + }; +} + +export function angularAnimationsDts(): TestFile { + return { + name: absoluteFrom('/node_modules/@angular/animations/index.d.ts'), + contents: ` + export declare class AnimationEvent { + element: any; + } ` }; } @@ -123,6 +152,9 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { // Feature is still in development. // TODO(alxhub): enable when DOM checking via lib.dom.d.ts is further along. checkTypeOfDomBindings: false, + checkTypeOfOutputEvents: true, + checkTypeOfAnimationEvents: true, + checkTypeOfDomEvents: true, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -163,6 +195,9 @@ export function tcb( checkTypeOfInputBindings: true, strictNullInputBindings: true, checkTypeOfDomBindings: false, + checkTypeOfOutputEvents: true, + checkTypeOfAnimationEvents: true, + checkTypeOfDomEvents: true, checkTypeOfPipes: true, checkTemplateBodies: true, strictSafeNavigationTypes: true, @@ -187,13 +222,15 @@ export function typecheck( const files = [ typescriptLibDts(), angularCoreDts(), + angularAnimationsDts(), // Add the typecheck file to the program, as the typecheck program is created with the // assumption that the typecheck file was already a root file in the original program. {name: typeCheckFilePath, contents: 'export const TYPECHECK = true;'}, {name: absoluteFrom('/main.ts'), contents: source}, ...additionalSources, ]; - const {program, host, options} = makeProgram(files, {strictNullChecks: true}, undefined, false); + const {program, host, options} = + makeProgram(files, {strictNullChecks: true, noImplicitAny: true}, undefined, false); const sf = program.getSourceFile(absoluteFrom('/main.ts')) !; const checker = program.getTypeChecker(); const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); @@ -294,6 +331,8 @@ class FakeEnvironment /* implements Environment */ { return ts.createParen(ts.createAsExpression(ts.createNull(), this.referenceType(ref))); } + declareOutputHelper(): ts.Expression { return ts.createIdentifier('_outputHelper'); } + reference(ref: Reference>): ts.Expression { return ref.node.name; } @@ -302,14 +341,17 @@ class FakeEnvironment /* implements Environment */ { return ts.createTypeReferenceNode(ref.node.name, /* typeArguments */ undefined); } - referenceCoreType(name: string, typeParamCount: number = 0): ts.TypeNode { + referenceExternalType(moduleName: string, name: string, typeParams?: Type[]): ts.TypeNode { const typeArgs: ts.TypeNode[] = []; - for (let i = 0; i < typeParamCount; i++) { - typeArgs.push(ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + if (typeParams !== undefined) { + for (let i = 0; i < typeParams.length; i++) { + typeArgs.push(ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + } } - const qName = ts.createQualifiedName(ts.createIdentifier('ng'), name); - return ts.createTypeReferenceNode(qName, typeParamCount > 0 ? typeArgs : undefined); + const ns = ts.createIdentifier(moduleName.replace('@angular/', '')); + const qName = ts.createQualifiedName(ns, name); + return ts.createTypeReferenceNode(qName, typeArgs.length > 0 ? typeArgs : undefined); } getPreludeStatements(): ts.Statement[] { return []; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index d2bac46b00..030395fb41 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -235,6 +235,42 @@ describe('type check blocks', () => { }); }); + describe('outputs', () => { + + it('should emit subscribe calls for directive outputs', () => { + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + outputs: {'outputField': 'dirOutput'}, + }]; + const TEMPLATE = `
`; + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain( + '_outputHelper(_t2.outputField).subscribe($event => (ctx).foo($event));'); + }); + + it('should emit a listener function with AnimationEvent for animation events', () => { + const TEMPLATE = `
`; + const block = tcb(TEMPLATE); + expect(block).toContain('($event: animations.AnimationEvent) => (ctx).foo($event);'); + }); + + it('should emit addEventListener calls for unclaimed outputs', () => { + const TEMPLATE = `
`; + const block = tcb(TEMPLATE); + expect(block).toContain('_t1.addEventListener("event", $event => (ctx).foo($event));'); + }); + + it('should allow to cast $event using $any', () => { + const TEMPLATE = `
`; + const block = tcb(TEMPLATE); + expect(block).toContain( + '_t1.addEventListener("event", $event => (ctx).foo(($event as any)));'); + }); + + }); + describe('config', () => { const DIRECTIVES: TestDeclaration[] = [{ type: 'directive', @@ -242,6 +278,7 @@ describe('type check blocks', () => { selector: '[dir]', exportAs: ['dir'], inputs: {'dirInput': 'dirInput'}, + outputs: {'outputField': 'dirOutput'}, hasNgTemplateContextGuard: true, }]; const BASE_CONFIG: TypeCheckingConfig = { @@ -251,6 +288,9 @@ describe('type check blocks', () => { checkTypeOfInputBindings: true, strictNullInputBindings: true, checkTypeOfDomBindings: false, + checkTypeOfOutputEvents: true, + checkTypeOfAnimationEvents: true, + checkTypeOfDomEvents: true, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -319,6 +359,63 @@ describe('type check blocks', () => { }); }); + describe('config.checkTypeOfOutputEvents', () => { + const TEMPLATE = `
`; + + 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));'); + expect(block).toContain( + '_t1.addEventListener("nonDirOutput", $event => (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);'); + // Note that DOM events are still checked, that is controlled by `checkTypeOfDomEvents` + expect(block).toContain( + '_t1.addEventListener("nonDirOutput", $event => (ctx).foo($event));'); + }); + }); + + describe('config.checkTypeOfAnimationEvents', () => { + const TEMPLATE = `
`; + + it('should check types of animation events when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('($event: animations.AnimationEvent) => (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);'); + }); + }); + + describe('config.checkTypeOfDomEvents', () => { + const TEMPLATE = `
`; + + 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));'); + expect(block).toContain( + '_t1.addEventListener("nonDirOutput", $event => (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);'); + }); + }); + describe('config.checkTypeOfPipes', () => { const TEMPLATE = `{{a | test:b:c}}`; diff --git a/packages/compiler-cli/test/ngtsc/fake_core/index.ts b/packages/compiler-cli/test/ngtsc/fake_core/index.ts index ddc353bf51..4e494f06d1 100644 --- a/packages/compiler-cli/test/ngtsc/fake_core/index.ts +++ b/packages/compiler-cli/test/ngtsc/fake_core/index.ts @@ -80,3 +80,7 @@ export enum ChangeDetectionStrategy { export const CUSTOM_ELEMENTS_SCHEMA: any = false; export const NO_ERRORS_SCHEMA: any = false; + +export class EventEmitter { + subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown { return null; } +} diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index f6b5dd4da9..2791395b1b 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -106,6 +106,42 @@ export declare class CommonModule { expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); }); + it('should check event bindings', () => { + env.write('test.ts', ` + import {Component, Directive, EventEmitter, NgModule, Output} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + update(data: string) {} + } + + @Directive({selector: '[dir]'}) + class TestDir { + @Output() update = new EventEmitter(); + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].messageText) + .toEqual(`Argument of type 'number' is not assignable to parameter of type 'string'.`); + expect(diags[1].messageText) + .toEqual(`Property 'updated' does not exist on type 'TestCmp'. Did you mean 'update'?`); + // Disabled because `checkTypeOfDomEvents` is disabled by default + // expect(diags[2].messageText) + // .toEqual( + // `Argument of type 'FocusEvent' is not assignable to parameter of type 'string'.`); + expect(diags[2].messageText).toEqual(`Property 'focused' does not exist on type 'TestCmp'.`); + }); + it('should check basic usage of NgIf', () => { env.write('test.ts', ` import {CommonModule} from '@angular/common'; diff --git a/packages/core/test/bundling/todo/index.ts b/packages/core/test/bundling/todo/index.ts index a16c76ecd3..532f3077c5 100644 --- a/packages/core/test/bundling/todo/index.ts +++ b/packages/core/test/bundling/todo/index.ts @@ -87,9 +87,9 @@ class TodoStore { diff --git a/packages/core/test/bundling/todo_i18n/index.ts b/packages/core/test/bundling/todo_i18n/index.ts index 648da7d67f..d57b51649b 100644 --- a/packages/core/test/bundling/todo_i18n/index.ts +++ b/packages/core/test/bundling/todo_i18n/index.ts @@ -82,9 +82,9 @@ class TodoStore {