From 72be5be9c1d6486a54d8ea02cc44222d19cbed36 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 28 Apr 2026 09:10:16 +0200 Subject: [PATCH] refactor(compiler-cli): remove checkTwoWayBoundEvents flag Removes the `checkTwoWayBoundEvents` flag since the code it generates is quite breaking and we never got the chance to enable it. Also it caused our tests to misrepresent how the compiler behaves for actual users. --- .../src/ngtsc/core/api/src/options.ts | 9 +--- .../src/ngtsc/core/src/compiler.ts | 3 -- .../test/model_signal_diagnostics_spec.ts | 34 +++--------- .../test/output_function_diagnostics.spec.ts | 5 +- .../typecheck/test/type_check_block_spec.ts | 11 ---- .../src/ngtsc/typecheck/testing/index.ts | 2 - .../test/ngtsc/authoring_inputs_spec.ts | 5 +- .../test/ngtsc/authoring_models_spec.ts | 30 +++-------- .../test/ngtsc/template_typecheck_spec.ts | 52 +++---------------- packages/compiler/src/typecheck/api.ts | 5 -- packages/compiler/src/typecheck/ops/events.ts | 19 +------ 11 files changed, 26 insertions(+), 149 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/options.ts index a88187f5b78..51e1b4694cc 100644 --- a/packages/compiler-cli/src/ngtsc/core/api/src/options.ts +++ b/packages/compiler-cli/src/ngtsc/core/api/src/options.ts @@ -112,12 +112,6 @@ export interface InternalOptions { */ _enableSelectorless?: 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; - /** * Whether this is a compilation of Angular core itself. * @@ -135,7 +129,8 @@ export interface InternalOptions { * Also includes a few miscellaneous options. */ export interface NgCompilerOptions - extends ts.CompilerOptions, + extends + ts.CompilerOptions, LegacyNgcOptions, BazelAndG3Options, DiagnosticOptions, diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 17df0f53ac5..e4e78b3e8bb 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -1053,7 +1053,6 @@ export class NgCompiler { const strictTemplates = this.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. Only Angular versions greater than 17.2 have the necessary @@ -1104,7 +1103,6 @@ export class NgCompiler { unusedStandaloneImports: this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, allowSignalsInTwoWayBindings, - checkTwoWayBoundEvents, allowDomEventAssertion, }; } else { @@ -1137,7 +1135,6 @@ export class NgCompiler { unusedStandaloneImports: this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, allowSignalsInTwoWayBindings, - checkTwoWayBoundEvents, allowDomEventAssertion, }; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/model_signal_diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/model_signal_diagnostics_spec.ts index 23dab7dcf03..80d40532a22 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/model_signal_diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/model_signal_diagnostics_spec.ts @@ -381,10 +381,7 @@ runInEachFileSystem(() => { otherVal!: string; `, template: `
`, - expected: [ - `TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`, - `TestComponent.html(1, 27): Type 'boolean' is not assignable to type 'string'.`, - ], + expected: [`TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`], }, { id: 'generic inference and two-way binding to directive, mix of zone input and model', @@ -408,10 +405,7 @@ runInEachFileSystem(() => { genVal!: boolean; otherVal!: string; `, - expected: [ - `TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`, - `TestComponent.html(1, 10): Type 'string' is not assignable to type 'boolean'.`, - ], + expected: [`TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`], }, { id: 'generic inference and two-way binding to directive (with `extends boolean`), all model inputs', @@ -432,10 +426,7 @@ runInEachFileSystem(() => { genVal!: boolean; otherVal!: string; `, - expected: [ - `TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`, - `TestComponent.html(1, 27): Type 'boolean' is not assignable to type 'string'.`, - ], + expected: [`TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`], }, { id: 'generic inference and two-way binding to directive (with `extends boolean`), mix of zone inputs and model', @@ -459,10 +450,7 @@ runInEachFileSystem(() => { genVal!: boolean; otherVal!: string; `, - expected: [ - `TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`, - `TestComponent.html(1, 27): Type 'boolean' is not assignable to type 'string'.`, - ], + expected: [`TestComponent.html(1, 29): Type 'string' is not assignable to type 'boolean'.`], }, { id: 'generic multi-inference and two-way bindings to directive, all model inputs', @@ -595,10 +583,7 @@ runInEachFileSystem(() => { outputs: {valueChange: {type: 'ModelSignal'}}, template: `
`, component: `bla = true;`, - expected: [ - `TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`, - `TestComponent.html(1, 10): Type 'string' is not assignable to type 'boolean'.`, - ], + expected: [`TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`], }, { id: 'two-way binding to primitive, valid', @@ -667,10 +652,7 @@ runInEachFileSystem(() => { outputs: {valueChange: {type: 'ModelSignal'}}, template: `
`, component: `val!: WritableSignal;`, - expected: [ - `TestComponent.html(1, 12): Type 'string' is not assignable to type 'boolean'.`, - `TestComponent.html(1, 10): Type 'boolean' is not assignable to type 'string'.`, - ], + expected: [`TestComponent.html(1, 12): Type 'string' is not assignable to type 'boolean'.`], }, { id: 'non-writable signal binding', @@ -680,7 +662,6 @@ runInEachFileSystem(() => { component: `val!: InputSignal;`, expected: [ `TestComponent.html(1, 10): Type 'InputSignal' is not assignable to type 'boolean'.`, - `TestComponent.html(1, 10): Type 'boolean' is not assignable to type 'InputSignal'.`, ], }, { @@ -701,9 +682,6 @@ runInEachFileSystem(() => { jasmine.stringContaining( `TestComponent.html(1, 12): Type '(v: string) => number' is not assignable to type '(v: number) => number`, ), - jasmine.stringContaining( - `TestComponent.html(1, 10): Type '(v: number) => number' is not assignable to type '(v: string) => number`, - ), ], }, ]; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/output_function_diagnostics.spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/output_function_diagnostics.spec.ts index 2a17b46dc83..b3c86afb7bf 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/output_function_diagnostics.spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/output_function_diagnostics.spec.ts @@ -31,10 +31,7 @@ runInEachFileSystem(() => { outputs: {'valueChange': {type: 'OutputEmitterRef'}}, template: `
`, component: `bla = true;`, - expected: [ - `TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`, - `TestComponent.html(1, 10): Type 'string' is not assignable to type 'boolean'.`, - ], + expected: [`TestComponent.html(1, 12): Type 'boolean' is not assignable to type 'string'.`], }, { id: 'two way data binding, valid', 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 index a62824d52b8..128f39c6ef4 100644 --- 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 @@ -875,8 +875,6 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('var _t1 = null! as i0.TwoWay;'); expect(block).toContain('_t1.input = i1.ɵunwrapWritableSignal((((this).value)));'); - expect(block).toContain('var _t2 = i1.ɵunwrapWritableSignal(((this).value));'); - expect(block).toContain('_t2 = $event;'); }); it('should handle a two-way binding to an input/output pair of a generic directive', () => { @@ -899,8 +897,6 @@ describe('type check blocks', () => { 'var _t1 = _ctor1({ "input": (i1.ɵunwrapWritableSignal(((this).value))) });', ); expect(block).toContain('_t1.input = i1.ɵunwrapWritableSignal((((this).value)));'); - expect(block).toContain('var _t2 = i1.ɵunwrapWritableSignal(((this).value));'); - expect(block).toContain('_t2 = $event;'); }); it('should handle a two-way binding to a model()', () => { @@ -927,8 +923,6 @@ describe('type check blocks', () => { expect(block).toContain( '_t1.input[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE] = i1.ɵunwrapWritableSignal((((this).value)));', ); - expect(block).toContain('var _t2 = i1.ɵunwrapWritableSignal(((this).value));'); - expect(block).toContain('_t2 = $event;'); }); it('should handle a two-way binding to an input with a transform', () => { @@ -970,8 +964,6 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('var _t1 = null! as boolean | string;'); expect(block).toContain('_t1 = i1.ɵunwrapWritableSignal((((this).value)));'); - expect(block).toContain('var _t3 = i1.ɵunwrapWritableSignal(((this).value));'); - expect(block).toContain('_t3 = $event;'); }); describe('experimental DOM checking via lib.dom.d.ts', () => { @@ -1099,8 +1091,6 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('var _t1 = null! as i0.TwoWay;'); expect(block).toContain('_t1.input = i1.ɵunwrapWritableSignal(((((this).value) as any)));'); - expect(block).toContain('var _t2 = i1.ɵunwrapWritableSignal((((this).value) as any));'); - expect(block).toContain('_t2 = $event;'); }); it('should detect writes to template variables', () => { @@ -1156,7 +1146,6 @@ describe('type check blocks', () => { controlFlowPreventingContentProjection: 'warning', unusedStandaloneImports: 'warning', allowSignalsInTwoWayBindings: true, - checkTwoWayBoundEvents: true, allowDomEventAssertion: true, }; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 3216dedc4a4..75c3e11b43e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -288,7 +288,6 @@ export const ALL_ENABLED_CONFIG: Readonly = { controlFlowPreventingContentProjection: 'warning', unusedStandaloneImports: 'warning', allowSignalsInTwoWayBindings: true, - checkTwoWayBoundEvents: true, allowDomEventAssertion: true, }; @@ -443,7 +442,6 @@ export function tcb( enableTemplateTypeChecker: false, useInlineTypeConstructors: true, allowSignalsInTwoWayBindings: true, - checkTwoWayBoundEvents: true, allowDomEventAssertion: true, ...config, }; diff --git a/packages/compiler-cli/test/ngtsc/authoring_inputs_spec.ts b/packages/compiler-cli/test/ngtsc/authoring_inputs_spec.ts index 358de9ad349..6122cfd3720 100644 --- 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, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); }); it('should handle a basic, primitive valued input', () => { @@ -255,9 +255,8 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toBe(`Type 'number' is not assignable to type 'string'.`); - expect(diags[1].messageText).toBe(`Type 'string' is not assignable to type 'number'.`); }); describe('type checking', () => { diff --git a/packages/compiler-cli/test/ngtsc/authoring_models_spec.ts b/packages/compiler-cli/test/ngtsc/authoring_models_spec.ts index 1dc192d4f62..d6359255929 100644 --- 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, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); }); it('should declare an input/output pair for a field initialized to a model()', () => { @@ -294,9 +294,8 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toBe(`Type 'boolean' is not assignable to type 'number'.`); - expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`); }); it('should check a signal value bound to a model input via a two-way binding', () => { @@ -323,9 +322,8 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toBe(`Type 'boolean' is not assignable to type 'number'.`); - expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`); }); it('should check two-way binding of a signal to a decorator-based input/output pair', () => { @@ -353,9 +351,8 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toBe(`Type 'boolean' is not assignable to type 'number'.`); - expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`); }); it('should not allow a non-writable signal to be assigned to a model', () => { @@ -382,13 +379,10 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toBe( `Type 'InputSignal' is not assignable to type 'number'.`, ); - expect(diags[1].messageText).toBe( - `Type 'number' is not assignable to type 'InputSignal'.`, - ); }); it('should allow a model signal to be bound to another model signal', () => { @@ -501,17 +495,12 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toEqual( jasmine.objectContaining({ messageText: `Type '{ id: number; }' is not assignable to type '{ id: string; }'.`, }), ); - expect(diags[1].messageText).toEqual( - jasmine.objectContaining({ - messageText: `Type '{ id: string; }' is not assignable to type '{ id: number; }'.`, - }), - ); }); it('should check generic two-way model binding with a signal value', () => { @@ -538,17 +527,12 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toEqual( jasmine.objectContaining({ messageText: `Type '{ id: number; }' is not assignable to type '{ id: string; }'.`, }), ); - expect(diags[1].messageText).toEqual( - jasmine.objectContaining({ - messageText: `Type '{ id: string; }' is not assignable to type '{ id: number; }'.`, - }), - ); }); it('should report unwrapped signals assigned to a model in a one-way binding', () => { diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 0c1d09b043d..fe4860f9c99 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -543,7 +543,7 @@ runInEachFileSystem(() => { }); it('should type check a two-way binding to a generic property', () => { - env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); env.write( 'test.ts', ` @@ -566,21 +566,16 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toEqual( jasmine.objectContaining({ messageText: `Type '{ id: number; }' is not assignable to type '{ id: string; }'.`, }), ); - expect(diags[1].messageText).toEqual( - jasmine.objectContaining({ - messageText: `Type '{ id: string; }' is not assignable to type '{ id: number; }'.`, - }), - ); }); 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, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); env.write( 'test.ts', ` @@ -615,7 +610,7 @@ runInEachFileSystem(() => { }); it('should type check a two-way binding to a function value', () => { - env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); env.write( 'test.ts', ` @@ -640,21 +635,16 @@ runInEachFileSystem(() => { ); const diags = env.driveDiagnostics(); - expect(diags.length).toBe(2); + expect(diags.length).toBe(1); expect(diags[0].messageText).toEqual( jasmine.objectContaining({ messageText: `Type '(val: string) => number' is not assignable to type 'TestFn'.`, }), ); - expect(diags[1].messageText).toEqual( - jasmine.objectContaining({ - messageText: `Type 'TestFn' is not assignable to type '(val: string) => number'.`, - }), - ); }); it('should be able to cast to any in a two-way binding', () => { - env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); env.write( 'test.ts', ` @@ -680,34 +670,6 @@ runInEachFileSystem(() => { expect(diags.length).toBe(0); }); - 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, _checkTwoWayBoundEvents: true}); - env.write( - 'test.ts', - ` - import {Component, Directive, Input, Output, EventEmitter} from '@angular/core'; - - @Directive({selector: '[dir]'}) - export class Dir { - @Input() value: string | number; - @Output() valueChange = new EventEmitter(); - } - - @Component({ - template: '
', - imports: [Dir], - }) - export class App { - value = 'hello'; - } - `, - ); - - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(1); - expect(diags[0].messageText).toBe(`Type 'number' is not assignable to type 'string'.`); - }); - it('should check the fallback content of ng-content', () => { env.write( 'test.ts', @@ -3084,7 +3046,7 @@ runInEachFileSystem(() => { }); it('should type check a two-way binding to an input with a transform', () => { - env.tsconfig({strictTemplates: true, _checkTwoWayBoundEvents: true}); + env.tsconfig({strictTemplates: true}); env.write( 'test.ts', ` diff --git a/packages/compiler/src/typecheck/api.ts b/packages/compiler/src/typecheck/api.ts index c4c290631f9..ccf5adb14ed 100644 --- a/packages/compiler/src/typecheck/api.ts +++ b/packages/compiler/src/typecheck/api.ts @@ -288,9 +288,4 @@ 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; } diff --git a/packages/compiler/src/typecheck/ops/events.ts b/packages/compiler/src/typecheck/ops/events.ts index 307f2bcf0a4..ffddcbbac48 100644 --- a/packages/compiler/src/typecheck/ops/events.ts +++ b/packages/compiler/src/typecheck/ops/events.ts @@ -297,24 +297,7 @@ function tcbCreateEventHandler( statements.push(assertionExpression); } - // 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 - // handle the case where the input has a wider type than the output (see #58971). - const target = tcb.allocateId(); - const initializer = tcb.env.config.allowSignalsInTwoWayBindings - ? unwrapWritableSignal(handler, tcb) - : handler; - - statements.push( - new TcbExpr(`var ${target} = ${initializer.print()}`), - new TcbExpr(`${target} = ${EVENT_PARAMETER}`), - ); - } else { - statements.push(handler); - } + statements.push(handler); let eventParamType: string | undefined; if (eventType === EventParamType.Infer) {