refactor(compiler-cli): move two-way binding fix behind flag (#59002)

crisbeto · alxhub · commit 6fd8a2097887 · 2024-12-05T16:11:02.000-08:00
Moves the fix for type checking the event side of two-way bindings behind a compiler flag so that we can roll it out in v20. PR Close #59002
diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/options.ts
@@ -120,6 +120,12 @@ export interface InternalOptions {
    * @internal
    */
   _enableHmr?: boolean;
+
+  // TODO(crisbeto): this is a temporary flag that will be removed in v20.
+  /**
+   * Whether to check the event side of two-way bindings.
+   */
+  _checkTwoWayBoundEvents?: boolean;
 }
 
 /**
diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts
@@ -1027,6 +1027,7 @@ export class NgCompiler {
     const strictTemplates = !!this.options.strictTemplates;
 
     const useInlineTypeConstructors = this.programDriver.supportsInlineOperations;
+    const checkTwoWayBoundEvents = this.options['_checkTwoWayBoundEvents'] ?? false;
 
     // Check whether the loaded version of `@angular/core` in the `ts.Program` supports unwrapping
     // writable signals for type-checking. If this check fails to find a suitable .d.ts file, fall
@@ -1080,6 +1081,7 @@ export class NgCompiler {
         unusedStandaloneImports:
           this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
         allowSignalsInTwoWayBindings,
+        checkTwoWayBoundEvents,
       };
     } else {
       typeCheckingConfig = {
@@ -1114,6 +1116,7 @@ export class NgCompiler {
         unusedStandaloneImports:
           this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
         allowSignalsInTwoWayBindings,
+        checkTwoWayBoundEvents,
       };
     }
 
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts
@@ -357,6 +357,11 @@ export interface TypeCheckingConfig {
    * Whether to descend into the bodies of control flow blocks (`@if`, `@switch` and `@for`).
    */
   checkControlFlowBodies: boolean;
+
+  /**
+   * Whether the event side of a two-way binding should be type checked.
+   */
+  checkTwoWayBoundEvents: boolean;
 }
 
 export type TemplateSourceMapping =
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -3094,7 +3094,8 @@ function tcbCreateEventHandler(
   const handler = tcbEventHandlerExpression(event.handler, tcb, scope);
   const statements: ts.Statement[] = [];
 
-  if (event.type === ParsedEventType.TwoWay) {
+  // TODO(crisbeto): remove the `checkTwoWayBoundEvents` check in v20.
+  if (event.type === ParsedEventType.TwoWay && tcb.env.config.checkTwoWayBoundEvents) {
     // If we're dealing with a two-way event, we create a variable initialized to the unwrapped
     // signal value of the expression and then we assign `$event` to it. Note that in most cases
     // this will already be covered by the corresponding input binding, however it allows us to
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
@@ -981,6 +981,7 @@ describe('type check blocks', () => {
       controlFlowPreventingContentProjection: 'warning',
       unusedStandaloneImports: 'warning',
       allowSignalsInTwoWayBindings: true,
+      checkTwoWayBoundEvents: true,
     };
 
     describe('config.applyTemplateContextGuards', () => {
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts
@@ -284,6 +284,7 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
   controlFlowPreventingContentProjection: 'warning',
   unusedStandaloneImports: 'warning',
   allowSignalsInTwoWayBindings: true,
+  checkTwoWayBoundEvents: true,
 };
 
 // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
@@ -423,6 +424,7 @@ export function tcb(
     useInlineTypeConstructors: true,
     suggestionsForSuboptimalTypeInference: false,
     allowSignalsInTwoWayBindings: true,
+    checkTwoWayBoundEvents: true,
     ...config,
   };
   options = options || {
diff --git a/packages/compiler-cli/test/ngtsc/authoring_inputs_spec.ts b/packages/compiler-cli/test/ngtsc/authoring_inputs_spec.ts
@@ -21,7 +21,7 @@ runInEachFileSystem(() => {
 
     beforeEach(() => {
       env = NgtscTestEnvironment.setup(testFiles);
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
     });
 
     it('should handle a basic, primitive valued input', () => {
diff --git a/packages/compiler-cli/test/ngtsc/authoring_models_spec.ts b/packages/compiler-cli/test/ngtsc/authoring_models_spec.ts
@@ -21,7 +21,7 @@ runInEachFileSystem(() => {
 
     beforeEach(() => {
       env = NgtscTestEnvironment.setup(testFiles);
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
     });
 
     it('should declare an input/output pair for a field initialized to a model()', () => {
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -544,7 +544,7 @@ runInEachFileSystem(() => {
     });
 
     it('should type check a two-way binding to a generic property', () => {
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
       env.write(
         'test.ts',
         `
@@ -582,7 +582,7 @@ runInEachFileSystem(() => {
     });
 
     it('should use the setter type when assigning using a two-way binding to an input with different getter and setter types', () => {
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
       env.write(
         'test.ts',
         `
@@ -618,7 +618,7 @@ runInEachFileSystem(() => {
     });
 
     it('should type check a two-way binding to a function value', () => {
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
       env.write(
         'test.ts',
         `
@@ -658,7 +658,7 @@ runInEachFileSystem(() => {
     });
 
     it('should type check a two-way binding to input/output pair where the input has a wider type than the output', () => {
-      env.tsconfig({strictTemplates: true});
+      env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
       env.write(
         'test.ts',
         `
@@ -3007,7 +3007,7 @@ runInEachFileSystem(() => {
       });
 
       it('should type check a two-way binding to an input with a transform', () => {
-        env.tsconfig({strictTemplates: true});
+        env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true});
         env.write(
           'test.ts',
           `
