From 2e9aeea0fed1a2eae261b95cb1479519d0428b83 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 25 Mar 2026 13:31:14 -0700 Subject: [PATCH] fix(forms): deduplicate writeValue calls in CVA interop Update bindings['controlValue'] during onChange to track the view value. This allows bindingUpdated to skip writeValue if the model value matches the last seen view value, preventing redundant writes. Fixes #67847 --- .../signals/src/directive/control_cva.ts | 14 +++++++--- .../forms/signals/test/web/interop.spec.ts | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/forms/signals/src/directive/control_cva.ts b/packages/forms/signals/src/directive/control_cva.ts index 1103ff2fd2d..13ecdb970fc 100644 --- a/packages/forms/signals/src/directive/control_cva.ts +++ b/packages/forms/signals/src/directive/control_cva.ts @@ -21,16 +21,22 @@ export function cvaControlCreate( host: ControlDirectiveHost, parent: FormField, ): () => void { - parent.controlValueAccessor!.registerOnChange((value: unknown) => - parent.state().controlValue.set(value as any), - ); + const bindings = createBindings(); + + parent.controlValueAccessor!.registerOnChange((value: unknown) => { + // Update tracking for 'controlValue' here so that when the effect runs, + // `bindingUpdated` sees that the model value matches the last seen view value. + // This prevents the framework from writing the same value back to the CVA (CVA loopback). + bindings['controlValue'] = value; + parent.state().controlValue.set(value as any); + }); parent.controlValueAccessor!.registerOnTouched(() => parent.state().markAsTouched()); parent.registerAsBinding(); - const bindings = createBindings(); return () => { const fieldState = parent.state(); const value = fieldState.value(); + if (bindingUpdated(bindings, 'controlValue', value)) { // We don't know if the interop control has underlying signals, so we must use `untracked` to // prevent writing to a signal in a reactive context. diff --git a/packages/forms/signals/test/web/interop.spec.ts b/packages/forms/signals/test/web/interop.spec.ts index 71359840b1c..169a5e189f6 100644 --- a/packages/forms/signals/test/web/interop.spec.ts +++ b/packages/forms/signals/test/web/interop.spec.ts @@ -61,6 +61,7 @@ describe('ControlValueAccessor', () => { }) class CustomControl implements ControlValueAccessor { value = ''; + writeCount = 0; disabled = false; private onChangeFn?: (value: string) => void; @@ -68,6 +69,7 @@ describe('ControlValueAccessor', () => { writeValue(newValue: string): void { this.value = newValue; + this.writeCount++; } registerOnChange(fn: (value: string) => void): void { @@ -132,6 +134,31 @@ describe('ControlValueAccessor', () => { expect(fixture.componentInstance.f().value()).toBe('typing'); }); + it('should not write back to CVA on view change', () => { + @Component({ + imports: [CustomControl, FormField], + template: ``, + }) + class TestCmp { + readonly f = form(signal('test')); + readonly control = viewChild.required(CustomControl); + } + + const fixture = act(() => TestBed.createComponent(TestCmp)); + const control = fixture.componentInstance.control(); + const input = fixture.nativeElement.querySelector('input'); + + expect(control.writeCount).toBe(1); // Initial write + + act(() => { + input.value = 'typing'; + input.dispatchEvent(new Event('input')); + }); + + expect(fixture.componentInstance.f().value()).toBe('typing'); + expect(control.writeCount).toBe(1); // Should still be 1 (No write-back!) + }); + it('should support debounce', async () => { const {promise, resolve} = promiseWithResolvers();