Skip to content

Commit d91b507

Browse files
committed
fix(forms): place controlCreate before listeners in signal forms template creation block
Previously, the ControlCreate IR op was inserted after the target element's create op (ElementStart/Element), but because the ordering phase places listeners (Listener, TwoWayListener) immediately after element-start ops, the ControlCreate op ended up being emitted after the ElementEnd instruction. This caused a behaviour discrepancy between [formField] and [(ngModel)]: inside an (input) event handler, the field value reported by Signal Forms still held the old value while ngModel already reflected the new one. The ControlCreate runtime instruction sets up the two-way sync between the DOM control and the field signal, so it must be called early during element creation (before any listeners that may read the updated value). Two compiler changes fix this: 1. control_directives.ts - findCreateInstruction now returns the *first* matching create op for a given target xref (ElementStart or Element) instead of the last one. Previously ElementEnd was in the eligible set which meant the search returned the ElementEnd op, causing ControlCreate to land after it. Removing ElementEnd and ContainerEnd from CONTROL_OP_CREATE_KINDS ensures the op is always inserted immediately after the opening tag. 2. empty_elements.ts - ElementStart+ElementEnd pairs that contain only a ControlCreate op between them can still be collapsed into a single Element instruction. Added ir.OpKind.ControlCreate to IGNORED_OP_KINDS so the collapseEmptyInstructions phase skips over it when looking for a matching start op. Compliance test golden files for two-way binding templates updated to reflect the new (correct) instruction order: controlCreate now appears immediately after elementStart, before the twoWayListener. Fixes angular#68182 Related: angular#63608
1 parent 2896c93 commit d91b507

7 files changed

Lines changed: 33 additions & 11 deletions

File tree

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: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,8 @@ 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,
5048
ir.OpKind.Template,
5149
]);
5250

@@ -55,16 +53,14 @@ function isRelevantCreateOp(createOp: ir.CreateOp): createOp is ir.CreateOp & {x
5553
}
5654

5755
function findCreateInstruction(view: ViewCompilationUnit, target: ir.XrefId): ir.CreateOp | null {
58-
let lastFoundOp: ir.CreateOp | null = null;
5956
for (const createOp of view.create) {
6057
if (!isRelevantCreateOp(createOp) || createOp.xref !== target) {
6158
continue;
6259
}
63-
64-
lastFoundOp = createOp;
60+
return createOp;
6561
}
6662

67-
return lastFoundOp;
63+
return null;
6864
}
6965

7066
function addControlInstruction(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const REPLACEMENTS = new Map<ir.OpKind, [ir.OpKind, ir.OpKind]>([
1818
/**
1919
* Op kinds that should not prevent merging of start/end ops.
2020
*/
21-
const IGNORED_OP_KINDS = new Set([ir.OpKind.Pipe]);
21+
const IGNORED_OP_KINDS = new Set([ir.OpKind.Pipe, ir.OpKind.ControlCreate]);
2222

2323
/**
2424
* Replace sequences of mergable instructions (e.g. `ElementStart` and `ElementEnd`) with a

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

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

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

0 commit comments

Comments
 (0)