Skip to content

Commit 5de354f

Browse files
committed
fix(forms): create controls before listeners
Create form controls before registering template listeners so custom control values are synchronized before user callbacks run. Cover Signal Forms, reactive forms, and template-driven forms.
1 parent 1b13db7 commit 5de354f

10 files changed

Lines changed: 122 additions & 16 deletions

File tree

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/control_bindings.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ export class MyComponent {
1212
i0.ɵɵelementStart(1, "div");
1313
i0.ɵɵtext(2, "Not a form control either.");
1414
i0.ɵɵelementEnd();
15-
i0.ɵɵelement(3, "input", 1);
15+
i0.ɵɵelementStart(3, "input", 1);
1616
i0.ɵɵcontrolCreate();
17+
i0.ɵɵelementEnd();
1718
}
1819
if (rf & 2) {
1920
i0.ɵɵadvance();

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ export class MyComponent {
88
consts: [["type", "radio", "id", "radio", 3, "formField", "value"], ["type", "radio", "id", "radio", 3, "value", "formField"]],
99
template: function MyComponent_Template(rf, ctx) {
1010
if (rf & 1) {
11-
i0.ɵɵelement(0, "input", 0);
11+
i0.ɵɵelementStart(0, "input", 0);
1212
i0.ɵɵcontrolCreate();
13-
i0.ɵɵelement(1, "input", 1);
13+
i0.ɵɵelementEnd();
14+
i0.ɵɵelementStart(1, "input", 1);
1415
i0.ɵɵcontrolCreate();
16+
i0.ɵɵelementEnd();
1517
}
1618
if (rf & 2) {
1719
i0.ɵɵproperty("formField", ctx.value)("value", "foo");

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/nested_two_way_template.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ function TestCmp_ng_template_1_Template(rf, ctx) {
22
if (rf & 1) {
33
const $_r2$ = $r3$.ɵɵgetCurrentView();
44
$r3$.ɵɵelementStart(0, "input", 0);
5+
$r3$.ɵɵcontrolCreate();
56
$r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_ng_template_1_Template_input_ngModelChange_0_listener($event) {
67
$r3$.ɵɵrestoreView($_r2$);
78
const $ctx_r1$ = $r3$.ɵɵnextContext();
89
$r3$.ɵɵtwoWayBindingSet($ctx_r1$.name, $event) || ($ctx_r1$.name = $event);
910
return $r3$.ɵɵresetView($event);
1011
});
1112
$r3$.ɵɵelementEnd();
12-
$r3$.ɵɵcontrolCreate();
1313
} if (rf & 2) {
1414
const $ctx_r0$ = $r3$.ɵɵnextContext();
1515
$r3$.ɵɵtwoWayProperty("ngModel", $ctx_r0$.name);

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/simple_two_way_template.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ function TestCmp_Template(rf, ctx) {
22
if (rf & 1) {
33
$r3$.ɵɵtext(0, "Name: ");
44
$r3$.ɵɵelementStart(1, "input", 0);
5+
$r3$.ɵɵcontrolCreate();
56
$r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_Template_input_ngModelChange_1_listener($event) {
67
$r3$.ɵɵtwoWayBindingSet(ctx.name, $event) || (ctx.name = $event);
78
return $event;
89
});
910
$r3$.ɵɵelementEnd();
10-
$r3$.ɵɵcontrolCreate();
1111
} if (rf & 2) {
1212
$r3$.ɵɵadvance();
1313
$r3$.ɵɵtwoWayProperty("ngModel", ctx.name);

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/two_way_binding_to_signal_loop_variable_template.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ function TestCmp_For_1_Template(rf, ctx) {
22
if (rf & 1) {
33
const $_r1$ = $r3$.ɵɵgetCurrentView();
44
$r3$.ɵɵelementStart(0, "input", 1);
5+
$r3$.ɵɵcontrolCreate();
56
$r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_For_1_Template_input_ngModelChange_0_listener($event) {
67
const $name_r2$ = $r3$.ɵɵrestoreView($_r1$).$implicit;
78
$r3$.ɵɵtwoWayBindingSet($name_r2$, $event);
89
return $r3$.ɵɵresetView($event);
910
});
1011
$r3$.ɵɵelementEnd();
11-
$r3$.ɵɵcontrolCreate();
1212
}
1313
if (rf & 2) {
1414
const $name_r2$ = ctx.$implicit;

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/two_way_to_any_template.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
function TestCmp_Template(rf, ctx) {
22
if (rf & 1) {
33
$r3$.ɵɵelementStart(0, "input", 0);
4+
$r3$.ɵɵcontrolCreate();
45
$r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_Template_input_ngModelChange_0_listener($event) {
56
$r3$.ɵɵtwoWayBindingSet(ctx.value, $event) || (ctx.value = $event);
67
return $event;
78
});
89
$r3$.ɵɵelementEnd();
9-
$r3$.ɵɵcontrolCreate();
1010
}
1111
if (rf & 2) {
1212
$r3$.ɵɵtwoWayProperty("ngModel", ctx.value);

packages/compiler/src/template/pipeline/src/phases/control_directives.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,28 +43,22 @@ function processView(view: ViewCompilationUnit): void {
4343
const CONTROL_OP_CREATE_KINDS = new Set([
4444
ir.OpKind.Container,
4545
ir.OpKind.ContainerStart,
46-
ir.OpKind.ContainerEnd,
4746
ir.OpKind.Element,
4847
ir.OpKind.ElementStart,
49-
ir.OpKind.ElementEnd,
50-
ir.OpKind.Template,
5148
]);
5249

5350
function isRelevantCreateOp(createOp: ir.CreateOp): createOp is ir.CreateOp & {xref: ir.XrefId} {
5451
return CONTROL_OP_CREATE_KINDS.has(createOp.kind);
5552
}
5653

5754
function findCreateInstruction(view: ViewCompilationUnit, target: ir.XrefId): ir.CreateOp | null {
58-
let lastFoundOp: ir.CreateOp | null = null;
5955
for (const createOp of view.create) {
60-
if (!isRelevantCreateOp(createOp) || createOp.xref !== target) {
61-
continue;
56+
if (isRelevantCreateOp(createOp) && createOp.xref === target) {
57+
return createOp;
6258
}
63-
64-
lastFoundOp = createOp;
6559
}
6660

67-
return lastFoundOp;
61+
return null;
6862
}
6963

7064
function addControlInstruction(

packages/forms/signals/test/web/form_field.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,32 @@ describe('field directive', () => {
136136
});
137137
expect(component.model()).toEqual({x: 'a', y: 'c'});
138138
});
139+
140+
it('should update field value before template (input) listener fires', () => {
141+
@Component({
142+
imports: [FormField],
143+
template: `<input [formField]="f.x" (input)="onInput()" />`,
144+
})
145+
class TestCmp {
146+
readonly model = signal({x: 'a'});
147+
readonly f = form(this.model);
148+
observedDuringInput: string | undefined;
149+
150+
onInput() {
151+
this.observedDuringInput = this.f.x().value();
152+
}
153+
}
154+
const fixture = act(() => TestBed.createComponent(TestCmp));
155+
const component = fixture.componentInstance;
156+
const input = fixture.nativeElement.firstChild as HTMLInputElement;
157+
158+
act(() => {
159+
input.value = 'b';
160+
input.dispatchEvent(new Event('input'));
161+
});
162+
expect(component.observedDuringInput).toBe('b');
163+
expect(component.model()).toEqual({x: 'b'});
164+
});
139165
});
140166

141167
describe('host directive mapping', () => {

packages/forms/signals/test/web/reactive_fvc.spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,30 @@ describe('FormControlDirective with FVC', () => {
7171
expect(fixture.componentInstance.ctrl.value).toBe('from-fvc');
7272
});
7373

74+
it('should update FormControl value before template (valueChange) listener fires', () => {
75+
@Component({
76+
template: `<my-fvc-input [formControl]="ctrl" (valueChange)="onValueChange()" />`,
77+
imports: [MyFvcInput, ReactiveFormsModule],
78+
})
79+
class TestCmp {
80+
ctrl = new FormControl('initial');
81+
observedDuringValueChange: string | null | undefined;
82+
83+
onValueChange() {
84+
this.observedDuringValueChange = this.ctrl.value;
85+
}
86+
}
87+
88+
const fixture = act(() => TestBed.createComponent(TestCmp));
89+
const component = fixture.componentInstance;
90+
const fvc = fixture.debugElement.query(By.directive(MyFvcInput)).componentInstance;
91+
92+
act(() => fvc.value.set('from-fvc'));
93+
94+
expect(component.observedDuringValueChange).toBe('from-fvc');
95+
expect(component.ctrl.value).toBe('from-fvc');
96+
});
97+
7498
it('should fall back to CVA when no FVC pattern is present', () => {
7599
@Component({
76100
template: `<input [formControl]="ctrl" />`,
@@ -553,6 +577,36 @@ describe('FormControlName with FVC', () => {
553577
expect(fixture.componentInstance.form.controls.name.value).toBe('from-fvc');
554578
});
555579

580+
it('should update FormControl value before template (valueChange) listener fires', () => {
581+
@Component({
582+
template: `
583+
<form [formGroup]="form">
584+
<my-fvc-input formControlName="name" (valueChange)="onValueChange()" />
585+
</form>
586+
`,
587+
imports: [MyFvcInput, ReactiveFormsModule],
588+
})
589+
class TestCmp {
590+
form = new FormGroup({
591+
name: new FormControl('initial'),
592+
});
593+
observedDuringValueChange: string | null | undefined;
594+
595+
onValueChange() {
596+
this.observedDuringValueChange = this.form.controls.name.value;
597+
}
598+
}
599+
600+
const fixture = act(() => TestBed.createComponent(TestCmp));
601+
const component = fixture.componentInstance;
602+
const fvc = fixture.debugElement.query(By.directive(MyFvcInput)).componentInstance;
603+
604+
act(() => fvc.value.set('from-fvc'));
605+
606+
expect(component.observedDuringValueChange).toBe('from-fvc');
607+
expect(component.form.controls.name.value).toBe('from-fvc');
608+
});
609+
556610
it('should fall back to CVA when no FVC pattern is present', () => {
557611
@Component({
558612
template: `

packages/forms/signals/test/web/template_fvc.spec.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,35 @@ describe('NgModel with FVC', () => {
7373
expect(fixture.componentInstance.val()).toBe('from-fvc');
7474
});
7575

76+
it('should update ngModel value before template (valueChange) listener fires', async () => {
77+
@Component({
78+
template: `<template-fvc-input
79+
[(ngModel)]="val"
80+
(valueChange)="onValueChange()"
81+
#model="ngModel"
82+
/>`,
83+
imports: [TemplateFvcInput, FormsModule],
84+
})
85+
class TestCmp {
86+
val = signal('initial');
87+
observedDuringValueChange: string | undefined;
88+
@ViewChild('model') model!: NgModel;
89+
90+
onValueChange() {
91+
this.observedDuringValueChange = this.model.control.value;
92+
}
93+
}
94+
95+
const fixture = await actAsync(() => TestBed.createComponent(TestCmp));
96+
const component = fixture.componentInstance;
97+
const fvc = fixture.debugElement.query(By.directive(TemplateFvcInput)).componentInstance;
98+
99+
act(() => fvc.value.set('from-fvc'));
100+
101+
expect(component.observedDuringValueChange).toBe('from-fvc');
102+
expect(component.val()).toBe('from-fvc');
103+
});
104+
76105
it('should fall back to CVA when no FVC pattern is present', async () => {
77106
@Component({
78107
template: `<input [(ngModel)]="val" />`,

0 commit comments

Comments
 (0)