AbstractControl constructed with an array of validators mutates the original array on control.addValidators()

[idpaterson]

Which @angular/* package(s) are the source of the bug?

forms

Is this a regression?

No

Description

The AbstractControl constructor accepts an array of validators but methods like addValidator modify that array. When the array of validators is meant to be shared between forms as in export const PHONE_VALIDATORS: ValidationFn[] = [...], validators dynamically added to individual controls make their way into that shared array.

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-ivy-ud7zdz?file=src/app/form1.component.ts

Please provide the exception or error you saw

Please follow the instructions in the StackBlitz demo. Two components each use the CONSTANT_VALIDATORS export, but form 1 also uses AbstractControl.addValidators to apply additional validation to its control. This third validator is unexpectedly pushed into CONSTANT_VALIDATORS so form 2 has 3 validation rules rather than the expected 2.

The expectation is that constructing an AbstractControl with an array of validators would create a shallow copy of the array that can be modified safely.

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 14.1.0
Node: 16.18.0
Package Manager: npm 8.19.2
OS: darwin x64

Angular: 14.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... google-maps, localize, platform-browser
... platform-browser-dynamic, platform-server, router
... youtube-player

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1401.0
@angular-devkit/build-angular   14.1.0
@angular-devkit/core            14.1.0
@angular-devkit/schematics      14.1.0
@schematics/angular             14.1.0
rxjs                            7.5.6
typescript                      4.7.4
webpack                         5.74.0

Anything else?

In our app a "required" validator is added or removed from controls depending on whether they are visible on the screen. This allows us to mark controls that are permission controlled as required via template driven markup such that they are not required for users without permission to view the control or in any other case where the control is not rendered.

We also use FormBuilder and many forms include controls that are shared from constant (or so we thought) FormBuilder configs. For example, fb.group({ name: null, age: null, ...UserProfileFormGroupConfig }) where individual controls within UserProfileFormGroupConfig may have an array of validators.

When these two things are combined we end up with the conditional required validator getting pushed into UserProfileFormGroupConfig control validator arrays and stuck there. Forms are marked invalid because of controls that are in the FormGroup but not on the screen because they inherited this required validator.

Our workaround was to refactor the form group config as a function that returns a new config every time to avoid constants. For cases where we were using a shared array of validators like in the StackBlitz example we made sure that the AbstractControl constructor received a spread copy rather than the original array.

[pkozlowski-opensource]

Re-opening this issue as the fix PR #47830 will be reverted via #47845

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}