refactor(compiler-cli): rework TCB for two-way bindings (#54252)

Reworks the TCB for two-way bindings to make them simpler and to avoid regressions for two-way bindings to generic inputs. The new TCB looks as follows:

```
var _t1: Dir;
var _t2 = _t1.input;
(_t1 as typeof _t2 | WritableSignal<typeof _t2>) = expression;
```

PR Close #54252

packages/compiler-cli/src/ngtsc/testing/fake_core/index.ts

@@ -136,8 +136,6 @@ export function signal<T>(initialValue: T): WritableSignal<T> {
   return null!;
 }
 
-export type ɵConditionallyUnwrapSignal<T> = T extends Signal<unknown>? ReturnType<T>: T;
-
 /**
  * -------
  * Signal inputs

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

@@ -358,17 +358,16 @@ export class SymbolBuilder {
     for (const node of nodes) {
       let assignment: ts.PropertyAccessExpression|ts.ElementAccessExpression|null = null;
 
-      // One-way bindings usually are in the form of `dir.input = expression`.
       if (isAccessExpression(node.left)) {
+        // One-way bindings usually are in the form of `dir.input = expression`.
         assignment = node.left;
       } else if (
-          // The property side of two-way bindings is in the
-          // form of `(dir.input as unknown as someType) = expression`.
           isTwoWayBinding && ts.isParenthesizedExpression(node.left) &&
           ts.isAsExpression(node.left.expression) &&
-          ts.isAsExpression(node.left.expression.expression) &&
-          isAccessExpression(node.left.expression.expression.expression)) {
-        assignment = node.left.expression.expression.expression;
+          isAccessExpression(node.left.expression.expression)) {
+        // The property side of two-way bindings is in the
+        // form of `(dir.input as someType) = expression`.
+        assignment = node.left.expression.expression;
       }
 
       if (assignment === null) {

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

@@ -791,9 +791,7 @@ class TcbDirectiveInputsOp extends TcbOp {
                   dirId, ts.factory.createIdentifier(fieldName));
         }
 
-        if (isTwoWayBinding) {
-          target = this.getTwoWayBindingExpression(target);
-        } else if (isSignal) {
+        if (isSignal) {
           // For signal inputs, we unwrap the target `InputSignal`. Note that
           // we intentionally do the following things:
           //   1. keep the direct access to `dir.[field]` so that modifiers are honored.
@@ -811,6 +809,10 @@ class TcbDirectiveInputsOp extends TcbOp {
           target = ts.factory.createElementAccessExpression(target, inputSignalBrandWriteSymbol);
         }
 
+        if (isTwoWayBinding) {
+          target = this.getTwoWayBindingTarget(target);
+        }
+
         if (attr.attribute.keySpan !== undefined) {
           addParseSpanInfo(target, attr.attribute.keySpan);
         }
@@ -849,50 +851,33 @@ class TcbDirectiveInputsOp extends TcbOp {
     }
   }
 
-  private getTwoWayBindingExpression(target: ts.LeftHandSideExpression): ts.LeftHandSideExpression {
-    // TODO(crisbeto): we should be able to avoid the extra variable that captures the type.
-    // Skipping it for since we don't have a good way to convert the `PropertyAccessExpression`
-    // into an `QualifiedName`.
+  private getTwoWayBindingTarget(target: ts.LeftHandSideExpression): ts.LeftHandSideExpression {
     // Two-way bindings to inputs allow both the input's defined type and a `WritableSignal`
     // of that type. For example `[(value)]="val"` where `@Input() value: number | string`
     // allows `val` to be `number | string | WritableSignal<number | string>`. We generate the
     // following expressions to expand the type:
     // ```
     // var captureType = dir.value;
-    // (id as unknown as ɵConditionallyUnwrapSignal<typeof captureType> |
-    // WritableSignal<ɵConditionallyUnwrapSignal<typeof captureType>>) = expression;
+    // (dir.value as typeof captureType | WritableSignal<typeof captureType>) = expression;
     // ```
-    // Note that the TCB can be simplified a bit by making the union type part of the utility type
-    // (e.g. `type ɵTwoWayAssign<T> = T extends Signal ? ReturnType<T> |
-    // WritableSignal<ReturnType<T>> : ReturnType<T> | WritableSignal<ReturnType<T>>`), however at
-    // the time of writing, this generates a suboptimal diagnostic message where TS splits up the
-    // signature, e.g. "Type 'number' is not assignable to type 'string | boolean |
-    // WritableSignal<string> | WritableSignal<false> | WritableSignal<true>'" instead of Type
-    // 'number' is not assignable to type 'string | boolean | WritableSignal<string | boolean>'.
+    // Some notes:
+    // - We wrap the assignment, insted of for example declaring a variable and assigning to it,
+    //   because keeping the assignment makes it easier to do lookups in the language service.
+    // - The `captureType` variable can be inline, but for signal input expressions it can be
+    //   long so we use it make the code a bit neater. It also saves us some code that would
+    //   have to convert property/element access expressions into type query nodes.
     const captureType = this.tcb.allocateId();
+    const captureTypeVar = tsCreateVariable(captureType, target);
+    markIgnoreDiagnostics(captureTypeVar);
+    this.scope.addStatement(captureTypeVar);
 
-    // ɵConditionallyUnwrapSignal<typeof captureType>
-    const unwrappedRef = this.tcb.env.referenceExternalType(
-        R3Identifiers.ConditionallyUnwrapSignal.moduleName,
-        R3Identifiers.ConditionallyUnwrapSignal.name,
-        [new ExpressionType(new TypeofExpr(new WrappedNodeExpr(captureType)))]);
-
-    // WritableSignal<ɵConditionallyUnwrapSignal<typeof captureType>>
+    const typeQuery = ts.factory.createTypeQueryNode(captureType);
     const writableSignalRef = this.tcb.env.referenceExternalType(
         R3Identifiers.WritableSignal.moduleName, R3Identifiers.WritableSignal.name,
-        [new ExpressionType(new WrappedNodeExpr(unwrappedRef))]);
-
-    // ɵConditionallyUnwrapSignal<typeof captureType> |
-    // WritableSignal<ɵConditionallyUnwrapSignal<typeof captureType>>
-    const type = ts.factory.createUnionTypeNode([unwrappedRef, writableSignalRef]);
-    this.scope.addStatement(tsCreateVariable(captureType, target));
+        [new ExpressionType(new TypeofExpr(new WrappedNodeExpr(captureType)))]);
 
-    // (target as unknown as ɵConditionallyUnwrapSignal<typeof captureType> |
-    // WritableSignal<ɵConditionallyUnwrapSignal<typeof captureType>>)
     return ts.factory.createParenthesizedExpression(ts.factory.createAsExpression(
-        ts.factory.createAsExpression(
-            target, ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword)),
-        type));
+        target, ts.factory.createUnionTypeNode([typeQuery, writableSignalRef])));
   }
 }
 

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

@@ -661,8 +661,7 @@ describe('type check blocks', () => {
     expect(block).toContain('var _t1 = null! as i0.TwoWay;');
     expect(block).toContain('var _t2 = _t1.input;');
     expect(block).toContain(
-        '(_t1.input as unknown as i1.ɵConditionallyUnwrapSignal<typeof _t2> | ' +
-        'i1.WritableSignal<i1.ɵConditionallyUnwrapSignal<typeof _t2>>) = (((this).value));');
+        '(_t1.input as typeof _t2 | i1.WritableSignal<typeof _t2>) = (((this).value));');
   });
 
   it('should handle a two-way binding to a model()', () => {
@@ -684,10 +683,9 @@ describe('type check blocks', () => {
     }];
     const block = tcb(TEMPLATE, DIRECTIVES);
     expect(block).toContain('var _t1 = null! as i0.TwoWay;');
-    expect(block).toContain('var _t2 = _t1.input;');
+    expect(block).toContain('var _t2 = _t1.input[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE];');
     expect(block).toContain(
-        '(_t1.input as unknown as i1.ɵConditionallyUnwrapSignal<typeof _t2> | ' +
-        'i1.WritableSignal<i1.ɵConditionallyUnwrapSignal<typeof _t2>>) = (((this).value));');
+        '(_t1.input[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE] as typeof _t2 | i1.WritableSignal<typeof _t2>) = (((this).value));');
   });
 
   it('should handle a two-way binding to an input with a transform', () => {
@@ -719,8 +717,7 @@ describe('type check blocks', () => {
     expect(block).toContain('var _t1 = null! as boolean | string;');
     expect(block).toContain('var _t2 = _t1;');
     expect(block).toContain(
-        '(_t1 as unknown as i1.ɵConditionallyUnwrapSignal<typeof _t2> | ' +
-        'i1.WritableSignal<i1.ɵConditionallyUnwrapSignal<typeof _t2>>) = (((this).value));');
+        '(_t1 as typeof _t2 | i1.WritableSignal<typeof _t2>) = (((this).value));');
   });
 
   describe('experimental DOM checking via lib.dom.d.ts', () => {

packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts

@@ -122,8 +122,6 @@ export function angularCoreDts(): TestFile {
       asReadonly(): Signal<T>;
     }
 
-    export type ɵConditionallyUnwrapSignal<T> = T extends Signal<unknown>? ReturnType<T>: T;
-
     /**
      * -------
      * Signal inputs

packages/compiler-cli/test/ngtsc/authoring_spec.ts

@@ -586,6 +586,37 @@ runInEachFileSystem(() => {
         }));
       });
 
+      it('should check two-way binding of a signal to a decorator-based input/output pair', () => {
+        env.write('test.ts', `
+          import {Component, Directive, Input, Output, signal, EventEmitter} from '@angular/core';
+
+          @Directive({
+            selector: '[dir]',
+            standalone: true,
+          })
+          export class TestDir {
+            @Input() value = 0;
+            @Output() valueChange = new EventEmitter<number>();
+          }
+
+          @Component({
+            standalone: true,
+            template: \`<div dir [(value)]="value"></div>\`,
+            imports: [TestDir],
+          })
+          export class TestComp {
+            value = signal(false);
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toEqual(jasmine.objectContaining({
+          messageText:
+              `Type 'WritableSignal<boolean>' is not assignable to type 'number | WritableSignal<number>'.`
+        }));
+      });
+
       it('should not allow a non-writable signal to be assigned to a model', () => {
         env.write('test.ts', `
           import {Component, Directive, model, input} from '@angular/core';
@@ -696,6 +727,120 @@ runInEachFileSystem(() => {
         expect(diagnostics[0].messageText)
             .toBe(`Required input 'value' from directive TestDir must be specified.`);
       });
+
+      it('should check generic two-way model binding with a primitive value', () => {
+        env.write('test.ts', `
+          import {Component, Directive, model} from '@angular/core';
+
+          @Directive({
+            selector: '[dir]',
+            standalone: true,
+          })
+          export class TestDir<T extends {id: string}> {
+            value = model.required<T>();
+          }
+
+          @Component({
+            standalone: true,
+            template: \`<div dir [(value)]="value"></div>\`,
+            imports: [TestDir],
+          })
+          export class TestComp {
+            value = {id: 1};
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toEqual(jasmine.objectContaining({
+          messageText:
+              `Type '{ id: number; }' is not assignable to type '{ id: string; } | WritableSignal<{ id: string; }>'.`
+        }));
+      });
+
+      it('should check generic two-way model binding with a signal value', () => {
+        env.write('test.ts', `
+          import {Component, Directive, model, signal} from '@angular/core';
+
+          @Directive({
+            selector: '[dir]',
+            standalone: true,
+          })
+          export class TestDir<T extends {id: string}> {
+            value = model.required<T>();
+          }
+
+          @Component({
+            standalone: true,
+            template: \`<div dir [(value)]="value"></div>\`,
+            imports: [TestDir],
+          })
+          export class TestComp {
+            value = signal({id: 1});
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toEqual(jasmine.objectContaining({
+          messageText:
+              `Type 'WritableSignal<{ id: number; }>' is not assignable to type '{ id: string; } | WritableSignal<{ id: string; }>'.`
+        }));
+      });
+
+      it('should report unwrapped signals assigned to a model in a one-way binding', () => {
+        env.write('test.ts', `
+          import {Component, Directive, model, signal} from '@angular/core';
+
+          @Directive({
+            selector: '[dir]',
+            standalone: true,
+          })
+          export class TestDir {
+            value = model(0);
+          }
+
+          @Component({
+            standalone: true,
+            template: \`<div dir [value]="value"></div>\`,
+            imports: [TestDir],
+          })
+          export class TestComp {
+            value = signal(1);
+          }
+        `);
+
+        const diagnostics = env.driveDiagnostics();
+        expect(diagnostics.length).toBe(1);
+        expect(diagnostics[0].messageText)
+            .toBe(`Type 'WritableSignal<number>' is not assignable to type 'number'.`);
+      });
+    });
+
+    it('should allow two-way binding to a generic model input', () => {
+      env.write('test.ts', `
+        import {Component, Directive, model, signal} from '@angular/core';
+
+        @Directive({
+          selector: '[dir]',
+          standalone: true,
+        })
+        export class TestDir<T> {
+          value = model.required<T>();
+        }
+
+        @Component({
+          standalone: true,
+          template: \`<div dir [(value)]="value"></div>\`,
+          imports: [TestDir],
+        })
+        export class TestComp {
+          value = signal(1);
+        }
+      `);
+
+      const diags = env.driveDiagnostics();
+      expect(diags).toEqual([]);
     });
   });
 });

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

@@ -418,6 +418,35 @@ export declare class AnimationEvent {
       expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![1])).toBe('ChildCmpDir');
     });
 
+    it('should type check a two-way binding to a generic property', () => {
+      env.tsconfig({strictTemplates: true});
+      env.write('test.ts', `
+        import {Component, Directive, Input, Output, EventEmitter} from '@angular/core';
+
+        @Directive({selector: '[dir]', standalone: true})
+        export class Dir<T extends {id: string}> {
+          @Input() val!: T;
+          @Output() valChange = new EventEmitter<T>();
+        }
+
+        @Component({
+          template: '<input dir [(val)]="invalidType">',
+          standalone: true,
+          imports: [Dir],
+        })
+        export class FooCmp {
+          invalidType = {id: 1};
+        }
+      `);
+
+      const diags = env.driveDiagnostics();
+      expect(diags.length).toBe(1);
+      expect(diags[0].messageText).toEqual(jasmine.objectContaining({
+        messageText:
+            `Type '{ id: number; }' is not assignable to type '{ id: string; } | WritableSignal<{ id: string; }>'.`
+      }));
+    });
+
     describe('strictInputTypes', () => {
       beforeEach(() => {
         env.write('test.ts', `

packages/compiler/src/render3/r3_identifiers.ts

@@ -409,5 +409,4 @@ export class Identifiers {
   static InputSignalBrandWriteType = {name: 'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE', moduleName: CORE};
   static UnwrapDirectiveSignalInputs = {name: 'ɵUnwrapDirectiveSignalInputs', moduleName: CORE};
   static WritableSignal = {name: 'WritableSignal', moduleName: CORE};
-  static ConditionallyUnwrapSignal = {name: 'ɵConditionallyUnwrapSignal', moduleName: CORE};
 }

packages/core/src/core_reactivity_export_internal.ts

@@ -20,7 +20,6 @@ export {
   CreateSignalOptions,
   signal,
   WritableSignal,
-  ɵConditionallyUnwrapSignal,
 } from './render3/reactivity/signal';
 export {
   untracked,

packages/core/src/render3/reactivity/signal.ts

@@ -43,15 +43,6 @@ export interface CreateSignalOptions<T> {
   equal?: ValueEqualityFn<T>;
 }
 
-/**
- * Type used during template type checking to either unwrap a signal or preserve a value if it's
- * not a signal. E.g. `ɵConditionallyUnwrapSignal<Signal<number>>` yields `number` while
- * `ɵConditionallyUnwrapSignal<number>` preserves the `number` type.
- *
- * @codeGenApi
- */
-export type ɵConditionallyUnwrapSignal<T> = T extends Signal<unknown>? ReturnType<T>: T;
-
 /**
  * Create a `Signal` that can be set or updated directly.
  */

packages/core/test/render3/jit_environment_spec.ts

@@ -34,7 +34,6 @@ const AOT_ONLY = new Set<string>([
   // used in type-checking.
   'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE',
   'ɵUnwrapDirectiveSignalInputs',
-  'ɵConditionallyUnwrapSignal',
   'WritableSignal',
 ]);