Skip to content

Commit 65fa5b5

Browse files
JeanMechethePunderWoman
authored andcommitted
fix(forms): Ensure the control instruction comes after the other bindings
Prior to this change, binding to radio value was sensitive to the order in which `value` & `formField` where binding in the template. The compiler change makes that order non-important. fixes angular#66402
1 parent 7fd076b commit 65fa5b5

6 files changed

Lines changed: 196 additions & 0 deletions

File tree

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,74 @@ export declare class MyComponent {
4848
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "ng-component", never, {}, {}, never, never, true, never>;
4949
}
5050

51+
/****************************************************************************************************
52+
* PARTIAL FILE: radio_bindings.js
53+
****************************************************************************************************/
54+
import { Component, Directive, input } from '@angular/core';
55+
import * as i0 from "@angular/core";
56+
export class FormField {
57+
formField = input(...(ngDevMode ? [undefined, { debugName: "formField" }] : []));
58+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FormField, deps: [], target: i0.ɵɵFactoryTarget.Directive });
59+
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "17.1.0", version: "0.0.0-PLACEHOLDER", type: FormField, isStandalone: true, selector: "[formField]", inputs: { formField: { classPropertyName: "formField", publicName: "formField", isSignal: true, isRequired: false, transformFunction: null } }, ngImport: i0 });
60+
}
61+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FormField, decorators: [{
62+
type: Directive,
63+
args: [{ selector: '[formField]' }]
64+
}], propDecorators: { formField: [{ type: i0.Input, args: [{ isSignal: true, alias: "formField", required: false }] }] } });
65+
// Notice that we check that the binding order doesn't matter
66+
export class MyComponent {
67+
value = 'foo';
68+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
69+
static ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
70+
<input
71+
type="radio"
72+
[formField]="value"
73+
[value]="'foo'"
74+
id="radio"
75+
/>
76+
77+
<input
78+
type="radio"
79+
[value]="'foo'"
80+
[formField]="value"
81+
id="radio"
82+
/>
83+
`, isInline: true, dependencies: [{ kind: "directive", type: FormField, selector: "[formField]", inputs: ["formField"] }] });
84+
}
85+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
86+
type: Component,
87+
args: [{
88+
template: `
89+
<input
90+
type="radio"
91+
[formField]="value"
92+
[value]="'foo'"
93+
id="radio"
94+
/>
95+
96+
<input
97+
type="radio"
98+
[value]="'foo'"
99+
[formField]="value"
100+
id="radio"
101+
/>
102+
`,
103+
imports: [FormField],
104+
}]
105+
}] });
106+
107+
/****************************************************************************************************
108+
* PARTIAL FILE: radio_bindings.d.ts
109+
****************************************************************************************************/
110+
import * as i0 from "@angular/core";
111+
export declare class FormField {
112+
readonly formField: import("@angular/core").InputSignal<string | undefined>;
113+
static ɵfac: i0.ɵɵFactoryDeclaration<FormField, never>;
114+
static ɵdir: i0.ɵɵDirectiveDeclaration<FormField, "[formField]", never, { "formField": { "alias": "formField"; "required": false; "isSignal": true; }; }, {}, never, never, true, never>;
115+
}
116+
export declare class MyComponent {
117+
value: string;
118+
static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
119+
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "ng-component", never, {}, {}, never, never, true, never>;
120+
}
121+

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/TEST_CASES.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@
99
"files": ["control_bindings.js"]
1010
}
1111
]
12+
},
13+
{
14+
"description": "should generate control instruction for `field` property bindings on radio inputs and execute it after the value",
15+
"inputFiles": ["radio_bindings.ts"],
16+
"expectations": [
17+
{
18+
"files": ["radio_bindings.js"]
19+
}
20+
]
1221
}
1322
]
1423
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
export class MyComponent {
2+
// ...
3+
static ɵcmp = /* @__PURE__ */i0.ɵɵdefineComponent({
4+
type: MyComponent,
5+
selectors: [["ng-component"]],
6+
decls: 2,
7+
vars: 6,
8+
consts: [["type", "radio", "id", "radio", 3, "formField", "value"], ["type", "radio", "id", "radio", 3, "value", "formField"]],
9+
template: function MyComponent_Template(rf, ctx) {
10+
if (rf & 1) {
11+
i0.ɵɵelement(0, "input", 0);
12+
i0.ɵɵcontrolCreate();
13+
i0.ɵɵelement(1, "input", 1);
14+
i0.ɵɵcontrolCreate();
15+
}
16+
if (rf & 2) {
17+
i0.ɵɵproperty("value", "foo");
18+
i0.ɵɵcontrol(ctx.value, "formField");
19+
i0.ɵɵadvance();
20+
i0.ɵɵproperty("value", "foo");
21+
i0.ɵɵcontrol(ctx.value, "formField");
22+
}
23+
},
24+
dependencies: [FormField],
25+
encapsulation: 2
26+
});
27+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { Component, Directive, input } from '@angular/core';
2+
3+
@Directive({selector: '[formField]'})
4+
export class FormField {
5+
readonly formField = input<string>();
6+
}
7+
8+
// Notice that we check that the binding order doesn't matter
9+
@Component({
10+
template: `
11+
<input
12+
type="radio"
13+
[formField]="value"
14+
[value]="'foo'"
15+
id="radio"
16+
/>
17+
18+
<input
19+
type="radio"
20+
[value]="'foo'"
21+
[formField]="value"
22+
id="radio"
23+
/>
24+
`,
25+
imports: [FormField],
26+
})
27+
export class MyComponent {
28+
value = 'foo';
29+
30+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const UPDATE_ORDERING: Array<Rule<ir.UpdateOp>> = [
6666
{test: kindWithInterpolationTest(ir.OpKind.Property, true)},
6767
{test: nonInterpolationPropertyKindTest},
6868
{test: kindWithInterpolationTest(ir.OpKind.Attribute, false)},
69+
{test: kindTest(ir.OpKind.Control)},
6970
];
7071

7172
/**
@@ -97,6 +98,7 @@ const handledOpKinds = new Set([
9798
ir.OpKind.DomProperty,
9899
ir.OpKind.Attribute,
99100
ir.OpKind.Animation,
101+
ir.OpKind.Control,
100102
]);
101103

102104
/**

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,6 +2667,30 @@ describe('field directive', () => {
26672667
expect(cmp.f().value()).toBe('b');
26682668
});
26692669

2670+
it('synchronizes with a radio group with bindings', () => {
2671+
const {cmp, inputA, inputB, inputC, ABC} = setupRadioWithBindingsGroup();
2672+
2673+
// All the inputs should have the same name.
2674+
expect(inputA.name).toBe('test');
2675+
expect(inputB.name).toBe('test');
2676+
expect(inputC.name).toBe('test');
2677+
2678+
// Model -> View
2679+
act(() => cmp.f().value.set(ABC.C));
2680+
expect(inputA.checked).toBe(false);
2681+
expect(inputB.checked).toBe(false);
2682+
expect(inputC.checked).toBe(true);
2683+
2684+
// View -> Model
2685+
act(() => {
2686+
inputB.click();
2687+
});
2688+
expect(inputA.checked).toBe(false);
2689+
expect(inputB.checked).toBe(true);
2690+
expect(inputC.checked).toBe(false);
2691+
expect(cmp.f().value()).toBe(ABC.B);
2692+
});
2693+
26702694
it('synchronizes with a textarea', () => {
26712695
@Component({
26722696
imports: [FormField],
@@ -4321,6 +4345,39 @@ function setupRadioGroup() {
43214345
return {cmp, inputA, inputB, inputC};
43224346
}
43234347

4348+
function setupRadioWithBindingsGroup() {
4349+
enum ABC {
4350+
A = 'a',
4351+
B = 'b',
4352+
C = 'c',
4353+
}
4354+
@Component({
4355+
imports: [FormField],
4356+
template: `
4357+
<form>
4358+
<input type="radio" [formField]="f" [value]="ABC.A" />
4359+
<input type="radio" [formField]="f" [value]="ABC.B" />
4360+
<input type="radio" [formField]="f" [value]="ABC.C" />
4361+
</form>
4362+
`,
4363+
})
4364+
class TestCmp {
4365+
f = form(signal(ABC.A), {
4366+
name: 'test',
4367+
});
4368+
ABC = ABC;
4369+
}
4370+
4371+
const fix = act(() => TestBed.createComponent(TestCmp));
4372+
const formEl = (fix.nativeElement as HTMLElement).firstChild as HTMLFormElement;
4373+
const inputs = Array.from(formEl.children) as HTMLInputElement[];
4374+
4375+
const [inputA, inputB, inputC] = inputs;
4376+
const cmp = fix.componentInstance as TestCmp;
4377+
4378+
return {cmp, inputA, inputB, inputC, ABC};
4379+
}
4380+
43244381
function act<T>(fn: () => T): T {
43254382
try {
43264383
return fn();

0 commit comments

Comments
 (0)