From 5ad2097be857dd037706a83c44016346237f4d59 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 7 Mar 2019 08:31:31 +0000 Subject: [PATCH] fix(ivy): teach template type checker about template attributes (#29041) For the template type checking to work correctly, it needs to know what attributes are bound to expressions or directives, which may require expressions in the template to be evaluated in a different scope. In inline templates, there are attributes that are now marked as "Template" attributes. We need to ensure that the template type checking code looks at these "bound" attributes as well as the "input" attributes. PR Close #29041 --- .../ngtsc/typecheck/src/type_check_block.ts | 47 ++++++++++++------- packages/compiler/src/render3/r3_ast.ts | 2 +- .../compiler/src/render3/view/t2_binder.ts | 41 ++++++++-------- 3 files changed, 50 insertions(+), 40 deletions(-) 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 3dfcc90f13..a928f16898 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, BindingType, BoundTarget, ImplicitReceiver, PropertyRead, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingType, BoundTarget, ImplicitReceiver, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference, ReferenceEmitter} from '../../imports'; @@ -16,6 +16,7 @@ import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api'; import {astToTypescript} from './expression'; + /** * Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a * "type check block" function. @@ -430,7 +431,10 @@ function tcbProcessTemplateDeclaration(tmpl: TmplAstTemplate, tcb: Context, scop // any template guards, and generate them if needed. dir.ngTemplateGuards.forEach(inputName => { // For each template guard function on the directive, look for a binding to that input. - const boundInput = tmpl.inputs.find(i => i.name === inputName); + const boundInput = tmpl.inputs.find(i => i.name === inputName) || + tmpl.templateAttrs.find( + (i: TmplAstTextAttribute | TmplAstBoundAttribute): i is TmplAstBoundAttribute => + i instanceof TmplAstBoundAttribute && i.name === inputName); if (boundInput !== undefined) { // If there is such a binding, generate an expression for it. const expr = tcbExpression(boundInput.value, tcb, scope); @@ -524,20 +528,28 @@ function tcbGetInputBindingExpressions( propMatch.set(inputs[key] as string, key); }); - // Add a binding expression to the map for each input of the directive that has a - // matching binding. - el.inputs.filter(input => propMatch.has(input.name)).forEach(input => { - // Produce an expression representing the value of the binding. - const expr = tcbExpression(input.value, tcb, scope); - - // Call the callback. - bindings.push({ - property: input.name, - field: propMatch.get(input.name) !, - expression: expr, - }); - }); + el.inputs.forEach(processAttribute); + if (el instanceof TmplAstTemplate) { + el.templateAttrs.forEach(processAttribute); + } return bindings; + + /** + * Add a binding expression to the map for each input/template attribute of the directive that has + * a matching binding. + */ + function processAttribute(attr: TmplAstBoundAttribute | TmplAstTextAttribute): void { + if (attr instanceof TmplAstBoundAttribute && propMatch.has(attr.name)) { + // Produce an expression representing the value of the binding. + const expr = tcbExpression(attr.value, tcb, scope); + // Call the callback. + bindings.push({ + property: attr.name, + field: propMatch.get(attr.name) !, + expression: expr, + }); + } + } } /** @@ -633,6 +645,7 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|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. @@ -662,8 +675,8 @@ function tcbResolveVariable(binding: TmplAstVariable, tcb: Context, scope: Scope if (tmpl === null) { throw new Error(`Expected TmplAstVariable to be mapped to a TmplAstTemplate`); } - // Look for a context variable for the template. This should've been declared before anything - // that could reference the template's variables. + // Look for a context variable for the template. This should've been declared before anything that + // could reference the template's variables. const ctx = scope.getTemplateCtx(tmpl); if (ctx === null) { throw new Error('Expected template context to exist.'); diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index e51bd6ff89..a5a3c17f6b 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -246,4 +246,4 @@ export function transformAll( changed = changed || newNode != node; } return changed ? result : nodes; -} \ No newline at end of file +} diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index fdab572717..d67b353551 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -282,27 +282,21 @@ class DirectiveBinder implements Visitor { } }); - // Associate bindings on the node with directives or with the node itself. - - // Inputs: - [...node.attributes, ...node.inputs].forEach(binding => { - let dir = directives.find(dir => dir.inputs.hasOwnProperty(binding.name)); + // Associate attributes/bindings on the node with directives or with the node itself. + const processAttribute = (attribute: BoundAttribute | BoundEvent | TextAttribute) => { + let dir = directives.find(dir => dir.inputs.hasOwnProperty(attribute.name)); if (dir !== undefined) { - this.bindings.set(binding, dir); + this.bindings.set(attribute, dir); } else { - this.bindings.set(binding, node); + this.bindings.set(attribute, node); } - }); - - // Outputs: - node.outputs.forEach(binding => { - let dir = directives.find(dir => dir.outputs.hasOwnProperty(binding.name)); - if (dir !== undefined) { - this.bindings.set(binding, dir); - } else { - this.bindings.set(binding, node); - } - }); + }; + node.attributes.forEach(processAttribute); + node.inputs.forEach(processAttribute); + node.outputs.forEach(processAttribute); + if (node instanceof Template) { + node.templateAttrs.forEach(processAttribute); + } // Recurse into the node's children. node.children.forEach(child => child.visit(this)); @@ -378,10 +372,12 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { private ingest(template: Template|Node[]): void { if (template instanceof Template) { - // For s, process inputs, outputs, variables, and child nodes. References were - // processed in the scope of the containing template. + // For s, process inputs, outputs, template attributes, + // variables, and child nodes. + // References were processed in the scope of the containing template. template.inputs.forEach(this.visitNode); template.outputs.forEach(this.visitNode); + template.templateAttrs.forEach(this.visitNode); template.variables.forEach(this.visitNode); template.children.forEach(this.visitNode); @@ -394,16 +390,17 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { } visitElement(element: Element) { - // Vist the inputs, outputs, and children of the element. + // Visit the inputs, outputs, and children of the element. element.inputs.forEach(this.visitNode); element.outputs.forEach(this.visitNode); element.children.forEach(this.visitNode); } visitTemplate(template: Template) { - // First, visit the inputs, outputs of the template node. + // First, visit inputs, outputs and template attributes of the template node. template.inputs.forEach(this.visitNode); template.outputs.forEach(this.visitNode); + template.templateAttrs.forEach(this.visitNode); // References are also evaluated in the outer context. template.references.forEach(this.visitNode);