refactor(compiler-cli): allow writable signals in two-way bindings (#…

…54252)

Updates the TCB generation logic to allow for `WritableSignal` to be assigned in two-way bindings.

PR Close #54252

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

@@ -136,6 +136,8 @@ 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/type_check_block.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
+import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ExpressionType, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType, TypeofExpr, WrappedNodeExpr} from '@angular/compiler';
 import ts from 'typescript';
 
 import {Reference} from '../../imports';
@@ -711,7 +711,7 @@ class TcbDirectiveInputsOp extends TcbOp {
 
       let assignment: ts.Expression = wrapForDiagnostics(expr);
 
-      for (const {fieldName, required, transformType, isSignal} of attr.inputs) {
+      for (const {fieldName, required, transformType, isSignal, isTwoWayBinding} of attr.inputs) {
         let target: ts.LeftHandSideExpression;
 
         if (required) {
@@ -791,12 +791,14 @@ class TcbDirectiveInputsOp extends TcbOp {
                   dirId, ts.factory.createIdentifier(fieldName));
         }
 
-        // 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.
-        //   2. follow the existing pattern where multiple targets assign a single expression.
-        //      This is a significant requirement for language service auto-completion.
-        if (isSignal) {
+        if (isTwoWayBinding) {
+          target = this.getTwoWayBindingExpression(target);
+        } else 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.
+          //   2. follow the existing pattern where multiple targets assign a single expression.
+          //      This is a significant requirement for language service auto-completion.
           const inputSignalBrandWriteSymbol = this.tcb.env.referenceExternalSymbol(
               R3Identifiers.InputSignalBrandWriteType.moduleName,
               R3Identifiers.InputSignalBrandWriteType.name);
@@ -846,6 +848,52 @@ class TcbDirectiveInputsOp extends TcbOp {
           this.tcb.id, this.node, this.dir.name, this.dir.isComponent, missing);
     }
   }
+
+  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`.
+    // 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;
+    // ```
+    // 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>'.
+    const captureType = this.tcb.allocateId();
+
+    // ɵ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 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));
+
+    // (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));
+  }
 }
 
 /**
@@ -2292,7 +2340,8 @@ interface TcbBoundAttribute {
     fieldName: ClassPropertyName,
     required: boolean,
     isSignal: boolean,
-    transformType: Reference<ts.TypeNode>|null
+    transformType: Reference<ts.TypeNode>|null,
+    isTwoWayBinding: boolean,
   }[];
 }
 
@@ -2527,12 +2576,16 @@ function getBoundAttributes(
     if (inputs !== null) {
       boundInputs.push({
         attribute: attr,
-        inputs: inputs.map(input => ({
-                             fieldName: input.classPropertyName,
-                             required: input.required,
-                             transformType: input.transform?.type || null,
-                             isSignal: input.isSignal,
-                           }))
+        inputs: inputs.map(input => {
+          return ({
+            fieldName: input.classPropertyName,
+            required: input.required,
+            transformType: input.transform?.type || null,
+            isSignal: input.isSignal,
+            isTwoWayBinding:
+                attr instanceof TmplAstBoundAttribute && attr.type === BindingType.TwoWay,
+          });
+        })
       });
     }
   };

packages/compiler-cli/src/ngtsc/typecheck/test/output_function_diagnostics.spec.ts

@@ -30,24 +30,6 @@ runInEachFileSystem(() => {
           `TestComponent.html(1, 24): Property 'x' does not exist on type 'void'.`,
         ],
       },
-      {
-        id: 'two way data binding, invalid',
-        inputs: {'value': {type: 'InputSignal<string>', isSignal: true}},
-        outputs: {'valueChange': {type: 'OutputEmitter<string>'}},
-        template: `<div dir [(value)]="bla">`,
-        component: `bla = true;`,
-        expected: [
-          `TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`,
-        ],
-      },
-      {
-        id: 'two way data binding, valid',
-        inputs: {'value': {type: 'InputSignal<string>', isSignal: true}},
-        outputs: {'valueChange': {type: 'OutputEmitter<string>'}},
-        template: `<div dir [(value)]="bla">`,
-        component: `bla: string = ''`,
-        expected: [],
-      },
       {
         id: 'complex output object',
         outputs: {'evt': {type: 'OutputEmitter<{works: boolean}>'}},

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

@@ -648,6 +648,81 @@ describe('type check blocks', () => {
     expect(block).toContain('((((this).foo)).$any(((this).a)))');
   });
 
+  it('should handle a two-way binding to an input/output pair', () => {
+    const TEMPLATE = `<div twoWay [(input)]="value"></div>`;
+    const DIRECTIVES: TestDeclaration[] = [{
+      type: 'directive',
+      name: 'TwoWay',
+      selector: '[twoWay]',
+      inputs: {input: 'input'},
+      outputs: {inputChange: 'inputChange'},
+    }];
+    const block = tcb(TEMPLATE, DIRECTIVES);
+    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));');
+  });
+
+  it('should handle a two-way binding to a model()', () => {
+    const TEMPLATE = `<div twoWay [(input)]="value"></div>`;
+    const DIRECTIVES: TestDeclaration[] = [{
+      type: 'directive',
+      name: 'TwoWay',
+      selector: '[twoWay]',
+      inputs: {
+        input: {
+          classPropertyName: 'input',
+          bindingPropertyName: 'input',
+          required: false,
+          isSignal: true,
+          transform: null,
+        }
+      },
+      outputs: {inputChange: 'inputChange'},
+    }];
+    const block = tcb(TEMPLATE, DIRECTIVES);
+    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));');
+  });
+
+  it('should handle a two-way binding to an input with a transform', () => {
+    const TEMPLATE = `<div twoWay [(input)]="value"></div>`;
+    const DIRECTIVES: TestDeclaration[] = [{
+      type: 'directive',
+      name: 'TwoWay',
+      selector: '[twoWay]',
+      inputs: {
+        input: {
+          classPropertyName: 'input',
+          bindingPropertyName: 'input',
+          required: false,
+          isSignal: false,
+          transform: {
+            node: ts.factory.createFunctionDeclaration(
+                undefined, undefined, undefined, undefined, [], undefined, undefined),
+            type: new Reference(ts.factory.createUnionTypeNode([
+              ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword),
+              ts.factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
+            ]))
+          },
+        }
+      },
+      outputs: {inputChange: 'inputChange'},
+      coercedInputFields: ['input'],
+    }];
+    const block = tcb(TEMPLATE, DIRECTIVES);
+    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));');
+  });
+
   describe('experimental DOM checking via lib.dom.d.ts', () => {
     it('should translate unclaimed bindings to their property equivalent', () => {
       const TEMPLATE = `<label [for]="'test'"></label>`;

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

@@ -122,6 +122,8 @@ 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/template_typecheck_spec.ts

@@ -2355,6 +2355,35 @@ export declare class AnimationEvent {
                 `Type 'HTMLDivElement' is not assignable to type ` +
                 `'HTMLInputElement | ElementRef<HTMLInputElement>'`);
       });
+
+      it('should type check a two-way binding to an input with a transform', () => {
+        env.tsconfig({strictTemplates: true});
+        env.write('test.ts', `
+          import {Component, Directive, Input, Output, EventEmitter} from '@angular/core';
+
+          export function toNumber(val: boolean | string) { return 1; }
+
+          @Directive({selector: '[dir]', standalone: true})
+          export class CoercionDir {
+            @Input({transform: toNumber}) val!: number;
+            @Output() valChange = new EventEmitter<number>();
+          }
+
+          @Component({
+            template: '<input dir [(val)]="invalidType">',
+            standalone: true,
+            imports: [CoercionDir],
+          })
+          export class FooCmp {
+            invalidType = 1;
+          }
+        `);
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText)
+            .toBe(
+                `Type 'number' is not assignable to type 'string | boolean | WritableSignal<string | boolean>'.`);
+      });
     });
 
     describe('restricted inputs', () => {

packages/compiler/src/render3/r3_identifiers.ts

@@ -408,4 +408,6 @@ export class Identifiers {
   // type-checking
   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,6 +20,7 @@ export {
   CreateSignalOptions,
   signal,
   WritableSignal,
+  ɵConditionallyUnwrapSignal,
 } from './render3/reactivity/signal';
 export {
   untracked,

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

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