diff --git a/packages/forms/src/model/form_array.ts b/packages/forms/src/model/form_array.ts index 1b3a6e3523a1..4363dd2f3dc5 100644 --- a/packages/forms/src/model/form_array.ts +++ b/packages/forms/src/model/form_array.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {ɵWritable as Writable} from '@angular/core'; +import {untracked, ɵWritable as Writable} from '@angular/core'; import {AsyncValidatorFn, ValidatorFn} from '../directives/validators'; @@ -321,12 +321,14 @@ export class FormArray = any> extends Abst emitEvent?: boolean; } = {}, ): void { - assertAllValuesPresent(this, false, value); - value.forEach((newValue: any, index: number) => { - assertControlPresent(this, false, index); - this.at(index).setValue(newValue, {onlySelf: true, emitEvent: options.emitEvent}); + untracked(() => { + assertAllValuesPresent(this, false, value); + value.forEach((newValue: any, index: number) => { + assertControlPresent(this, false, index); + this.at(index).setValue(newValue, {onlySelf: true, emitEvent: options.emitEvent}); + }); + this.updateValueAndValidity(options); }); - this.updateValueAndValidity(options); } /** diff --git a/packages/forms/src/model/form_control.ts b/packages/forms/src/model/form_control.ts index 13ea67343970..dd881a1d9969 100644 --- a/packages/forms/src/model/form_control.ts +++ b/packages/forms/src/model/form_control.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {ɵWritable as Writable} from '@angular/core'; +import {untracked, ɵWritable as Writable} from '@angular/core'; import {AsyncValidatorFn, ValidatorFn} from '../directives/validators'; import {removeListItem} from '../util'; @@ -505,13 +505,15 @@ export const FormControl: ɵFormControlCtor = class FormControl emitViewToModelChange?: boolean; } = {}, ): void { - (this as Writable).value = this._pendingValue = value; - if (this._onChange.length && options.emitModelToViewChange !== false) { - this._onChange.forEach((changeFn) => - changeFn(this.value, options.emitViewToModelChange !== false), - ); - } - this.updateValueAndValidity(options); + untracked(() => { + (this as Writable).value = this._pendingValue = value; + if (this._onChange.length && options.emitModelToViewChange !== false) { + this._onChange.forEach((changeFn) => + changeFn(this.value, options.emitViewToModelChange !== false), + ); + } + this.updateValueAndValidity(options); + }); } override patchValue( diff --git a/packages/forms/src/model/form_group.ts b/packages/forms/src/model/form_group.ts index 8ab7341ef9b1..6868152c9b61 100644 --- a/packages/forms/src/model/form_group.ts +++ b/packages/forms/src/model/form_group.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {ɵWritable as Writable} from '@angular/core'; +import {untracked, ɵWritable as Writable} from '@angular/core'; import {AsyncValidatorFn, ValidatorFn} from '../directives/validators'; @@ -430,15 +430,17 @@ export class FormGroup< emitEvent?: boolean; } = {}, ): void { - assertAllValuesPresent(this, true, value); - (Object.keys(value) as Array).forEach((name) => { - assertControlPresent(this, true, name as any); - (this.controls as any)[name].setValue((value as any)[name], { - onlySelf: true, - emitEvent: options.emitEvent, + untracked(() => { + assertAllValuesPresent(this, true, value); + (Object.keys(value) as Array).forEach((name) => { + assertControlPresent(this, true, name as any); + (this.controls as any)[name].setValue((value as any)[name], { + onlySelf: true, + emitEvent: options.emitEvent, + }); }); + this.updateValueAndValidity(options); }); - this.updateValueAndValidity(options); } /** diff --git a/packages/forms/test/form_array_spec.ts b/packages/forms/test/form_array_spec.ts index 8cc1062aff3b..d6dba976be69 100644 --- a/packages/forms/test/form_array_spec.ts +++ b/packages/forms/test/form_array_spec.ts @@ -6,18 +6,23 @@ * found in the LICENSE file at https://angular.dev/license */ +import {Component, Directive, effect, forwardRef, signal} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import {of} from 'rxjs'; import { AbstractControl, + ControlValueAccessor, FormArray, FormControl, FormGroup, + NG_VALUE_ACCESSOR, + ReactiveFormsModule, ValidationErrors, ValidatorFn, } from '../index'; import {Validators} from '../src/validators'; -import {of} from 'rxjs'; -import {useAutoTick, timeout} from '@angular/private/testing'; +import {timeout, useAutoTick} from '@angular/private/testing'; import {asyncValidator} from './util'; (function () { @@ -1626,5 +1631,132 @@ import {asyncValidator} from './util'; }); }); }); + + describe('FormArray.setValue is untracked', () => { + @Directive({ + selector: '[testCva]', + providers: [ + { + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => TestCvaDirective), + multi: true, + }, + ], + }) + class TestCvaDirective implements ControlValueAccessor { + // This is the “dangerous” signal read that must NOT get tracked by the caller of setValue(). + static cvaSignal = signal(0); + + writeValue(value: unknown): void { + // If setValue() is not untracked, the *caller* effect/computed may accidentally track this read. + TestCvaDirective.cvaSignal(); + } + + registerOnChange(_: (value: unknown) => void): void {} + registerOnTouched(_: () => void): void {} + setDisabledState(_: boolean): void {} + } + + @Component({ + imports: [ReactiveFormsModule, TestCvaDirective], + template: ``, + }) + class HostComponent { + control = new FormArray([new FormControl('')]); + } + + it('should NOT track signals read inside CVA.writeValue when setValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.setValue([driver()]); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when patchValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.patchValue([driver()]); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when reset is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.reset([driver()]); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + }); }); })(); diff --git a/packages/forms/test/form_control_spec.ts b/packages/forms/test/form_control_spec.ts index 5f23171e6ce8..7b4729d260ce 100644 --- a/packages/forms/test/form_control_spec.ts +++ b/packages/forms/test/form_control_spec.ts @@ -6,10 +6,21 @@ * found in the LICENSE file at https://angular.dev/license */ -import {AsyncValidatorFn, FormArray, FormControl, FormGroup, Validators} from '../index'; - +import {Component, Directive, effect, forwardRef, signal} from '@angular/core'; +import { + AsyncValidatorFn, + ControlValueAccessor, + FormArray, + FormControl, + FormGroup, + NG_VALUE_ACCESSOR, + ReactiveFormsModule, + Validators, +} from '../index'; + +import {TestBed} from '@angular/core/testing'; +import {timeout, useAutoTick} from '@angular/private/testing'; import {asyncValidator, asyncValidatorReturningObservable} from './util'; -import {useAutoTick, timeout} from '@angular/private/testing'; (function () { function otherAsyncValidator() { @@ -1656,5 +1667,138 @@ import {useAutoTick, timeout} from '@angular/private/testing'; expect(FormControl.name).toBe('FormControl'); }); }); + + describe('FormControl.setValue is untracked', () => { + @Directive({ + selector: '[testCva]', + providers: [ + { + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => TestCvaDirective), + multi: true, + }, + ], + }) + class TestCvaDirective implements ControlValueAccessor { + // This is the “dangerous” signal read that must NOT get tracked by the caller of setValue(). + static cvaSignal = signal(0); + + // Optional: helps debugging / sanity checks + lastReadInWriteValue: number | null = null; + lastWrittenValue: unknown = null; + + writeValue(value: unknown): void { + // If setValue() is not untracked, the *caller* effect/computed may accidentally track this read. + this.lastReadInWriteValue = TestCvaDirective.cvaSignal(); + this.lastWrittenValue = value; + } + + registerOnChange(_: (value: unknown) => void): void {} + registerOnTouched(_: () => void): void {} + setDisabledState(_: boolean): void {} + } + + @Component({ + imports: [ReactiveFormsModule, TestCvaDirective], + template: ``, + }) + class HostComponent { + control = new FormControl(''); + } + + it('should NOT track signals read inside CVA.writeValue when setValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + // setValue will invoke CVA.writeValue which reads TestCvaDirective.cvaSignal(). + fixture.componentInstance.control.setValue(driver()); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when patchValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.patchValue(driver()); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when reset is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.reset(driver()); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + }); }); })(); diff --git a/packages/forms/test/form_group_spec.ts b/packages/forms/test/form_group_spec.ts index 41703f15cb40..1672ca8cb93e 100644 --- a/packages/forms/test/form_group_spec.ts +++ b/packages/forms/test/form_group_spec.ts @@ -6,18 +6,23 @@ * found in the LICENSE file at https://angular.dev/license */ +import {Component, Directive, effect, forwardRef, signal} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import {timeout, useAutoTick} from '@angular/private/testing'; import {filter, map, of} from 'rxjs'; import { AbstractControl, ControlEvent, + ControlValueAccessor, FormArray, FormControl, FormGroup, + NG_VALUE_ACCESSOR, + ReactiveFormsModule, ValidationErrors, Validators, ValueChangeEvent, } from '../index'; -import {useAutoTick, timeout} from '@angular/private/testing'; import {FormControlStatus, StatusChangeEvent} from '../src/model/abstract_model'; import { @@ -2718,5 +2723,132 @@ import { }); expect(consoleWarnSpy).toHaveBeenCalledTimes(1); }); + + describe('FormGroup.setValue is untracked', () => { + @Directive({ + selector: '[testCva]', + providers: [ + { + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => TestCvaDirective), + multi: true, + }, + ], + }) + class TestCvaDirective implements ControlValueAccessor { + // This is the “dangerous” signal read that must NOT get tracked by the caller of setValue(). + static cvaSignal = signal(0); + + writeValue(value: unknown): void { + // If setValue() is not untracked, the *caller* effect/computed may accidentally track this read. + TestCvaDirective.cvaSignal(); + } + + registerOnChange(_: (value: unknown) => void): void {} + registerOnTouched(_: () => void): void {} + setDisabledState(_: boolean): void {} + } + + @Component({ + imports: [ReactiveFormsModule, TestCvaDirective], + template: ``, + }) + class HostComponent { + control = new FormGroup({one: new FormControl('')}); + } + + it('should NOT track signals read inside CVA.writeValue when setValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.setValue({one: driver()}); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when patchValue is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.patchValue({one: driver()}); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + + it('should NOT track signals read inside CVA.writeValue when reset is called inside an effect', async () => { + const fixture = TestBed.createComponent(HostComponent); + fixture.detectChanges(); // wires up FormControlDirective + CVA + + const driver = signal('A'); + let runs = 0; + + // Create the effect inside the Angular injection context. + TestBed.runInInjectionContext(() => { + effect(() => { + runs++; + + // Only dependency we *want* is `driver()`. + fixture.componentInstance.control.reset({one: driver()}); + }); + }); + + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the CVA signal should NOT re-run the effect. + TestCvaDirective.cvaSignal.set(1); + await fixture.whenStable(); + expect(runs).toBe(1); + + // Changing the driver signal SHOULD re-run the effect. + driver.set('B'); + await fixture.whenStable(); + expect(runs).toBe(2); + }); + }); }); })();