feat(forms): AbstractControl to store raw validators in addition to combined validators function (#37881)

This commit refactors the way we store validators in AbstractControl-based classes:
in addition to the combined validators function that we have, we also store the original list of validators.
This is needed to have an ability to clean them up later at destroy time (currently it's problematic since
they are combined in a single function).

The change preserves backwards compatibility by making sure public APIs stay the same.
The only public API update is the change to the `AbstractControl` class constructor to extend the set
of possible types that it can accept and process (which should not be breaking).

PR Close #37881

goldens/public-api/forms/forms.d.ts

@@ -1,5 +1,6 @@
 export declare abstract class AbstractControl {
-    asyncValidator: AsyncValidatorFn | null;
+    get asyncValidator(): AsyncValidatorFn | null;
+    set asyncValidator(asyncValidatorFn: AsyncValidatorFn | null);
     get dirty(): boolean;
     get disabled(): boolean;
     get enabled(): boolean;
@@ -15,10 +16,11 @@ export declare abstract class AbstractControl {
     get untouched(): boolean;
     get updateOn(): FormHooks;
     get valid(): boolean;
-    validator: ValidatorFn | null;
+    get validator(): ValidatorFn | null;
+    set validator(validatorFn: ValidatorFn | null);
     readonly value: any;
     readonly valueChanges: Observable<any>;
-    constructor(validator: ValidatorFn | null, asyncValidator: AsyncValidatorFn | null);
+    constructor(validators: ValidatorFn | ValidatorFn[] | null, asyncValidators: AsyncValidatorFn | AsyncValidatorFn[] | null);
     clearAsyncValidators(): void;
     clearValidators(): void;
     disable(opts?: {

packages/forms/src/model.ts

@@ -69,20 +69,38 @@ function _find(control: AbstractControl, path: Array<string|number>|string, deli
   return controlToFind;
 }
 
-function coerceToValidator(validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|
+/**
-                           null): ValidatorFn|null {
+ * Gets validators from either an options object or given validators.
-  const validator = isOptionsObj(validatorOrOpts) ? validatorOrOpts.validators : validatorOrOpts;
+ */
+function pickValidators(validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|
+                        null): ValidatorFn|ValidatorFn[]|null {
+  return (isOptionsObj(validatorOrOpts) ? validatorOrOpts.validators : validatorOrOpts) || null;
+}
+
+/**
+ * Creates validator function by combining provided validators.
+ */
+function coerceToValidator(validator: ValidatorFn|ValidatorFn[]|null): ValidatorFn|null {
   return Array.isArray(validator) ? composeValidators(validator) : validator || null;
 }
 
-function coerceToAsyncValidator(
+/**
+ * Gets async validators from either an options object or given validators.
+ */
+function pickAsyncValidators(
     asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null,
     validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null): AsyncValidatorFn|
-    null {
+    AsyncValidatorFn[]|null {
-  const origAsyncValidator =
+  return (isOptionsObj(validatorOrOpts) ? validatorOrOpts.asyncValidators : asyncValidator) || null;
-      isOptionsObj(validatorOrOpts) ? validatorOrOpts.asyncValidators : asyncValidator;
+}
-  return Array.isArray(origAsyncValidator) ? composeAsyncValidators(origAsyncValidator) :
+
-                                             origAsyncValidator || null;
+/**
+ * Creates async validator function by combining provided async validators.
+ */
+function coerceToAsyncValidator(asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|
+                                null): AsyncValidatorFn|null {
+  return Array.isArray(asyncValidator) ? composeAsyncValidators(asyncValidator) :
+                                         asyncValidator || null;
 }
 
 export type FormHooks = 'change'|'blur'|'submit';
@@ -159,6 +177,43 @@ export abstract class AbstractControl {
   private _parent!: FormGroup|FormArray;
   private _asyncValidationSubscription: any;
 
+  /**
+   * Contains the result of merging synchronous validators into a single validator function
+   * (combined using `Validators.compose`).
+   *
+   * @internal
+   */
+  private _composedValidatorFn: ValidatorFn|null;
+
+  /**
+   * Contains the result of merging asynchronous validators into a single validator function
+   * (combined using `Validators.composeAsync`).
+   *
+   * @internal
+   */
+  private _composedAsyncValidatorFn: AsyncValidatorFn|null;
+
+  /**
+   * Synchronous validators as they were provided:
+   *  - in `AbstractControl` constructor
+   *  - as an argument while calling `setValidators` function
+   *  - while calling the setter on the `validator` field (e.g. `control.validator = validatorFn`)
+   *
+   * @internal
+   */
+  private _rawValidators: ValidatorFn|ValidatorFn[]|null;
+
+  /**
+   * Asynchronous validators as they were provided:
+   *  - in `AbstractControl` constructor
+   *  - as an argument while calling `setAsyncValidators` function
+   *  - while calling the setter on the `asyncValidator` field (e.g. `control.asyncValidator =
+   * asyncValidatorFn`)
+   *
+   * @internal
+   */
+  private _rawAsyncValidators: AsyncValidatorFn|AsyncValidatorFn[]|null;
+
   /**
    * The current value of the control.
    *
@@ -175,11 +230,39 @@ export abstract class AbstractControl {
   /**
    * Initialize the AbstractControl instance.
    *
-   * @param validator The function that determines the synchronous validity of this control.
+   * @param validators The function or array of functions that is used to determine the validity of
-   * @param asyncValidator The function that determines the asynchronous validity of this
+   *     this control synchronously.
-   * control.
+   * @param asyncValidators The function or array of functions that is used to determine validity of
+   *     this control asynchronously.
    */
-  constructor(public validator: ValidatorFn|null, public asyncValidator: AsyncValidatorFn|null) {}
+  constructor(
+      validators: ValidatorFn|ValidatorFn[]|null,
+      asyncValidators: AsyncValidatorFn|AsyncValidatorFn[]|null) {
+    this._rawValidators = validators;
+    this._rawAsyncValidators = asyncValidators;
+    this._composedValidatorFn = coerceToValidator(this._rawValidators);
+    this._composedAsyncValidatorFn = coerceToAsyncValidator(this._rawAsyncValidators);
+  }
+
+  /**
+   * The function that is used to determine the validity of this control synchronously.
+   */
+  get validator(): ValidatorFn|null {
+    return this._composedValidatorFn;
+  }
+  set validator(validatorFn: ValidatorFn|null) {
+    this._rawValidators = this._composedValidatorFn = validatorFn;
+  }
+
+  /**
+   * The function that is used to determine the validity of this control asynchronously.
+   */
+  get asyncValidator(): AsyncValidatorFn|null {
+    return this._composedAsyncValidatorFn;
+  }
+  set asyncValidator(asyncValidatorFn: AsyncValidatorFn|null) {
+    this._rawAsyncValidators = this._composedAsyncValidatorFn = asyncValidatorFn;
+  }
 
   /**
    * The parent control.
@@ -349,7 +432,8 @@ export abstract class AbstractControl {
    *
    */
   setValidators(newValidator: ValidatorFn|ValidatorFn[]|null): void {
-    this.validator = coerceToValidator(newValidator);
+    this._rawValidators = newValidator;
+    this._composedValidatorFn = coerceToValidator(newValidator);
   }
 
   /**
@@ -361,7 +445,8 @@ export abstract class AbstractControl {
    *
    */
   setAsyncValidators(newValidator: AsyncValidatorFn|AsyncValidatorFn[]|null): void {
-    this.asyncValidator = coerceToAsyncValidator(newValidator);
+    this._rawAsyncValidators = newValidator;
+    this._composedAsyncValidatorFn = coerceToAsyncValidator(newValidator);
   }
 
   /**
@@ -1061,9 +1146,7 @@ export class FormControl extends AbstractControl {
       formState: any = null,
       validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
       asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null) {
-    super(
+    super(pickValidators(validatorOrOpts), pickAsyncValidators(asyncValidator, validatorOrOpts));
-        coerceToValidator(validatorOrOpts),
-        coerceToAsyncValidator(asyncValidator, validatorOrOpts));
     this._applyFormState(formState);
     this._setUpdateStrategy(validatorOrOpts);
     this.updateValueAndValidity({onlySelf: true, emitEvent: false});
@@ -1316,9 +1399,7 @@ export class FormGroup extends AbstractControl {
       public controls: {[key: string]: AbstractControl},
       validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
       asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null) {
-    super(
+    super(pickValidators(validatorOrOpts), pickAsyncValidators(asyncValidator, validatorOrOpts));
-        coerceToValidator(validatorOrOpts),
-        coerceToAsyncValidator(asyncValidator, validatorOrOpts));
     this._initObservables();
     this._setUpdateStrategy(validatorOrOpts);
     this._setUpControls();
@@ -1738,9 +1819,7 @@ export class FormArray extends AbstractControl {
       public controls: AbstractControl[],
       validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
       asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null) {
-    super(
+    super(pickValidators(validatorOrOpts), pickAsyncValidators(asyncValidator, validatorOrOpts));
-        coerceToValidator(validatorOrOpts),
-        coerceToAsyncValidator(asyncValidator, validatorOrOpts));
     this._initObservables();
     this._setUpdateStrategy(validatorOrOpts);
     this._setUpControls();

packages/forms/test/form_control_spec.ts

@@ -263,6 +263,57 @@ describe('FormControl', () => {
       expect(c.valid).toEqual(true);
     });
 
+    it('should override validators using `setValidators` function', () => {
+      const c = new FormControl('');
+      expect(c.valid).toEqual(true);
+
+      c.setValidators([Validators.minLength(5), Validators.required]);
+
+      c.setValue('');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('abc');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('abcde');
+      expect(c.valid).toEqual(true);
+
+      // Define new set of validators, overriding previously applied ones.
+      c.setValidators([Validators.maxLength(2)]);
+
+      c.setValue('abcdef');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('a');
+      expect(c.valid).toEqual(true);
+    });
+
+    it('should override validators by setting `control.validator` field value', () => {
+      const c = new FormControl('');
+      expect(c.valid).toEqual(true);
+
+      // Define new set of validators, overriding previously applied ones.
+      c.validator = Validators.compose([Validators.minLength(5), Validators.required]);
+
+      c.setValue('');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('abc');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('abcde');
+      expect(c.valid).toEqual(true);
+
+      // Define new set of validators, overriding previously applied ones.
+      c.validator = Validators.compose([Validators.maxLength(2)]);
+
+      c.setValue('abcdef');
+      expect(c.valid).toEqual(false);
+
+      c.setValue('a');
+      expect(c.valid).toEqual(true);
+    });
+
     it('should clear validators', () => {
       const c = new FormControl('', Validators.required);
       expect(c.valid).toEqual(false);
@@ -412,6 +463,59 @@ describe('FormControl', () => {
          expect(c.valid).toEqual(true);
        }));
 
+    it('should override validators using `setAsyncValidators` function', fakeAsync(() => {
+         const c = new FormControl('');
+         expect(c.valid).toEqual(true);
+
+         c.setAsyncValidators([asyncValidator('expected')]);
+
+         c.setValue('');
+         tick();
+         expect(c.valid).toEqual(false);
+
+         c.setValue('expected');
+         tick();
+         expect(c.valid).toEqual(true);
+
+         // Define new set of validators, overriding previously applied ones.
+         c.setAsyncValidators([asyncValidator('new expected')]);
+
+         c.setValue('expected');
+         tick();
+         expect(c.valid).toEqual(false);
+
+         c.setValue('new expected');
+         tick();
+         expect(c.valid).toEqual(true);
+       }));
+
+    it('should override validators by setting `control.asyncValidator` field value',
+       fakeAsync(() => {
+         const c = new FormControl('');
+         expect(c.valid).toEqual(true);
+
+         c.asyncValidator = Validators.composeAsync([asyncValidator('expected')]);
+
+         c.setValue('');
+         tick();
+         expect(c.valid).toEqual(false);
+
+         c.setValue('expected');
+         tick();
+         expect(c.valid).toEqual(true);
+
+         // Define new set of validators, overriding previously applied ones.
+         c.asyncValidator = Validators.composeAsync([asyncValidator('new expected')]);
+
+         c.setValue('expected');
+         tick();
+         expect(c.valid).toEqual(false);
+
+         c.setValue('new expected');
+         tick();
+         expect(c.valid).toEqual(true);
+       }));
+
     it('should clear async validators', fakeAsync(() => {
          const c = new FormControl('value', [asyncValidator('expected'), otherAsyncValidator]);