Skip to content

Commit 2e9aeea

Browse files
alxhubatscott
authored andcommitted
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 angular#67847
1 parent a0aa830 commit 2e9aeea

2 files changed

Lines changed: 37 additions & 4 deletions

File tree

packages/forms/signals/src/directive/control_cva.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,22 @@ export function cvaControlCreate(
2121
host: ControlDirectiveHost,
2222
parent: FormField<unknown>,
2323
): () => void {
24-
parent.controlValueAccessor!.registerOnChange((value: unknown) =>
25-
parent.state().controlValue.set(value as any),
26-
);
24+
const bindings = createBindings<ControlBindingKey | 'controlValue'>();
25+
26+
parent.controlValueAccessor!.registerOnChange((value: unknown) => {
27+
// Update tracking for 'controlValue' here so that when the effect runs,
28+
// `bindingUpdated` sees that the model value matches the last seen view value.
29+
// This prevents the framework from writing the same value back to the CVA (CVA loopback).
30+
bindings['controlValue'] = value;
31+
parent.state().controlValue.set(value as any);
32+
});
2733
parent.controlValueAccessor!.registerOnTouched(() => parent.state().markAsTouched());
2834
parent.registerAsBinding();
2935

30-
const bindings = createBindings<ControlBindingKey | 'controlValue'>();
3136
return () => {
3237
const fieldState = parent.state();
3338
const value = fieldState.value();
39+
3440
if (bindingUpdated(bindings, 'controlValue', value)) {
3541
// We don't know if the interop control has underlying signals, so we must use `untracked` to
3642
// prevent writing to a signal in a reactive context.

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,15 @@ describe('ControlValueAccessor', () => {
6161
})
6262
class CustomControl implements ControlValueAccessor {
6363
value = '';
64+
writeCount = 0;
6465
disabled = false;
6566

6667
private onChangeFn?: (value: string) => void;
6768
private onTouchedFn?: () => void;
6869

6970
writeValue(newValue: string): void {
7071
this.value = newValue;
72+
this.writeCount++;
7173
}
7274

7375
registerOnChange(fn: (value: string) => void): void {
@@ -132,6 +134,31 @@ describe('ControlValueAccessor', () => {
132134
expect(fixture.componentInstance.f().value()).toBe('typing');
133135
});
134136

137+
it('should not write back to CVA on view change', () => {
138+
@Component({
139+
imports: [CustomControl, FormField],
140+
template: `<custom-control [formField]="f" />`,
141+
})
142+
class TestCmp {
143+
readonly f = form(signal('test'));
144+
readonly control = viewChild.required(CustomControl);
145+
}
146+
147+
const fixture = act(() => TestBed.createComponent(TestCmp));
148+
const control = fixture.componentInstance.control();
149+
const input = fixture.nativeElement.querySelector('input');
150+
151+
expect(control.writeCount).toBe(1); // Initial write
152+
153+
act(() => {
154+
input.value = 'typing';
155+
input.dispatchEvent(new Event('input'));
156+
});
157+
158+
expect(fixture.componentInstance.f().value()).toBe('typing');
159+
expect(control.writeCount).toBe(1); // Should still be 1 (No write-back!)
160+
});
161+
135162
it('should support debounce', async () => {
136163
const {promise, resolve} = promiseWithResolvers<void>();
137164

0 commit comments

Comments
 (0)