diff --git a/packages/forms/src/directives/reactive_directives/form_group_directive.ts b/packages/forms/src/directives/reactive_directives/form_group_directive.ts index fb339db997..f8ab4a62a9 100644 --- a/packages/forms/src/directives/reactive_directives/form_group_directive.ts +++ b/packages/forms/src/directives/reactive_directives/form_group_directive.ts @@ -114,7 +114,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan /** @nodoc */ ngOnDestroy() { if (this.form) { - cleanUpValidators(this.form, this, /* handleOnValidatorChange */ false); + cleanUpValidators(this.form, this); // Currently the `onCollectionChange` callback is rewritten each time the // `_registerOnCollectionChange` function is invoked. The implication is that cleanup should @@ -348,9 +348,9 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan } private _updateValidators() { - setUpValidators(this.form, this, /* handleOnValidatorChange */ false); + setUpValidators(this.form, this); if (this._oldForm) { - cleanUpValidators(this._oldForm, this, /* handleOnValidatorChange */ false); + cleanUpValidators(this._oldForm, this); } } diff --git a/packages/forms/src/directives/shared.ts b/packages/forms/src/directives/shared.ts index a2dbbe667d..e8025b2327 100644 --- a/packages/forms/src/directives/shared.ts +++ b/packages/forms/src/directives/shared.ts @@ -37,7 +37,7 @@ export function setUpControl(control: FormControl, dir: NgControl): void { if (!dir.valueAccessor) _throwError(dir, 'No value accessor for form control with'); } - setUpValidators(control, dir, /* handleOnValidatorChange */ true); + setUpValidators(control, dir); dir.valueAccessor!.writeValue(control.value); @@ -79,7 +79,7 @@ export function cleanUpControl( dir.valueAccessor.registerOnTouched(noop); } - cleanUpValidators(control, dir, /* handleOnValidatorChange */ true); + cleanUpValidators(control, dir); if (control) { dir._invokeOnDestroyCallbacks(); @@ -122,12 +122,8 @@ export function setUpDisabledChangeHandler(control: FormControl, dir: NgControl) * * @param control Form control where directive validators should be setup. * @param dir Directive instance that contains validators to be setup. - * @param handleOnValidatorChange Flag that determines whether directive validators should be setup - * to handle validator input change. */ -export function setUpValidators( - control: AbstractControl, dir: AbstractControlDirective, - handleOnValidatorChange: boolean): void { +export function setUpValidators(control: AbstractControl, dir: AbstractControlDirective): void { const validators = getControlValidators(control); if (dir.validator !== null) { control.setValidators(mergeValidators(validators, dir.validator)); @@ -151,11 +147,9 @@ export function setUpValidators( } // Re-run validation when validator binding changes, e.g. minlength=3 -> minlength=4 - if (handleOnValidatorChange) { - const onValidatorChange = () => control.updateValueAndValidity(); - registerOnValidatorChange(dir._rawValidators, onValidatorChange); - registerOnValidatorChange(dir._rawAsyncValidators, onValidatorChange); - } + const onValidatorChange = () => control.updateValueAndValidity(); + registerOnValidatorChange(dir._rawValidators, onValidatorChange); + registerOnValidatorChange(dir._rawAsyncValidators, onValidatorChange); } /** @@ -165,13 +159,10 @@ export function setUpValidators( * * @param control Form control from where directive validators should be removed. * @param dir Directive instance that contains validators to be removed. - * @param handleOnValidatorChange Flag that determines whether directive validators should also be - * cleaned up to stop handling validator input change (if previously configured to do so). * @returns true if a control was updated as a result of this action. */ export function cleanUpValidators( - control: AbstractControl|null, dir: AbstractControlDirective, - handleOnValidatorChange: boolean): boolean { + control: AbstractControl|null, dir: AbstractControlDirective): boolean { let isControlUpdated = false; if (control !== null) { if (dir.validator !== null) { @@ -200,12 +191,10 @@ export function cleanUpValidators( } } - if (handleOnValidatorChange) { - // Clear onValidatorChange callbacks by providing a noop function. - const noop = () => {}; - registerOnValidatorChange(dir._rawValidators, noop); - registerOnValidatorChange(dir._rawAsyncValidators, noop); - } + // Clear onValidatorChange callbacks by providing a noop function. + const noop = () => {}; + registerOnValidatorChange(dir._rawValidators, noop); + registerOnValidatorChange(dir._rawAsyncValidators, noop); return isControlUpdated; } @@ -264,7 +253,7 @@ export function setUpFormContainer( control: FormGroup|FormArray, dir: AbstractFormGroupDirective|FormArrayName) { if (control == null && (typeof ngDevMode === 'undefined' || ngDevMode)) _throwError(dir, 'Cannot find control with'); - setUpValidators(control, dir, /* handleOnValidatorChange */ false); + setUpValidators(control, dir); } /** @@ -276,7 +265,7 @@ export function setUpFormContainer( */ export function cleanUpFormContainer( control: FormGroup|FormArray, dir: AbstractFormGroupDirective|FormArrayName): boolean { - return cleanUpValidators(control, dir, /* handleOnValidatorChange */ false); + return cleanUpValidators(control, dir); } function _noControlError(dir: NgControl) { diff --git a/packages/forms/test/reactive_integration_spec.ts b/packages/forms/test/reactive_integration_spec.ts index 7cecc14201..588c344b42 100644 --- a/packages/forms/test/reactive_integration_spec.ts +++ b/packages/forms/test/reactive_integration_spec.ts @@ -2733,6 +2733,80 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]'); expect(form.controls.pin.errors).toEqual({max: {max: -10, actual: 0}}); }); }); + + it('should fire registerOnValidatorChange for validators attached to the formGroups', + () => { + let registerOnValidatorChangeFired = 0; + let registerOnAsyncValidatorChangeFired = 0; + + @Directive({ + selector: '[ng-noop-validator]', + providers: [ + {provide: NG_VALIDATORS, useExisting: forwardRef(() => NoOpValidator), multi: true} + ] + }) + class NoOpValidator implements Validator { + @Input() validatorInput = ''; + + validate(c: AbstractControl) { + return null; + } + + public registerOnValidatorChange(fn: () => void) { + registerOnValidatorChangeFired++; + } + } + + @Directive({ + selector: '[ng-noop-async-validator]', + providers: [{ + provide: NG_ASYNC_VALIDATORS, + useExisting: forwardRef(() => NoOpAsyncValidator), + multi: true + }] + }) + class NoOpAsyncValidator implements AsyncValidator { + @Input() validatorInput = ''; + + validate(c: AbstractControl) { + return Promise.resolve(null); + } + + public registerOnValidatorChange(fn: () => void) { + registerOnAsyncValidatorChangeFired++; + } + } + + @Component({ + selector: 'ng-model-noop-validation', + template: ` +
+ +
+ ` + }) + class NgModelNoOpValidation { + validatorInput = 'bar'; + + fooGroup = new FormGroup({ + fooInput: new FormControl(''), + }); + } + + const fixture = initTest(NgModelNoOpValidation, NoOpValidator, NoOpAsyncValidator); + fixture.detectChanges(); + + expect(registerOnValidatorChangeFired).toBe(1); + expect(registerOnAsyncValidatorChangeFired).toBe(1); + + fixture.componentInstance.validatorInput = 'baz'; + fixture.detectChanges(); + + // Changing the validator input should not cause the onValidatorChange to be called + // again. + expect(registerOnValidatorChangeFired).toBe(1); + expect(registerOnAsyncValidatorChangeFired).toBe(1); + }); }); }); diff --git a/packages/forms/test/template_integration_spec.ts b/packages/forms/test/template_integration_spec.ts index 8e441c27f9..e35d281d64 100644 --- a/packages/forms/test/template_integration_spec.ts +++ b/packages/forms/test/template_integration_spec.ts @@ -9,7 +9,7 @@ import {ɵgetDOM as getDOM} from '@angular/common'; import {Component, Directive, forwardRef, Input, Type, ViewChild} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; -import {AbstractControl, AsyncValidator, COMPOSITION_BUFFER_MODE, ControlValueAccessor, FormControl, FormsModule, MaxValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgForm, NgModel} from '@angular/forms'; +import {AbstractControl, AsyncValidator, COMPOSITION_BUFFER_MODE, ControlValueAccessor, FormControl, FormsModule, MaxValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgForm, NgModel, Validator} from '@angular/forms'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {dispatchEvent, sortedClassList} from '@angular/platform-browser/testing/src/browser_util'; import {merge} from 'rxjs'; @@ -1967,6 +1967,79 @@ import {NgModelCustomComp, NgModelCustomWrapper} from './value_accessor_integrat expect(form.valid).toBeFalse(); expect(form.controls.min_max.errors).toEqual({max: {max: -10, actual: 0}}); })); + + it('should call registerOnValidatorChange as a part of a formGroup setup', fakeAsync(() => { + let registerOnValidatorChangeFired = 0; + let registerOnAsyncValidatorChangeFired = 0; + + @Directive({ + selector: '[ng-noop-validator]', + providers: [ + {provide: NG_VALIDATORS, useExisting: forwardRef(() => NoOpValidator), multi: true} + ] + }) + class NoOpValidator implements Validator { + @Input() validatorInput = ''; + + validate(c: AbstractControl) { + return null; + } + + public registerOnValidatorChange(fn: () => void) { + registerOnValidatorChangeFired++; + } + } + + @Directive({ + selector: '[ng-noop-async-validator]', + providers: [{ + provide: NG_ASYNC_VALIDATORS, + useExisting: forwardRef(() => NoOpAsyncValidator), + multi: true + }] + }) + class NoOpAsyncValidator implements AsyncValidator { + @Input() validatorInput = ''; + + validate(c: AbstractControl) { + return Promise.resolve(null); + } + + public registerOnValidatorChange(fn: () => void) { + registerOnAsyncValidatorChangeFired++; + } + } + + @Component({ + selector: 'ng-model-noop-validation', + template: ` +
+
+ +
+
+ ` + }) + class NgModelNoOpValidation { + validatorInput = 'foo'; + emptyGroup = {}; + } + + const fixture = initTest(NgModelNoOpValidation, NoOpValidator, NoOpAsyncValidator); + fixture.detectChanges(); + tick(); + + expect(registerOnValidatorChangeFired).toBe(1); + expect(registerOnAsyncValidatorChangeFired).toBe(1); + + fixture.componentInstance.validatorInput = 'bar'; + fixture.detectChanges(); + + // Changing validator inputs should not cause `registerOnValidatorChange` to be invoked, + // since it's invoked just once during the setup phase. + expect(registerOnValidatorChangeFired).toBe(1); + expect(registerOnAsyncValidatorChangeFired).toBe(1); + })); }); describe('IME events', () => {