refactor(compiler): distinguish two-way bindings in the AST (#54065)

During the template parsing stage two-way bindings are split up into a property and event binding. All the downstream code treats these binding the same as their one-way equivalents. For some future work we'll have to distinguish between the two so these changes update the `BoundElementProperty.type` and `ParsedEvent.type` to include a `TwoWay` type. All existing call-sites have been updated to treat `TwoWay` the same as `Property`/`Regular`, but more specialized logic will be added in the future.

PR Close #54065

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

@@ -913,19 +913,20 @@ class TcbDomSchemaCheckerOp extends TcbOp {
 
     // TODO(alxhub): this could be more efficient.
     for (const binding of this.element.inputs) {
-      if (binding.type === BindingType.Property && this.claimedInputs.has(binding.name)) {
+      const isPropertyBinding =
+          binding.type === BindingType.Property || binding.type === BindingType.TwoWay;
+
+      if (isPropertyBinding && this.claimedInputs.has(binding.name)) {
         // Skip this binding as it was claimed by a directive.
         continue;
       }
 
-      if (binding.type === BindingType.Property) {
-        if (binding.name !== 'style' && binding.name !== 'class') {
-          // A direct binding to a property.
-          const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
-          this.tcb.domSchemaChecker.checkProperty(
-              this.tcb.id, this.element, propertyName, binding.sourceSpan, this.tcb.schemas,
-              this.tcb.hostIsStandalone);
-        }
+      if (isPropertyBinding && binding.name !== 'style' && binding.name !== 'class') {
+        // A direct binding to a property.
+        const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
+        this.tcb.domSchemaChecker.checkProperty(
+            this.tcb.id, this.element, propertyName, binding.sourceSpan, this.tcb.schemas,
+            this.tcb.hostIsStandalone);
       }
     }
     return null;
@@ -1107,14 +1108,17 @@ class TcbUnclaimedInputsOp extends TcbOp {
 
     // TODO(alxhub): this could be more efficient.
     for (const binding of this.element.inputs) {
-      if (binding.type === BindingType.Property && this.claimedInputs.has(binding.name)) {
+      const isPropertyBinding =
+          binding.type === BindingType.Property || binding.type === BindingType.TwoWay;
+
+      if (isPropertyBinding && this.claimedInputs.has(binding.name)) {
         // Skip this binding as it was claimed by a directive.
         continue;
       }
 
       const expr = widenBinding(tcbExpression(binding.value, this.tcb, this.scope), this.tcb);
 
-      if (this.tcb.env.config.checkTypeOfDomBindings && binding.type === BindingType.Property) {
+      if (this.tcb.env.config.checkTypeOfDomBindings && isPropertyBinding) {
         if (binding.name !== 'style' && binding.name !== 'class') {
           if (elId === null) {
             elId = this.scope.resolve(this.element);
@@ -1164,7 +1168,8 @@ export class TcbDirectiveOutputsOp extends TcbOp {
     const outputs = this.dir.outputs;
 
     for (const output of this.node.outputs) {
-      if (output.type !== ParsedEventType.Regular || !outputs.hasBindingPropertyName(output.name)) {
+      if (output.type === ParsedEventType.Animation ||
+          !outputs.hasBindingPropertyName(output.name)) {
         continue;
       }
 
@@ -2511,7 +2516,8 @@ function getBoundAttributes(
 
   const processAttribute = (attr: TmplAstBoundAttribute|TmplAstTextAttribute) => {
     // Skip non-property bindings.
-    if (attr instanceof TmplAstBoundAttribute && attr.type !== BindingType.Property) {
+    if (attr instanceof TmplAstBoundAttribute && attr.type !== BindingType.Property &&
+        attr.type !== BindingType.TwoWay) {
       return;
     }
 

packages/compiler/src/expression_parser/ast.ts

@@ -842,14 +842,17 @@ export class ParsedProperty {
 export enum ParsedPropertyType {
   DEFAULT,
   LITERAL_ATTR,
-  ANIMATION
+  ANIMATION,
+  TWO_WAY,
 }
 
 export const enum ParsedEventType {
   // DOM or Directive event
   Regular,
   // Animation specific event
   Animation,
+  // Event side of a two-way binding (e.g. `[(property)]="expression"`).
+  TwoWay,
 }
 
 export class ParsedEvent {
@@ -882,6 +885,8 @@ export const enum BindingType {
   Style,
   // A binding to an animation reference (e.g. `[animate.key]="expression"`).
   Animation,
+  // Property side of a two-way binding (e.g. `[(property)]="expression"`).
+  TwoWay,
 }
 
 export class BoundElementProperty {

packages/compiler/src/render3/r3_template_transform.ts

@@ -474,7 +474,7 @@ class HtmlAstToIvyAst implements html.Visitor {
         const identifier = bindParts[IDENT_KW_IDX];
         const keySpan = createKeySpan(srcSpan, bindParts[KW_BIND_IDX], identifier);
         this.bindingParser.parsePropertyBinding(
-            identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
+            identifier, value, false, false, srcSpan, absoluteOffset, attribute.valueSpan,
             matchableAttributes, parsedProperties, keySpan);
 
       } else if (bindParts[KW_LET_IDX]) {
@@ -502,7 +502,7 @@ class HtmlAstToIvyAst implements html.Visitor {
         const identifier = bindParts[IDENT_KW_IDX];
         const keySpan = createKeySpan(srcSpan, bindParts[KW_BINDON_IDX], identifier);
         this.bindingParser.parsePropertyBinding(
-            identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
+            identifier, value, false, true, srcSpan, absoluteOffset, attribute.valueSpan,
             matchableAttributes, parsedProperties, keySpan);
         this.parseAssignmentEvent(
             identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents,
@@ -536,14 +536,14 @@ class HtmlAstToIvyAst implements html.Visitor {
       const keySpan = createKeySpan(srcSpan, delims.start, identifier);
       if (delims.start === BINDING_DELIMS.BANANA_BOX.start) {
         this.bindingParser.parsePropertyBinding(
-            identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
+            identifier, value, false, true, srcSpan, absoluteOffset, attribute.valueSpan,
             matchableAttributes, parsedProperties, keySpan);
         this.parseAssignmentEvent(
             identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents,
             keySpan);
       } else if (delims.start === BINDING_DELIMS.PROPERTY.start) {
         this.bindingParser.parsePropertyBinding(
-            identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
+            identifier, value, false, false, srcSpan, absoluteOffset, attribute.valueSpan,
             matchableAttributes, parsedProperties, keySpan);
       } else {
         const events: ParsedEvent[] = [];

packages/compiler/src/render3/view/template.ts

@@ -713,7 +713,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     element.inputs.forEach(input => {
       const stylingInputWasSet = stylingBuilder.registerBoundInput(input);
       if (!stylingInputWasSet) {
-        if (input.type === BindingType.Property && input.i18n) {
+        if ((input.type === BindingType.Property || input.type === BindingType.TwoWay) &&
+            input.i18n) {
           boundI18nAttrs.push(input);
         } else {
           allOtherInputs.push(input);
@@ -865,7 +866,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
           }
           this.allocateBindingSlots(value);
 
-          if (inputType === BindingType.Property) {
+          if (inputType === BindingType.Property || inputType === BindingType.TwoWay) {
             if (value instanceof Interpolation) {
               // prop="{{value}}" and friends
               this.interpolatedUpdateInstruction(

packages/compiler/src/render3/view/util.ts

@@ -341,7 +341,7 @@ function getAttrsForDirectiveMatching(elOrTpl: t.Element|t.Template): {[name: st
     });
 
     elOrTpl.inputs.forEach(i => {
-      if (i.type === BindingType.Property) {
+      if (i.type === BindingType.Property || i.type === BindingType.TwoWay) {
         attributesMap[i.name] = '';
       }
     });

packages/compiler/src/template/pipeline/src/ingest.ts

@@ -125,8 +125,8 @@ export function ingestHostAttribute(
 }
 
 export function ingestHostEvent(job: HostBindingCompilationJob, event: e.ParsedEvent) {
-  const [phase, target] = event.type === e.ParsedEventType.Regular ? [null, event.targetOrPhase] :
-                                                                     [event.targetOrPhase, null];
+  const [phase, target] = event.type !== e.ParsedEventType.Animation ? [null, event.targetOrPhase] :
+                                                                       [event.targetOrPhase, null];
   const eventBinding = ir.createListenerOp(
       job.root.xref, new ir.SlotHandle(), event.name, null,
       makeListenerHandlerOps(job.root, event.handler, event.handlerSpan), phase, target, true,
@@ -846,6 +846,8 @@ function convertAstWithInterpolation(
 // TODO: Can we populate Template binding kinds in ingest?
 const BINDING_KINDS = new Map<e.BindingType, ir.BindingKind>([
   [e.BindingType.Property, ir.BindingKind.Property],
+  // TODO(crisbeto): we'll need a different BindingKind for two-way bindings.
+  [e.BindingType.TwoWay, ir.BindingKind.Property],
   [e.BindingType.Attribute, ir.BindingKind.Attribute],
   [e.BindingType.Class, ir.BindingKind.ClassName],
   [e.BindingType.Style, ir.BindingKind.StyleProperty],
@@ -1042,8 +1044,8 @@ function createTemplateBinding(
   // update instruction.
   if (templateKind === ir.TemplateKind.Structural) {
     if (!isStructuralTemplateAttribute &&
-        (type === e.BindingType.Property || type === e.BindingType.Class ||
-         type === e.BindingType.Style)) {
+        (type === e.BindingType.Property || type === e.BindingType.TwoWay ||
+         type === e.BindingType.Class || type === e.BindingType.Style)) {
       // Because this binding doesn't really target the ng-template, it must be a binding on an
       // inner node of a structural template. We can't skip it entirely, because we still need it on
       // the ng-template's consts (e.g. for the purposes of directive matching). However, we should

packages/compiler/src/template_parser/binding_parser.ts

@@ -51,7 +51,7 @@ export class BindingParser {
       const expression = properties[propName];
       if (typeof expression === 'string') {
         this.parsePropertyBinding(
-            propName, expression, true, sourceSpan, sourceSpan.start.offset, undefined, [],
+            propName, expression, true, false, sourceSpan, sourceSpan.start.offset, undefined, [],
             // Use the `sourceSpan` for  `keySpan`. This isn't really accurate, but neither is the
             // sourceSpan, as it represents the sourceSpan of the host itself rather than the
             // source of the host binding (which doesn't exist in the template). Regardless,
@@ -169,7 +169,8 @@ export class BindingParser {
         const srcSpan = isIvyAst ? bindingSpan : sourceSpan;
         const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan);
         this._parsePropertyAst(
-            key, binding.value, srcSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
+            key, binding.value, false, srcSpan, keySpan, valueSpan, targetMatchableAttrs,
+            targetProps);
       } else {
         targetMatchableAttrs.push([key, '' /* value */]);
         // Since this is a literal attribute with no RHS, source span should be
@@ -239,8 +240,8 @@ export class BindingParser {
   }
 
   parsePropertyBinding(
-      name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan,
-      absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
+      name: string, expression: string, isHost: boolean, isPartOfAssignmentBinding: boolean,
+      sourceSpan: ParseSourceSpan, absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
       targetMatchableAttrs: string[][], targetProps: ParsedProperty[], keySpan: ParseSourceSpan) {
     if (name.length === 0) {
       this._reportError(`Property name is missing in binding`, sourceSpan);
@@ -272,7 +273,8 @@ export class BindingParser {
     } else {
       this._parsePropertyAst(
           name, this.parseBinding(expression, isHost, valueSpan || sourceSpan, absoluteOffset),
-          sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
+          isPartOfAssignmentBinding, sourceSpan, keySpan, valueSpan, targetMatchableAttrs,
+          targetProps);
     }
   }
 
@@ -284,19 +286,21 @@ export class BindingParser {
     const expr = this.parseInterpolation(value, valueSpan || sourceSpan, interpolatedTokens);
     if (expr) {
       this._parsePropertyAst(
-          name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
+          name, expr, false, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
       return true;
     }
     return false;
   }
 
   private _parsePropertyAst(
-      name: string, ast: ASTWithSource, sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan,
-      valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
-      targetProps: ParsedProperty[]) {
+      name: string, ast: ASTWithSource, isPartOfAssignmentBinding: boolean,
+      sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan, valueSpan: ParseSourceSpan|undefined,
+      targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
     targetMatchableAttrs.push([name, ast.source!]);
-    targetProps.push(
-        new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, keySpan, valueSpan));
+    targetProps.push(new ParsedProperty(
+        name, ast,
+        isPartOfAssignmentBinding ? ParsedPropertyType.TWO_WAY : ParsedPropertyType.DEFAULT,
+        sourceSpan, keySpan, valueSpan));
   }
 
   private _parseAnimation(
@@ -387,7 +391,8 @@ export class BindingParser {
       boundPropertyName = mapPropertyName ? mappedPropName : boundProp.name;
       securityContexts = calcPossibleSecurityContexts(
           this._schemaRegistry, elementSelector, mappedPropName, false);
-      bindingType = BindingType.Property;
+      bindingType =
+          boundProp.type === ParsedPropertyType.TWO_WAY ? BindingType.TwoWay : BindingType.Property;
       if (!skipValidation) {
         this._validatePropertyOrAttributeName(mappedPropName, boundProp.sourceSpan, false);
       }
@@ -465,7 +470,8 @@ export class BindingParser {
     const ast = this._parseAction(expression, isAssignmentEvent, handlerSpan);
     targetMatchableAttrs.push([name!, ast.source!]);
     targetEvents.push(new ParsedEvent(
-        eventName, target, ParsedEventType.Regular, ast, sourceSpan, handlerSpan, keySpan));
+        eventName, target, isAssignmentEvent ? ParsedEventType.TwoWay : ParsedEventType.Regular,
+        ast, sourceSpan, handlerSpan, keySpan));
     // Don't detect directives for event names for now,
     // so don't add the event name to the matchableAttrs
   }

packages/compiler/test/render3/r3_template_transform_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {BindingType} from '../../src/expression_parser/ast';
+import {BindingType, ParsedEventType} from '../../src/expression_parser/ast';
 import * as t from '../../src/render3/r3_ast';
 import {unparse} from '../expression_parser/utils/unparser';
 
@@ -70,6 +70,7 @@ class R3AstHumanizer implements t.Visitor<void> {
   visitBoundEvent(event: t.BoundEvent) {
     this.result.push([
       'BoundEvent',
+      event.type,
       event.name,
       event.target,
       unparse(event.handler),
@@ -465,25 +466,25 @@ describe('R3 template transform', () => {
     it('should parse bound events with a target', () => {
       expectFromHtml('<div (window:event)="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundEvent', 'event', 'window', 'v'],
+        ['BoundEvent', ParsedEventType.Regular, 'event', 'window', 'v'],
       ]);
     });
 
     it('should parse event names case sensitive', () => {
       expectFromHtml('<div (some-event)="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundEvent', 'some-event', null, 'v'],
+        ['BoundEvent', ParsedEventType.Regular, 'some-event', null, 'v'],
       ]);
       expectFromHtml('<div (someEvent)="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundEvent', 'someEvent', null, 'v'],
+        ['BoundEvent', ParsedEventType.Regular, 'someEvent', null, 'v'],
       ]);
     });
 
     it('should parse bound events via on-', () => {
       expectFromHtml('<div on-event="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundEvent', 'event', null, 'v'],
+        ['BoundEvent', ParsedEventType.Regular, 'event', null, 'v'],
       ]);
     });
 
@@ -494,24 +495,24 @@ describe('R3 template transform', () => {
     it('should parse bound events and properties via [(...)]', () => {
       expectFromHtml('<div [(prop)]="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundAttribute', BindingType.Property, 'prop', 'v'],
-        ['BoundEvent', 'propChange', null, 'v = $event'],
+        ['BoundAttribute', BindingType.TwoWay, 'prop', 'v'],
+        ['BoundEvent', ParsedEventType.TwoWay, 'propChange', null, 'v = $event'],
       ]);
     });
 
     it('should parse bound events and properties via bindon-', () => {
       expectFromHtml('<div bindon-prop="v"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundAttribute', BindingType.Property, 'prop', 'v'],
-        ['BoundEvent', 'propChange', null, 'v = $event'],
+        ['BoundAttribute', BindingType.TwoWay, 'prop', 'v'],
+        ['BoundEvent', ParsedEventType.TwoWay, 'propChange', null, 'v = $event'],
       ]);
     });
 
     it('should parse bound events and properties via [(...)] with non-null operator', () => {
       expectFromHtml('<div [(prop)]="v!"></div>').toEqual([
         ['Element', 'div'],
-        ['BoundAttribute', BindingType.Property, 'prop', 'v!'],
-        ['BoundEvent', 'propChange', null, 'v = $event'],
+        ['BoundAttribute', BindingType.TwoWay, 'prop', 'v!'],
+        ['BoundEvent', ParsedEventType.TwoWay, 'propChange', null, 'v = $event'],
       ]);
     });
 
@@ -536,7 +537,7 @@ describe('R3 template transform', () => {
     it('should parse bound animation events when event name is empty', () => {
       expectFromHtml('<div (@)="onAnimationEvent($event)"></div>', true).toEqual([
         ['Element', 'div'],
-        ['BoundEvent', '', null, 'onAnimationEvent($event)'],
+        ['BoundEvent', ParsedEventType.Animation, '', null, 'onAnimationEvent($event)'],
       ]);
       expect(() => parse('<div (@)></div>'))
           .toThrowError(/Animation event name is missing in binding/);