Skip to content

Commit b93c6a4

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). The ControlCreate runtime instruction reads the current TNode via getCurrentTNode(), so it must run while the target element's TNode is still active - i.e. between its ElementStart and ElementEnd. control_directives.ts is changed to guarantee this: - CONTROL_OP_CREATE_KINDS no longer includes ElementEnd, ContainerEnd or Template. ElementEnd/ContainerEnd would let the op attach after the closing tag (parent TNode), and Template restores the parent TNode as well, so none of them are valid insertion targets. - findCreateInstruction now returns the *first* matching create op for a given target xref (ElementStart or Element) instead of the last one, so ControlCreate is always inserted immediately after the opening tag. empty_elements.ts is intentionally left unchanged: ControlCreate is NOT added to IGNORED_OP_KINDS. It must *block* the collapse of an empty ElementStart/ElementEnd pair, otherwise the op would be re-parented to the collapsed element and lose the correct TNode. As a result, an element carrying a control binding keeps its explicit elementStart/elementEnd pair instead of collapsing to a single element() instruction. 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 5ad8231 commit b93c6a4

8 files changed

Lines changed: 39 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', () => {

0 commit comments

Comments
 (0)