From 65fa5b5439b9489dfa5f8fdfa24567cd9a32fac4 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 8 Jan 2026 16:49:43 +0100 Subject: [PATCH] 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 #66402 --- .../control_bindings/GOLDEN_PARTIAL.js | 71 +++++++++++++++++++ .../control_bindings/TEST_CASES.json | 9 +++ .../control_bindings/radio_bindings.js | 27 +++++++ .../control_bindings/radio_bindings.ts | 30 ++++++++ .../template/pipeline/src/phases/ordering.ts | 2 + .../test/web/form_field_directive.spec.ts | 57 +++++++++++++++ 6 files changed, 196 insertions(+) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/GOLDEN_PARTIAL.js index 16a0b402bf3..b05eb685153 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/GOLDEN_PARTIAL.js @@ -48,3 +48,74 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: radio_bindings.js + ****************************************************************************************************/ +import { Component, Directive, input } from '@angular/core'; +import * as i0 from "@angular/core"; +export class FormField { + formField = input(...(ngDevMode ? [undefined, { debugName: "formField" }] : [])); + static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FormField, deps: [], target: i0.ɵɵFactoryTarget.Directive }); + 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 }); +} +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FormField, decorators: [{ + type: Directive, + args: [{ selector: '[formField]' }] + }], propDecorators: { formField: [{ type: i0.Input, args: [{ isSignal: true, alias: "formField", required: false }] }] } }); +// Notice that we check that the binding order doesn't matter +export class MyComponent { + value = 'foo'; + static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); + static ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "ng-component", ngImport: i0, template: ` + + + + `, isInline: true, dependencies: [{ kind: "directive", type: FormField, selector: "[formField]", inputs: ["formField"] }] }); +} +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + template: ` + + + + `, + imports: [FormField], + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: radio_bindings.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class FormField { + readonly formField: import("@angular/core").InputSignal; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class MyComponent { + value: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/TEST_CASES.json index 00ecec89cfc..7110cca5b9b 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/TEST_CASES.json @@ -9,6 +9,15 @@ "files": ["control_bindings.js"] } ] + }, + { + "description": "should generate control instruction for `field` property bindings on radio inputs and execute it after the value", + "inputFiles": ["radio_bindings.ts"], + "expectations": [ + { + "files": ["radio_bindings.js"] + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.js new file mode 100644 index 00000000000..6d610d17aad --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.js @@ -0,0 +1,27 @@ +export class MyComponent { + // ... + static ɵcmp = /* @__PURE__ */i0.ɵɵdefineComponent({ + type: MyComponent, + selectors: [["ng-component"]], + decls: 2, + vars: 6, + consts: [["type", "radio", "id", "radio", 3, "formField", "value"], ["type", "radio", "id", "radio", 3, "value", "formField"]], + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵelement(0, "input", 0); + i0.ɵɵcontrolCreate(); + i0.ɵɵelement(1, "input", 1); + i0.ɵɵcontrolCreate(); + } + if (rf & 2) { + i0.ɵɵproperty("value", "foo"); + i0.ɵɵcontrol(ctx.value, "formField"); + i0.ɵɵadvance(); + i0.ɵɵproperty("value", "foo"); + i0.ɵɵcontrol(ctx.value, "formField"); + } + }, + dependencies: [FormField], + encapsulation: 2 + }); +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.ts new file mode 100644 index 00000000000..31e75696198 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/radio_bindings.ts @@ -0,0 +1,30 @@ +import { Component, Directive, input } from '@angular/core'; + +@Directive({selector: '[formField]'}) +export class FormField { + readonly formField = input(); +} + +// Notice that we check that the binding order doesn't matter +@Component({ + template: ` + + + + `, + imports: [FormField], +}) +export class MyComponent { + value = 'foo'; + +} diff --git a/packages/compiler/src/template/pipeline/src/phases/ordering.ts b/packages/compiler/src/template/pipeline/src/phases/ordering.ts index 5a16d20c19c..ec034235ceb 100644 --- a/packages/compiler/src/template/pipeline/src/phases/ordering.ts +++ b/packages/compiler/src/template/pipeline/src/phases/ordering.ts @@ -66,6 +66,7 @@ const UPDATE_ORDERING: Array> = [ {test: kindWithInterpolationTest(ir.OpKind.Property, true)}, {test: nonInterpolationPropertyKindTest}, {test: kindWithInterpolationTest(ir.OpKind.Attribute, false)}, + {test: kindTest(ir.OpKind.Control)}, ]; /** @@ -97,6 +98,7 @@ const handledOpKinds = new Set([ ir.OpKind.DomProperty, ir.OpKind.Attribute, ir.OpKind.Animation, + ir.OpKind.Control, ]); /** diff --git a/packages/forms/signals/test/web/form_field_directive.spec.ts b/packages/forms/signals/test/web/form_field_directive.spec.ts index 8ba8fe29e41..e245dca2217 100644 --- a/packages/forms/signals/test/web/form_field_directive.spec.ts +++ b/packages/forms/signals/test/web/form_field_directive.spec.ts @@ -2667,6 +2667,30 @@ describe('field directive', () => { expect(cmp.f().value()).toBe('b'); }); + it('synchronizes with a radio group with bindings', () => { + const {cmp, inputA, inputB, inputC, ABC} = setupRadioWithBindingsGroup(); + + // All the inputs should have the same name. + expect(inputA.name).toBe('test'); + expect(inputB.name).toBe('test'); + expect(inputC.name).toBe('test'); + + // Model -> View + act(() => cmp.f().value.set(ABC.C)); + expect(inputA.checked).toBe(false); + expect(inputB.checked).toBe(false); + expect(inputC.checked).toBe(true); + + // View -> Model + act(() => { + inputB.click(); + }); + expect(inputA.checked).toBe(false); + expect(inputB.checked).toBe(true); + expect(inputC.checked).toBe(false); + expect(cmp.f().value()).toBe(ABC.B); + }); + it('synchronizes with a textarea', () => { @Component({ imports: [FormField], @@ -4321,6 +4345,39 @@ function setupRadioGroup() { return {cmp, inputA, inputB, inputC}; } +function setupRadioWithBindingsGroup() { + enum ABC { + A = 'a', + B = 'b', + C = 'c', + } + @Component({ + imports: [FormField], + template: ` +
+ + + +
+ `, + }) + class TestCmp { + f = form(signal(ABC.A), { + name: 'test', + }); + ABC = ABC; + } + + const fix = act(() => TestBed.createComponent(TestCmp)); + const formEl = (fix.nativeElement as HTMLElement).firstChild as HTMLFormElement; + const inputs = Array.from(formEl.children) as HTMLInputElement[]; + + const [inputA, inputB, inputC] = inputs; + const cmp = fix.componentInstance as TestCmp; + + return {cmp, inputA, inputB, inputC, ABC}; +} + function act(fn: () => T): T { try { return fn();