From 856db56cca87b369cd46d55c0f1b3b7977583e6c Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Wed, 5 Aug 2020 19:14:08 -0700 Subject: [PATCH] refactor(forms): get rid of duplicate functions (#38371) This commit performs minor refactoring in Forms package to get rid of duplicate functions. It looks like the functions were duplicated due to a slightly different type signatures, but their logic is completely identical. The logic in retained functions remains the same and now these function also accept a generic type to achieve the same level of type safety. PR Close #38371 --- goldens/circular-deps/packages.json | 5 -- .../bundling/forms/bundle.golden_symbols.json | 14 +++--- .../src/directives/normalize_validator.ts | 27 ----------- packages/forms/src/directives/shared.ts | 12 +++-- packages/forms/src/validators.ts | 46 +++++++++++++------ packages/forms/test/validators_spec.ts | 20 ++++---- 6 files changed, 58 insertions(+), 66 deletions(-) delete mode 100644 packages/forms/src/directives/normalize_validator.ts diff --git a/goldens/circular-deps/packages.json b/goldens/circular-deps/packages.json index e8aebc9230..924b196222 100644 --- a/goldens/circular-deps/packages.json +++ b/goldens/circular-deps/packages.json @@ -1855,11 +1855,6 @@ "packages/forms/src/directives/ng_model.ts", "packages/forms/src/directives/ng_model_group.ts" ], - [ - "packages/forms/src/directives/normalize_validator.ts", - "packages/forms/src/model.ts", - "packages/forms/src/directives/shared.ts" - ], [ "packages/forms/src/directives/reactive_directives/form_control_directive.ts", "packages/forms/src/directives/shared.ts", diff --git a/packages/core/test/bundling/forms/bundle.golden_symbols.json b/packages/core/test/bundling/forms/bundle.golden_symbols.json index 85063ccff6..53a49c913c 100644 --- a/packages/core/test/bundling/forms/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms/bundle.golden_symbols.json @@ -701,9 +701,6 @@ { "name": "_keyMap" }, - { - "name": "_mergeErrors" - }, { "name": "_noControlError" }, @@ -914,6 +911,9 @@ { "name": "executeTemplate" }, + { + "name": "executeValidators" + }, { "name": "executeViewQueryFn" }, @@ -1346,6 +1346,9 @@ { "name": "mergeAll" }, + { + "name": "mergeErrors" + }, { "name": "mergeHostAttribute" }, @@ -1401,10 +1404,7 @@ "name": "noop" }, { - "name": "normalizeAsyncValidator" - }, - { - "name": "normalizeValidator" + "name": "normalizeValidators" }, { "name": "observable" diff --git a/packages/forms/src/directives/normalize_validator.ts b/packages/forms/src/directives/normalize_validator.ts deleted file mode 100644 index 3246bdd4f9..0000000000 --- a/packages/forms/src/directives/normalize_validator.ts +++ /dev/null @@ -1,27 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {AbstractControl} from '../model'; -import {AsyncValidator, AsyncValidatorFn, Validator, ValidatorFn} from './validators'; - -export function normalizeValidator(validator: ValidatorFn|Validator): ValidatorFn { - if (!!(validator).validate) { - return (c: AbstractControl) => (validator).validate(c); - } else { - return validator; - } -} - -export function normalizeAsyncValidator(validator: AsyncValidatorFn| - AsyncValidator): AsyncValidatorFn { - if (!!(validator).validate) { - return (c: AbstractControl) => (validator).validate(c); - } else { - return validator; - } -} diff --git a/packages/forms/src/directives/shared.ts b/packages/forms/src/directives/shared.ts index 51c9182bae..04c950aa9f 100644 --- a/packages/forms/src/directives/shared.ts +++ b/packages/forms/src/directives/shared.ts @@ -9,7 +9,8 @@ import {isDevMode} from '@angular/core'; import {FormArray, FormControl, FormGroup} from '../model'; -import {Validators} from '../validators'; +import {normalizeValidators, Validators} from '../validators'; + import {AbstractControlDirective} from './abstract_control_directive'; import {AbstractFormGroupDirective} from './abstract_form_group_directive'; import {CheckboxControlValueAccessor} from './checkbox_value_accessor'; @@ -17,7 +18,6 @@ import {ControlContainer} from './control_container'; import {ControlValueAccessor} from './control_value_accessor'; import {DefaultValueAccessor} from './default_value_accessor'; import {NgControl} from './ng_control'; -import {normalizeAsyncValidator, normalizeValidator} from './normalize_validator'; import {NumberValueAccessor} from './number_value_accessor'; import {RadioControlValueAccessor} from './radio_control_value_accessor'; import {RangeValueAccessor} from './range_value_accessor'; @@ -142,13 +142,15 @@ function _throwError(dir: AbstractControlDirective, message: string): void { } export function composeValidators(validators: Array): ValidatorFn|null { - return validators != null ? Validators.compose(validators.map(normalizeValidator)) : null; + return validators != null ? Validators.compose(normalizeValidators(validators)) : + null; } export function composeAsyncValidators(validators: Array): AsyncValidatorFn|null { - return validators != null ? Validators.composeAsync(validators.map(normalizeAsyncValidator)) : - null; + return validators != null ? + Validators.composeAsync(normalizeValidators(validators)) : + null; } export function isPropertyUpdated(changes: {[key: string]: any}, viewModel: any): boolean { diff --git a/packages/forms/src/validators.ts b/packages/forms/src/validators.ts index d14709fa84..d2fb2bbf86 100644 --- a/packages/forms/src/validators.ts +++ b/packages/forms/src/validators.ts @@ -10,7 +10,7 @@ import {InjectionToken, ɵisObservable as isObservable, ɵisPromise as isPromise import {forkJoin, from, Observable} from 'rxjs'; import {map} from 'rxjs/operators'; -import {AsyncValidatorFn, ValidationErrors, Validator, ValidatorFn} from './directives/validators'; +import {AsyncValidator, AsyncValidatorFn, ValidationErrors, Validator, ValidatorFn} from './directives/validators'; import {AbstractControl} from './model'; function isEmptyInputValue(value: any): boolean { @@ -435,7 +435,7 @@ export class Validators { if (presentValidators.length == 0) return null; return function(control: AbstractControl) { - return _mergeErrors(_executeValidators(control, presentValidators)); + return mergeErrors(executeValidators(control, presentValidators)); }; } @@ -456,8 +456,9 @@ export class Validators { if (presentValidators.length == 0) return null; return function(control: AbstractControl) { - const observables = _executeAsyncValidators(control, presentValidators).map(toObservable); - return forkJoin(observables).pipe(map(_mergeErrors)); + const observables = + executeValidators(control, presentValidators).map(toObservable); + return forkJoin(observables).pipe(map(mergeErrors)); }; } } @@ -474,15 +475,7 @@ export function toObservable(r: any): Observable { return obs; } -function _executeValidators(control: AbstractControl, validators: ValidatorFn[]): any[] { - return validators.map(v => v(control)); -} - -function _executeAsyncValidators(control: AbstractControl, validators: AsyncValidatorFn[]): any[] { - return validators.map(v => v(control)); -} - -function _mergeErrors(arrayOfErrors: ValidationErrors[]): ValidationErrors|null { +function mergeErrors(arrayOfErrors: (ValidationErrors|null)[]): ValidationErrors|null { let res: {[key: string]: any} = {}; // Not using Array.reduce here due to a Chrome 80 bug @@ -493,3 +486,30 @@ function _mergeErrors(arrayOfErrors: ValidationErrors[]): ValidationErrors|null return Object.keys(res).length === 0 ? null : res; } + +type GenericValidatorFn = (control: AbstractControl) => any; + +function executeValidators( + control: AbstractControl, validators: V[]): ReturnType[] { + return validators.map(validator => validator(control)); +} + +function isValidatorFn(validator: V|Validator|AsyncValidator): validator is V { + return !(validator as Validator).validate; +} + +/** + * Given the list of validators that may contain both functions as well as classes, return the list + * of validator functions (convert validator classes into validator functions). This is needed to + * have consistent structure in validators list before composing them. + * + * @param validators The set of validators that may contain validators both in plain function form + * as well as represented as a validator class. + */ +export function normalizeValidators(validators: (V|Validator|AsyncValidator)[]): V[] { + return validators.map(validator => { + return isValidatorFn(validator) ? + validator : + ((c: AbstractControl) => validator.validate(c)) as unknown as V; + }); +} \ No newline at end of file diff --git a/packages/forms/test/validators_spec.ts b/packages/forms/test/validators_spec.ts index 9450cfee17..db5d5262d5 100644 --- a/packages/forms/test/validators_spec.ts +++ b/packages/forms/test/validators_spec.ts @@ -8,12 +8,12 @@ import {fakeAsync, tick} from '@angular/core/testing'; import {describe, expect, it} from '@angular/core/testing/src/testing_internal'; -import {AbstractControl, AsyncValidatorFn, FormArray, FormControl, Validators} from '@angular/forms'; -import {normalizeAsyncValidator} from '@angular/forms/src/directives/normalize_validator'; -import {AsyncValidator, ValidationErrors, ValidatorFn} from '@angular/forms/src/directives/validators'; +import {AbstractControl, AsyncValidator, AsyncValidatorFn, FormArray, FormControl, ValidationErrors, ValidatorFn, Validators} from '@angular/forms'; import {Observable, of, timer} from 'rxjs'; import {first, map} from 'rxjs/operators'; +import {normalizeValidators} from '../src/validators'; + (function() { function validator(key: string, error: any): ValidatorFn { return (c: AbstractControl) => { @@ -413,11 +413,12 @@ describe('Validators', () => { })); it('should normalize and evaluate async validator-directives correctly', fakeAsync(() => { - const v = Validators.composeAsync( - [normalizeAsyncValidator(new AsyncValidatorDirective('expected', {'one': true}))])!; + const normalizedValidators = normalizeValidators( + [new AsyncValidatorDirective('expected', {'one': true})]); + const validatorFn = Validators.composeAsync(normalizedValidators)!; let errorMap: {[key: string]: any}|null = undefined!; - (v(new FormControl('invalid')) as Observable) + (validatorFn(new FormControl('invalid')) as Observable) .pipe(first()) .subscribe((errors: {[key: string]: any}|null) => errorMap = errors); tick(); @@ -475,11 +476,12 @@ describe('Validators', () => { }); it('should normalize and evaluate async validator-directives correctly', () => { - const v = Validators.composeAsync( - [normalizeAsyncValidator(new AsyncValidatorDirective('expected', {'one': true}))])!; + const normalizedValidators = normalizeValidators( + [new AsyncValidatorDirective('expected', {'one': true})]); + const validatorFn = Validators.composeAsync(normalizedValidators)!; let errorMap: {[key: string]: any}|null = undefined!; - (v(new FormControl('invalid')) as Observable) + (validatorFn(new FormControl('invalid')) as Observable) .pipe(first()) .subscribe((errors: {[key: string]: any}|null) => errorMap = errors)!;