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
This commit is contained in:
Pete Bacon Darwin 2019-03-07 08:31:31 +00:00 committed by Kara Erickson
parent 809452b921
commit 5ad2097be8
3 changed files with 50 additions and 40 deletions

View File

@ -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.');

View File

@ -246,4 +246,4 @@ export function transformAll<Result extends Node>(
changed = changed || newNode != node;
}
return changed ? result : nodes;
}
}

View File

@ -282,27 +282,21 @@ class DirectiveBinder<DirectiveT extends DirectiveMeta> 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 <ng-template>s, process inputs, outputs, variables, and child nodes. References were
// processed in the scope of the containing template.
// For <ng-template>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);