fix(ivy): type-check multiple bindings to the same input (#34649)

Currently, the template type-checker gives an error if there are multiple
bindings to the same input. This commit aligns the behavior of the template
type-checker with the View Engine runtime: only the first binding to a field
has any effect. The rest are ignored.

PR Close #34649
This commit is contained in:
Alex Rickabaugh 2019-12-19 14:35:52 -08:00 committed by Andrew Kushnir
parent 22c957a93d
commit cfe5dccdd2
2 changed files with 30 additions and 14 deletions

View File

@ -1174,7 +1174,10 @@ type TcbDirectiveInput = {
function tcbGetDirectiveInputs(
el: TmplAstElement | TmplAstTemplate, dir: TypeCheckableDirectiveMeta, tcb: Context,
scope: Scope): TcbDirectiveInput[] {
const directiveInputs: TcbDirectiveInput[] = [];
// Only the first binding to a property is written.
// TODO(alxhub): produce an error for duplicate bindings to the same property, independently of
// this logic.
const directiveInputs = new Map<string, TcbDirectiveInput>();
// `dir.inputs` is an object map of field names on the directive class to property names.
// This is backwards from what's needed to match bindings - a map of properties to field names
// is desired. Invert `dir.inputs` into `propMatch` to create this map.
@ -1185,13 +1188,6 @@ function tcbGetDirectiveInputs(
propMatch.set(inputs[key] as string, key);
});
// To determine which of directive's inputs are unset, we keep track of the set of field names
// that have not been seen yet. A field is removed from this set once a binding to it is found.
// Note: it's actually important here that `propMatch.values()` isn't used, as there can be
// multiple fields which share the same property name and only one of them will be listed as a
// value in `propMatch`.
const unsetFields = new Set(Object.keys(inputs));
el.inputs.forEach(processAttribute);
el.attributes.forEach(processAttribute);
if (el instanceof TmplAstTemplate) {
@ -1199,11 +1195,16 @@ function tcbGetDirectiveInputs(
}
// Add unset directive inputs for each of the remaining unset fields.
for (const field of unsetFields) {
directiveInputs.push({type: 'unset', field});
// Note: it's actually important here that `propMatch.values()` isn't used, as there can be
// multiple fields which share the same property name and only one of them will be listed as a
// value in `propMatch`.
for (const field of Object.keys(inputs)) {
if (!directiveInputs.has(field)) {
directiveInputs.set(field, {type: 'unset', field});
}
}
return directiveInputs;
return Array.from(directiveInputs.values());
/**
* Add a binding expression to the map for each input/template attribute of the directive that has
@ -1226,8 +1227,10 @@ function tcbGetDirectiveInputs(
}
const field = propMatch.get(attr.name) !;
// Remove the field from the set of unseen fields, now that it's been assigned to.
unsetFields.delete(field);
// Skip the attribute if a previous binding also wrote to it.
if (directiveInputs.has(field)) {
return;
}
let expr: ts.Expression;
if (attr instanceof TmplAstBoundAttribute) {
@ -1238,7 +1241,7 @@ function tcbGetDirectiveInputs(
expr = ts.createStringLiteral(attr.value);
}
directiveInputs.push({
directiveInputs.set(field, {
type: 'binding',
field: field,
expression: expr,

View File

@ -51,6 +51,19 @@ describe('type check blocks', () => {
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": ("value")');
});
it('should handle multiple bindings to the same property', () => {
const TEMPLATE = `<div dir-a [inputA]="1" [inputA]="2"></div>`;
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'DirA',
selector: '[dir-a]',
inputs: {inputA: 'inputA'},
}];
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('"inputA": (1)');
expect(block).not.toContain('"inputA": (2)');
});
it('should handle empty bindings', () => {
const TEMPLATE = `<div dir-a [inputA]=""></div>`;
const DIRECTIVES: TestDeclaration[] = [{