refactor(compiler-cli): do not emit signal unwrap calls in versions o…

…lder than 17.2 (#54423)

In order to allow both signals and non-signals in two-way bindings, we have to pass the expression through `ɵunwrapWritableSignal`. The problem is that the language service uses a bundled compiler that is fairly new, but it may be compiling an older version of Angular that doesn't expose `ɵunwrapWritableSignal` (see angular/vscode-ng-language-service#2001).

These changes add a `_angularCoreVersion` flag to the compiler which the language service can use to pass the parsed Angular version to the compiler which can then decide whether to emit the function.

PR Close #54423

packages/compiler-cli/src/ngtsc/core/api/src/options.ts

@@ -74,6 +74,14 @@ export interface InternalOptions {
    * @internal
    */
   _enableBlockSyntax?: boolean;
+
+  /**
+   * Detected version of `@angular/core` in the workspace. Used by the
+   * compiler to adjust the output depending on the available symbols.
+   *
+   * @internal
+   */
+  _angularCoreVersion?: string;
 }
 
 /**

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -266,6 +266,7 @@ export class NgCompiler {
   readonly ignoreForEmit: Set<ts.SourceFile>;
   readonly enableTemplateTypeChecker: boolean;
   private readonly enableBlockSyntax: boolean;
+  private readonly angularCoreVersion: Version|null;
 
   /**
    * `NgCompiler` can be reused for multiple compilations (for resource-only changes), and each
@@ -330,7 +331,10 @@ export class NgCompiler {
   ) {
     this.enableTemplateTypeChecker =
         enableTemplateTypeChecker || (options['_enableTemplateTypeChecker'] ?? false);
+    // TODO(crisbeto): remove this flag and base `enableBlockSyntx` on the `angularCoreVersion`.
     this.enableBlockSyntax = options['_enableBlockSyntax'] ?? true;
+    this.angularCoreVersion =
+        options['_angularCoreVersion'] == null ? null : new Version(options['_angularCoreVersion']);
     this.constructionDiagnostics.push(
         ...this.adapter.constructionDiagnostics, ...verifyCompatibleTypeCheckOptions(this.options));
 
@@ -791,6 +795,14 @@ export class NgCompiler {
 
     const useInlineTypeConstructors = this.programDriver.supportsInlineOperations;
 
+    // Only Angular versions greater than 17.2 have the necessary symbols to type check signals in
+    // two-way bindings. We also allow version 0.0.0 in case somebody is using Angular at head.
+    const allowSignalsInTwoWayBindings = this.angularCoreVersion === null ||
+        this.angularCoreVersion.major > 17 ||
+        this.angularCoreVersion.major === 17 && this.angularCoreVersion.minor >= 2 ||
+        (this.angularCoreVersion.major === 0 && this.angularCoreVersion.minor === 0 ||
+         this.angularCoreVersion.patch === 0);
+
     // First select a type-checking configuration, based on whether full template type-checking is
     // requested.
     let typeCheckingConfig: TypeCheckingConfig;
@@ -829,6 +841,7 @@ export class NgCompiler {
         suggestionsForSuboptimalTypeInference: this.enableTemplateTypeChecker && !strictTemplates,
         controlFlowPreventingContentProjection:
             this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
+        allowSignalsInTwoWayBindings,
       };
     } else {
       typeCheckingConfig = {
@@ -859,6 +872,7 @@ export class NgCompiler {
         suggestionsForSuboptimalTypeInference: false,
         controlFlowPreventingContentProjection:
             this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
+        allowSignalsInTwoWayBindings,
       };
     }
 
@@ -1441,3 +1455,17 @@ function versionMapFromProgram(
   }
   return versions;
 }
+
+/** Representation of a parsed persion string. */
+class Version {
+  readonly major: number;
+  readonly minor: number;
+  readonly patch: number;
+
+  constructor(public full: string) {
+    [this.major, this.minor, this.patch] = full.split('.').map(part => {
+      const parsed = parseInt(part);
+      return isNaN(parsed) ? -1 : parsed;
+    });
+  }
+}

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

@@ -330,6 +330,11 @@ export interface TypeCheckingConfig {
    * opportunities to improve their own developer experience.
    */
   suggestionsForSuboptimalTypeInference: boolean;
+
+  /**
+   * Whether the type of two-way bindings should be widened to allow `WritableSignal`.
+   */
+  allowSignalsInTwoWayBindings: boolean;
 }
 
 

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

@@ -815,7 +815,7 @@ class TcbDirectiveInputsOp extends TcbOp {
         }
 
         // Two-way bindings accept `T | WritableSignal<T>` so we have to unwrap the value.
-        if (isTwoWayBinding) {
+        if (isTwoWayBinding && this.tcb.env.config.allowSignalsInTwoWayBindings) {
           assignment = unwrapWritableSignal(assignment, this.tcb);
         }
 
@@ -2499,7 +2499,7 @@ function tcbCallTypeCtor(
       // For bound inputs, the property is assigned the binding expression.
       let expr = widenBinding(input.expression, tcb);
 
-      if (input.isTwoWayBinding) {
+      if (input.isTwoWayBinding && tcb.env.config.allowSignalsInTwoWayBindings) {
         expr = unwrapWritableSignal(expr, tcb);
       }
 

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

@@ -673,8 +673,10 @@ describe('type check blocks', () => {
       isGeneric: true,
     }];
     const block = tcb(TEMPLATE, DIRECTIVES);
-    expect(block).toContain('const _ctor1: <T extends string = any>(init: Pick<i0.TwoWay<T>, "input">) => i0.TwoWay<T> = null!');
-    expect(block).toContain('var _t1 = _ctor1({ "input": (i1.ɵunwrapWritableSignal(((this).value))) });');
+    expect(block).toContain(
+        'const _ctor1: <T extends string = any>(init: Pick<i0.TwoWay<T>, "input">) => i0.TwoWay<T> = null!');
+    expect(block).toContain(
+        'var _t1 = _ctor1({ "input": (i1.ɵunwrapWritableSignal(((this).value))) });');
     expect(block).toContain('_t1.input = i1.ɵunwrapWritableSignal((((this).value)));');
   });
 
@@ -697,7 +699,8 @@ describe('type check blocks', () => {
     }];
     const block = tcb(TEMPLATE, DIRECTIVES);
     expect(block).toContain('var _t1 = null! as i0.TwoWay;');
-    expect(block).toContain('_t1.input[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE] = i1.ɵunwrapWritableSignal((((this).value)));');
+    expect(block).toContain(
+        '_t1.input[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE] = i1.ɵunwrapWritableSignal((((this).value)));');
   });
 
   it('should handle a two-way binding to an input with a transform', () => {
@@ -871,6 +874,7 @@ describe('type check blocks', () => {
       useInlineTypeConstructors: true,
       suggestionsForSuboptimalTypeInference: false,
       controlFlowPreventingContentProjection: 'warning',
+      allowSignalsInTwoWayBindings: true,
     };
 
     describe('config.applyTemplateContextGuards', () => {
@@ -1214,6 +1218,37 @@ describe('type check blocks', () => {
             '_t1.fieldA = (((this).foo)); ');
       });
     });
+
+    describe('config.allowSignalsInTwoWayBindings', () => {
+      it('should not unwrap signals in two-way binding expressions', () => {
+        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, {...BASE_CONFIG, allowSignalsInTwoWayBindings: false});
+        expect(block).not.toContain('ɵunwrapWritableSignal');
+      });
+
+      it('should not unwrap signals in two-way bindings to generic directives', () => {
+        const TEMPLATE = `<div twoWay [(input)]="value"></div>`;
+        const DIRECTIVES: TestDeclaration[] = [{
+          type: 'directive',
+          name: 'TwoWay',
+          selector: '[twoWay]',
+          inputs: {input: 'input'},
+          outputs: {inputChange: 'inputChange'},
+          isGeneric: true,
+        }];
+        const block =
+            tcb(TEMPLATE, DIRECTIVES, {...BASE_CONFIG, allowSignalsInTwoWayBindings: false});
+        expect(block).not.toContain('ɵunwrapWritableSignal');
+      });
+    });
   });
 
   it('should use `any` type for type constructors with bound generic params ' +

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

@@ -214,6 +214,7 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
   useInlineTypeConstructors: true,
   suggestionsForSuboptimalTypeInference: false,
   controlFlowPreventingContentProjection: 'warning',
+  allowSignalsInTwoWayBindings: true,
 };
 
 // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
@@ -325,6 +326,7 @@ export function tcb(
     enableTemplateTypeChecker: false,
     useInlineTypeConstructors: true,
     suggestionsForSuboptimalTypeInference: false,
+    allowSignalsInTwoWayBindings: true,
     ...config
   };
   options = options || {

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

@@ -531,5 +531,63 @@ runInEachFileSystem(() => {
       const diags = env.driveDiagnostics();
       expect(diags).toEqual([]);
     });
+
+    it('should not widen the type of two-way bindings on Angular versions less than 17.2', () => {
+      env.tsconfig({_angularCoreVersion: '16.50.60', strictTemplates: true});
+      env.write('test.ts', `
+        import {Component, Directive, Input, Output, EventEmitter, signal} 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(1);
+        }
+      `);
+
+      const diags = env.driveDiagnostics();
+      expect(diags.length).toBe(1);
+      expect(diags[0].messageText)
+          .toBe(`Type 'WritableSignal<number>' is not assignable to type 'number'.`);
+    });
+
+    it('should widen the type of two-way bindings on Angular version 0.0.0', () => {
+      env.tsconfig({_angularCoreVersion: '0.0.0-PLACEHOLDER', strictTemplates: true});
+      env.write('test.ts', `
+        import {Component, Directive, Input, Output, EventEmitter, signal} 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(1);
+        }
+      `);
+
+      const diags = env.driveDiagnostics();
+      expect(diags).toEqual([]);
+    });
   });
 });

packages/language-service/api.ts

@@ -31,6 +31,11 @@ export interface PluginConfig {
    * versions of Angular that do not support blocks (pre-v17) used with the language service.
    */
   enableBlockSyntax?: false;
+
+  /**
+   * Version of `@angular/core` that was detected in the user's workspace.
+   */
+  angularCoreVersion?: string;
 }
 
 export type GetTcbResponse = {

packages/language-service/src/language_service.ts

@@ -523,6 +523,8 @@ function parseNgCompilerOptions(
     options['_enableBlockSyntax'] = false;
   }
 
+  options['_angularCoreVersion'] = config.angularCoreVersion;
+
   return options;
 }
 

packages/language-service/test/legacy/language_service_spec.ts

@@ -83,7 +83,7 @@ describe('language service adapter', () => {
     });
 
     it('should always disable block syntax if enableBlockSyntax is false', () => {
-      const {project, tsLS, configFileFs} = setup();
+      const {project, tsLS} = setup();
       const ngLS = new LanguageService(project, tsLS, {
         enableBlockSyntax: false,
       });
@@ -92,6 +92,17 @@ describe('language service adapter', () => {
         '_enableBlockSyntax': false,
       }));
     });
+
+    it('should pass the @angular/core version along to the compiler', () => {
+      const {project, tsLS} = setup();
+      const ngLS = new LanguageService(project, tsLS, {
+        angularCoreVersion: '17.2.11-rc.8',
+      });
+
+      expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({
+        '_angularCoreVersion': '17.2.11-rc.8',
+      }));
+    });
   });
 
   describe('compiler options diagnostics', () => {