Skip to content

Commit 0329c13

Browse files
crisbetopkozlowski-opensource
authored andcommitted
fix(forms): don't mutate validators array (angular#47830)
Fixes that the `AbstractControl` was mutating the validators arrays being passed into the constructor an helper methods like `setValidators`. Fixes angular#47827. PR Close angular#47830
1 parent b4c4a95 commit 0329c13

File tree

4 files changed

+54
-24
lines changed

4 files changed

+54
-24
lines changed

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -632,12 +632,6 @@
632632
{
633633
"name": "cleanUpView"
634634
},
635-
{
636-
"name": "coerceToAsyncValidator"
637-
},
638-
{
639-
"name": "coerceToValidator"
640-
},
641635
{
642636
"name": "collectNativeNodes"
643637
},

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -608,15 +608,9 @@
608608
{
609609
"name": "cleanUpView"
610610
},
611-
{
612-
"name": "coerceToAsyncValidator"
613-
},
614611
{
615612
"name": "coerceToBoolean"
616613
},
617-
{
618-
"name": "coerceToValidator"
619-
},
620614
{
621615
"name": "collectNativeNodes"
622616
},

packages/forms/src/model/abstract_model.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -372,15 +372,15 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
372372
*
373373
* @internal
374374
*/
375-
private _composedValidatorFn: ValidatorFn|null;
375+
private _composedValidatorFn!: ValidatorFn|null;
376376

377377
/**
378378
* Contains the result of merging asynchronous validators into a single validator function
379379
* (combined using `Validators.composeAsync`).
380380
*
381381
* @internal
382382
*/
383-
private _composedAsyncValidatorFn: AsyncValidatorFn|null;
383+
private _composedAsyncValidatorFn!: AsyncValidatorFn|null;
384384

385385
/**
386386
* Synchronous validators as they were provided:
@@ -390,7 +390,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
390390
*
391391
* @internal
392392
*/
393-
private _rawValidators: ValidatorFn|ValidatorFn[]|null;
393+
private _rawValidators!: ValidatorFn|ValidatorFn[]|null;
394394

395395
/**
396396
* Asynchronous validators as they were provided:
@@ -401,7 +401,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
401401
*
402402
* @internal
403403
*/
404-
private _rawAsyncValidators: AsyncValidatorFn|AsyncValidatorFn[]|null;
404+
private _rawAsyncValidators!: AsyncValidatorFn|AsyncValidatorFn[]|null;
405405

406406
/**
407407
* The current value of the control.
@@ -427,10 +427,8 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
427427
constructor(
428428
validators: ValidatorFn|ValidatorFn[]|null,
429429
asyncValidators: AsyncValidatorFn|AsyncValidatorFn[]|null) {
430-
this._rawValidators = validators;
431-
this._rawAsyncValidators = asyncValidators;
432-
this._composedValidatorFn = coerceToValidator(this._rawValidators);
433-
this._composedAsyncValidatorFn = coerceToAsyncValidator(this._rawAsyncValidators);
430+
this.setValidators(validators);
431+
this.setAsyncValidators(asyncValidators);
434432
}
435433

436434
/**
@@ -620,8 +618,8 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
620618
* using `addValidators()` method instead.
621619
*/
622620
setValidators(validators: ValidatorFn|ValidatorFn[]|null): void {
623-
this._rawValidators = validators;
624-
this._composedValidatorFn = coerceToValidator(validators);
621+
this._rawValidators = Array.isArray(validators) ? validators.slice() : validators;
622+
this._composedValidatorFn = coerceToValidator(this._rawValidators);
625623
}
626624

627625
/**
@@ -635,8 +633,8 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
635633
* using `addAsyncValidators()` method instead.
636634
*/
637635
setAsyncValidators(validators: AsyncValidatorFn|AsyncValidatorFn[]|null): void {
638-
this._rawAsyncValidators = validators;
639-
this._composedAsyncValidatorFn = coerceToAsyncValidator(validators);
636+
this._rawAsyncValidators = Array.isArray(validators) ? validators.slice() : validators;
637+
this._composedAsyncValidatorFn = coerceToAsyncValidator(this._rawAsyncValidators);
640638
}
641639

642640
/**

packages/forms/test/form_control_spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,16 @@ describe('FormControl', () => {
266266
expect(c.valid).toEqual(true);
267267
});
268268

269+
it('should not mutate the validators array when overriding using setValidators', () => {
270+
const control = new FormControl('');
271+
const originalValidators = [Validators.required];
272+
273+
control.setValidators(originalValidators);
274+
control.addValidators(Validators.minLength(10));
275+
276+
expect(originalValidators.length).toBe(1);
277+
});
278+
269279
it('should override validators by setting `control.validator` field value', () => {
270280
const c = new FormControl('');
271281
expect(c.valid).toEqual(true);
@@ -357,6 +367,30 @@ describe('FormControl', () => {
357367
expect(c.hasValidator(Validators.required)).toEqual(false);
358368
});
359369

370+
it('should not mutate the validators array when adding/removing sync validators', () => {
371+
const originalValidators = [Validators.required];
372+
const control = new FormControl('', originalValidators);
373+
374+
control.addValidators(Validators.min(10));
375+
expect(originalValidators.length).toBe(1);
376+
377+
control.removeValidators(Validators.required);
378+
expect(originalValidators.length).toBe(1);
379+
});
380+
381+
it('should not mutate the validators array when adding/removing async validators', () => {
382+
const firstValidator = asyncValidator('one');
383+
const secondValidator = asyncValidator('two');
384+
const originalValidators = [firstValidator];
385+
const control = new FormControl('', null, originalValidators);
386+
387+
control.addAsyncValidators(secondValidator);
388+
expect(originalValidators.length).toBe(1);
389+
390+
control.removeAsyncValidators(firstValidator);
391+
expect(originalValidators.length).toBe(1);
392+
});
393+
360394
it('should return false when checking presence of a validator not identical by reference',
361395
() => {
362396
const minValidator = Validators.min(5);
@@ -518,6 +552,16 @@ describe('FormControl', () => {
518552
expect(c.valid).toEqual(true);
519553
}));
520554

555+
it('should not mutate the validators array when overriding using setValidators', () => {
556+
const control = new FormControl('');
557+
const originalValidators = [asyncValidator('one')];
558+
559+
control.setAsyncValidators(originalValidators);
560+
control.addAsyncValidators(asyncValidator('two'));
561+
562+
expect(originalValidators.length).toBe(1);
563+
});
564+
521565
it('should override validators by setting `control.asyncValidator` field value',
522566
fakeAsync(() => {
523567
const c = new FormControl('');

0 commit comments

Comments
 (0)