Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(compiler-cli): remove checkTwoWayBoundEvents flag

Browse source 
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.
This commit is contained in:
Kristiyan Kostadinov 2026-04-28 09:10:16 +02:00 committed by Alon Mishne
parent 273ff07469
commit 72be5be9c1
11 changed files with 26 additions and 149 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -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,

3
packages/compiler-cli/src/ngtsc/core/src/compiler.ts
View file

@ -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,
};
}

34
packages/compiler-cli/src/ngtsc/typecheck/test/model_signal_diagnostics_spec.ts
View file

@ -381,10 +381,7 @@ runInEachFileSystem(() => {
otherVal!: string;
`,
template: `<div dir [(gen)]="genVal" [(other)]="otherVal">`,
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<string>'}},
template: `<div dir [(value)]="bla">`,
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<boolean>'}},
template: `<div dir [(value)]="val">`,
component: `val!: WritableSignal<string>;`,
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<boolean>;`,
expected: [
`TestComponent.html(1, 10): Type 'InputSignal<boolean>' is not assignable to type 'boolean'.`,
`TestComponent.html(1, 10): Type 'boolean' is not assignable to type 'InputSignal<boolean>'.`,
],
},
{
@ -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`,
),
],
},
];

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

@ -31,10 +31,7 @@ runInEachFileSystem(() => {
outputs: {'valueChange': {type: 'OutputEmitterRef<string>'}},
template: `<div dir [(value)]="bla">`,
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',

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

@ -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,
};

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

@ -288,7 +288,6 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
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,
};

5
packages/compiler-cli/test/ngtsc/authoring_inputs_spec.ts
View file

@ -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', () => {

30
packages/compiler-cli/test/ngtsc/authoring_models_spec.ts
View file

@ -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<number>' is not assignable to type 'number'.`,
);
expect(diags[1].messageText).toBe(
`Type 'number' is not assignable to type 'InputSignal<number>'.`,
);
});
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', () => {

52
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View file

@ -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<number>();
}
@Component({
template: '<div dir [(value)]="value"></div>',
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',
`

5
packages/compiler/src/typecheck/api.ts
View file

@ -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;
}

19
packages/compiler/src/typecheck/ops/events.ts
View file

@ -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) {