fix(compiler): handle two-way bindings to signal-based template variables in instruction generation (#54714)

Updates the instruction generation for two-way bindings to only emit the `twoWayBindingSet` call when writing to template variables. Since template variables are constants, it's only allowed to write to them when they're signals. Non-signal values are flagged during template type checking.

Fixes #54670.

PR Close #54714

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

@@ -215,6 +215,7 @@ export interface TemplateTypeChecker {
 
   /**
    * Gets the target of a template expression, if possible.
+   * See `BoundTarget.getExpressionTarget` for more information.
    */
   getExpressionTarget(expression: AST, clazz: ts.ClassDeclaration): TmplAstReference|TmplAstVariable
       |null;

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js

@@ -917,3 +917,61 @@ export declare class App {
     static ɵcmp: i0.ɵɵComponentDeclaration<App, "ng-component", never, {}, {}, never, never, true, never>;
 }
 
+/****************************************************************************************************
+ * PARTIAL FILE: two_way_binding_to_signal_loop_variable.js
+ ****************************************************************************************************/
+import { Component, Directive, model, signal } from '@angular/core';
+import * as i0 from "@angular/core";
+export class NgModelDirective {
+    constructor() {
+        this.ngModel = model.required();
+    }
+}
+NgModelDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: NgModelDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
+NgModelDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "17.1.0", version: "0.0.0-PLACEHOLDER", type: NgModelDirective, isStandalone: true, selector: "[ngModel]", inputs: { ngModel: { classPropertyName: "ngModel", publicName: "ngModel", isSignal: true, isRequired: true, transformFunction: null } }, outputs: { ngModel: "ngModelChange" }, ngImport: i0 });
+i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: NgModelDirective, decorators: [{
+            type: Directive,
+            args: [{
+                    selector: '[ngModel]',
+                    standalone: true,
+                }]
+        }] });
+export class TestCmp {
+    constructor() {
+        this.names = [signal('Angular')];
+    }
+}
+TestCmp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestCmp, deps: [], target: i0.ɵɵFactoryTarget.Component });
+TestCmp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: TestCmp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
+    @for (name of names; track $index) {
+      <input [(ngModel)]="name" />
+    }
+  `, isInline: true, dependencies: [{ kind: "directive", type: NgModelDirective, selector: "[ngModel]", inputs: ["ngModel"], outputs: ["ngModelChange"] }] });
+i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestCmp, decorators: [{
+            type: Component,
+            args: [{
+                    template: `
+    @for (name of names; track $index) {
+      <input [(ngModel)]="name" />
+    }
+  `,
+                    standalone: true,
+                    imports: [NgModelDirective],
+                }]
+        }] });
+
+/****************************************************************************************************
+ * PARTIAL FILE: two_way_binding_to_signal_loop_variable.d.ts
+ ****************************************************************************************************/
+import * as i0 from "@angular/core";
+export declare class NgModelDirective {
+    ngModel: import("@angular/core").ModelSignal<string>;
+    static ɵfac: i0.ɵɵFactoryDeclaration<NgModelDirective, never>;
+    static ɵdir: i0.ɵɵDirectiveDeclaration<NgModelDirective, "[ngModel]", never, { "ngModel": { "alias": "ngModel"; "required": true; "isSignal": true; }; }, { "ngModel": "ngModelChange"; }, never, never, true, never>;
+}
+export declare class TestCmp {
+    names: import("@angular/core").WritableSignal<string>[];
+    static ɵfac: i0.ɵɵFactoryDeclaration<TestCmp, never>;
+    static ɵcmp: i0.ɵɵComponentDeclaration<TestCmp, "ng-component", never, {}, {}, never, never, true, never>;
+}
+

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json

@@ -353,6 +353,23 @@
           "failureMessage": "Incorrect template"
         }
       ]
+    },
+    {
+      "description": "should generate a two-way binding to a @for loop variable that is a signal",
+      "inputFiles": [
+        "two_way_binding_to_signal_loop_variable.ts"
+      ],
+      "expectations": [
+        {
+          "files": [
+            {
+              "expected": "two_way_binding_to_signal_loop_variable_template.js",
+              "generated": "two_way_binding_to_signal_loop_variable.js"
+            }
+          ],
+          "failureMessage": "Incorrect template"
+        }
+      ]
     }
   ]
 }

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/two_way_binding_to_signal_loop_variable.ts

@@ -0,0 +1,22 @@
+import {Component, Directive, model, signal} from '@angular/core';
+
+@Directive({
+  selector: '[ngModel]',
+  standalone: true,
+})
+export class NgModelDirective {
+  ngModel = model.required<string>();
+}
+
+@Component({
+  template: `
+    @for (name of names; track $index) {
+      <input [(ngModel)]="name" />
+    }
+  `,
+  standalone: true,
+  imports: [NgModelDirective],
+})
+export class TestCmp {
+  names = [signal('Angular')];
+}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/two_way_binding_to_signal_loop_variable_template.js

@@ -0,0 +1,27 @@
+function TestCmp_For_1_Template(rf, ctx) {
+  if (rf & 1) {
+    const $_r1$ = $r3$.ɵɵgetCurrentView();
+    $r3$.ɵɵelementStart(0, "input", 0);
+    $r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_For_1_Template_input_ngModelChange_0_listener($event) {
+      const $name_r2$ = $r3$.ɵɵrestoreView($_r1$).$implicit;
+      $r3$.ɵɵtwoWayBindingSet($name_r2$, $event);
+      return $r3$.ɵɵresetView($event);
+    });
+    $r3$.ɵɵelementEnd();
+  }
+  if (rf & 2) {
+    const $name_r2$ = ctx.$implicit;
+    $r3$.ɵɵtwoWayProperty("ngModel", $name_r2$);
+  }
+}
+
+…
+
+function TestCmp_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵrepeaterCreate(0, TestCmp_For_1_Template, 1, 1, "input", null, $r3$.ɵɵrepeaterTrackByIndex);
+  }
+  if (rf & 2) {
+    $r3$.ɵɵrepeater(ctx.names);
+  }
+}

packages/compiler/src/template/pipeline/src/phases/transform_two_way_binding_set.ts

@@ -32,12 +32,24 @@ export function transformTwoWayBindingSet(job: CompilationJob): void {
   }
 }
 
-function wrapSetOperation(target: o.ReadPropExpr|o.ReadKeyExpr, value: o.Expression): o.Expression {
+function wrapSetOperation(
+    target: o.ReadPropExpr|o.ReadKeyExpr|ir.ReadVariableExpr, value: o.Expression): o.Expression {
+  // ASSUMPTION: here we're assuming that `ReadVariableExpr` will be a reference
+  // to a local template variable. This appears to be the case at the time of writing.
+  // If the expression is targeting a variable read, we only emit the `twoWayBindingSet` since
+  // the fallback would be attempting to write into a constant. Invalid usages will be flagged
+  // during template type checking.
+  if (target instanceof ir.ReadVariableExpr) {
+    return ng.twoWayBindingSet(target, value);
+  }
+
   return ng.twoWayBindingSet(target, value).or(target.set(value));
 }
 
-function isReadExpression(value: unknown): value is o.ReadPropExpr|o.ReadKeyExpr {
-  return value instanceof o.ReadPropExpr || value instanceof o.ReadKeyExpr;
+function isReadExpression(value: unknown): value is o.ReadPropExpr|o.ReadKeyExpr|
+    ir.ReadVariableExpr {
+  return value instanceof o.ReadPropExpr || value instanceof o.ReadKeyExpr ||
+      value instanceof ir.ReadVariableExpr;
 }
 
 function wrapAction(target: o.Expression, value: o.Expression): o.Expression {

packages/core/test/acceptance/authoring/model_inputs_spec.ts

@@ -589,4 +589,44 @@ describe('model inputs', () => {
     fixture.detectChanges();
     expect(() => fixture.destroy()).not.toThrow();
   });
+
+  it('should support two-way binding to a signal @for loop variable', () => {
+    @Directive({selector: '[dir]', standalone: true})
+    class Dir {
+      value = model(0);
+    }
+
+    @Component({
+      template: `
+        @for (value of values; track $index) {
+          <div [(value)]="value" dir></div>
+        }
+      `,
+      standalone: true,
+      imports: [Dir],
+    })
+    class App {
+      @ViewChild(Dir) dir!: Dir;
+      values = [signal(1)];
+    }
+
+    const fixture = TestBed.createComponent(App);
+    const host = fixture.componentInstance;
+    fixture.detectChanges();
+
+    // Initial value.
+    expect(host.values[0]()).toBe(1);
+    expect(host.dir.value()).toBe(1);
+
+    // Changing the value from within the directive.
+    host.dir.value.set(2);
+    expect(host.values[0]()).toBe(2);
+    expect(host.dir.value()).toBe(2);
+
+    // Changing the value from the outside.
+    host.values[0].set(3);
+    fixture.detectChanges();
+    expect(host.values[0]()).toBe(3);
+    expect(host.dir.value()).toBe(3);
+  });
 });