Skip to content

Commit 856db56

Browse files
committed
refactor(forms): get rid of duplicate functions (angular#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 angular#38371
1 parent 354e66e commit 856db56

File tree

6 files changed

+58
-66
lines changed

6 files changed

+58
-66
lines changed

goldens/circular-deps/packages.json

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,11 +1855,6 @@
18551855
"packages/forms/src/directives/ng_model.ts",
18561856
"packages/forms/src/directives/ng_model_group.ts"
18571857
],
1858-
[
1859-
"packages/forms/src/directives/normalize_validator.ts",
1860-
"packages/forms/src/model.ts",
1861-
"packages/forms/src/directives/shared.ts"
1862-
],
18631858
[
18641859
"packages/forms/src/directives/reactive_directives/form_control_directive.ts",
18651860
"packages/forms/src/directives/shared.ts",

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -701,9 +701,6 @@
701701
{
702702
"name": "_keyMap"
703703
},
704-
{
705-
"name": "_mergeErrors"
706-
},
707704
{
708705
"name": "_noControlError"
709706
},
@@ -914,6 +911,9 @@
914911
{
915912
"name": "executeTemplate"
916913
},
914+
{
915+
"name": "executeValidators"
916+
},
917917
{
918918
"name": "executeViewQueryFn"
919919
},
@@ -1346,6 +1346,9 @@
13461346
{
13471347
"name": "mergeAll"
13481348
},
1349+
{
1350+
"name": "mergeErrors"
1351+
},
13491352
{
13501353
"name": "mergeHostAttribute"
13511354
},
@@ -1401,10 +1404,7 @@
14011404
"name": "noop"
14021405
},
14031406
{
1404-
"name": "normalizeAsyncValidator"
1405-
},
1406-
{
1407-
"name": "normalizeValidator"
1407+
"name": "normalizeValidators"
14081408
},
14091409
{
14101410
"name": "observable"

packages/forms/src/directives/normalize_validator.ts

Lines changed: 0 additions & 27 deletions
This file was deleted.

packages/forms/src/directives/shared.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@
99
import {isDevMode} from '@angular/core';
1010

1111
import {FormArray, FormControl, FormGroup} from '../model';
12-
import {Validators} from '../validators';
12+
import {normalizeValidators, Validators} from '../validators';
13+
1314
import {AbstractControlDirective} from './abstract_control_directive';
1415
import {AbstractFormGroupDirective} from './abstract_form_group_directive';
1516
import {CheckboxControlValueAccessor} from './checkbox_value_accessor';
1617
import {ControlContainer} from './control_container';
1718
import {ControlValueAccessor} from './control_value_accessor';
1819
import {DefaultValueAccessor} from './default_value_accessor';
1920
import {NgControl} from './ng_control';
20-
import {normalizeAsyncValidator, normalizeValidator} from './normalize_validator';
2121
import {NumberValueAccessor} from './number_value_accessor';
2222
import {RadioControlValueAccessor} from './radio_control_value_accessor';
2323
import {RangeValueAccessor} from './range_value_accessor';
@@ -142,13 +142,15 @@ function _throwError(dir: AbstractControlDirective, message: string): void {
142142
}
143143

144144
export function composeValidators(validators: Array<Validator|ValidatorFn>): ValidatorFn|null {
145-
return validators != null ? Validators.compose(validators.map(normalizeValidator)) : null;
145+
return validators != null ? Validators.compose(normalizeValidators<ValidatorFn>(validators)) :
146+
null;
146147
}
147148

148149
export function composeAsyncValidators(validators: Array<AsyncValidator|AsyncValidatorFn>):
149150
AsyncValidatorFn|null {
150-
return validators != null ? Validators.composeAsync(validators.map(normalizeAsyncValidator)) :
151-
null;
151+
return validators != null ?
152+
Validators.composeAsync(normalizeValidators<AsyncValidatorFn>(validators)) :
153+
null;
152154
}
153155

154156
export function isPropertyUpdated(changes: {[key: string]: any}, viewModel: any): boolean {

packages/forms/src/validators.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {InjectionToken, ɵisObservable as isObservable, ɵisPromise as isPromise
1010
import {forkJoin, from, Observable} from 'rxjs';
1111
import {map} from 'rxjs/operators';
1212

13-
import {AsyncValidatorFn, ValidationErrors, Validator, ValidatorFn} from './directives/validators';
13+
import {AsyncValidator, AsyncValidatorFn, ValidationErrors, Validator, ValidatorFn} from './directives/validators';
1414
import {AbstractControl} from './model';
1515

1616
function isEmptyInputValue(value: any): boolean {
@@ -435,7 +435,7 @@ export class Validators {
435435
if (presentValidators.length == 0) return null;
436436

437437
return function(control: AbstractControl) {
438-
return _mergeErrors(_executeValidators(control, presentValidators));
438+
return mergeErrors(executeValidators<ValidatorFn>(control, presentValidators));
439439
};
440440
}
441441

@@ -456,8 +456,9 @@ export class Validators {
456456
if (presentValidators.length == 0) return null;
457457

458458
return function(control: AbstractControl) {
459-
const observables = _executeAsyncValidators(control, presentValidators).map(toObservable);
460-
return forkJoin(observables).pipe(map(_mergeErrors));
459+
const observables =
460+
executeValidators<AsyncValidatorFn>(control, presentValidators).map(toObservable);
461+
return forkJoin(observables).pipe(map(mergeErrors));
461462
};
462463
}
463464
}
@@ -474,15 +475,7 @@ export function toObservable(r: any): Observable<any> {
474475
return obs;
475476
}
476477

477-
function _executeValidators(control: AbstractControl, validators: ValidatorFn[]): any[] {
478-
return validators.map(v => v(control));
479-
}
480-
481-
function _executeAsyncValidators(control: AbstractControl, validators: AsyncValidatorFn[]): any[] {
482-
return validators.map(v => v(control));
483-
}
484-
485-
function _mergeErrors(arrayOfErrors: ValidationErrors[]): ValidationErrors|null {
478+
function mergeErrors(arrayOfErrors: (ValidationErrors|null)[]): ValidationErrors|null {
486479
let res: {[key: string]: any} = {};
487480

488481
// Not using Array.reduce here due to a Chrome 80 bug
@@ -493,3 +486,30 @@ function _mergeErrors(arrayOfErrors: ValidationErrors[]): ValidationErrors|null
493486

494487
return Object.keys(res).length === 0 ? null : res;
495488
}
489+
490+
type GenericValidatorFn = (control: AbstractControl) => any;
491+
492+
function executeValidators<V extends GenericValidatorFn>(
493+
control: AbstractControl, validators: V[]): ReturnType<V>[] {
494+
return validators.map(validator => validator(control));
495+
}
496+
497+
function isValidatorFn<V>(validator: V|Validator|AsyncValidator): validator is V {
498+
return !(validator as Validator).validate;
499+
}
500+
501+
/**
502+
* Given the list of validators that may contain both functions as well as classes, return the list
503+
* of validator functions (convert validator classes into validator functions). This is needed to
504+
* have consistent structure in validators list before composing them.
505+
*
506+
* @param validators The set of validators that may contain validators both in plain function form
507+
* as well as represented as a validator class.
508+
*/
509+
export function normalizeValidators<V>(validators: (V|Validator|AsyncValidator)[]): V[] {
510+
return validators.map(validator => {
511+
return isValidatorFn<V>(validator) ?
512+
validator :
513+
((c: AbstractControl) => validator.validate(c)) as unknown as V;
514+
});
515+
}

packages/forms/test/validators_spec.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88

99
import {fakeAsync, tick} from '@angular/core/testing';
1010
import {describe, expect, it} from '@angular/core/testing/src/testing_internal';
11-
import {AbstractControl, AsyncValidatorFn, FormArray, FormControl, Validators} from '@angular/forms';
12-
import {normalizeAsyncValidator} from '@angular/forms/src/directives/normalize_validator';
13-
import {AsyncValidator, ValidationErrors, ValidatorFn} from '@angular/forms/src/directives/validators';
11+
import {AbstractControl, AsyncValidator, AsyncValidatorFn, FormArray, FormControl, ValidationErrors, ValidatorFn, Validators} from '@angular/forms';
1412
import {Observable, of, timer} from 'rxjs';
1513
import {first, map} from 'rxjs/operators';
1614

15+
import {normalizeValidators} from '../src/validators';
16+
1717
(function() {
1818
function validator(key: string, error: any): ValidatorFn {
1919
return (c: AbstractControl) => {
@@ -413,11 +413,12 @@ describe('Validators', () => {
413413
}));
414414

415415
it('should normalize and evaluate async validator-directives correctly', fakeAsync(() => {
416-
const v = Validators.composeAsync(
417-
[normalizeAsyncValidator(new AsyncValidatorDirective('expected', {'one': true}))])!;
416+
const normalizedValidators = normalizeValidators<AsyncValidatorFn>(
417+
[new AsyncValidatorDirective('expected', {'one': true})]);
418+
const validatorFn = Validators.composeAsync(normalizedValidators)!;
418419

419420
let errorMap: {[key: string]: any}|null = undefined!;
420-
(v(new FormControl('invalid')) as Observable<ValidationErrors|null>)
421+
(validatorFn(new FormControl('invalid')) as Observable<ValidationErrors|null>)
421422
.pipe(first())
422423
.subscribe((errors: {[key: string]: any}|null) => errorMap = errors);
423424
tick();
@@ -475,11 +476,12 @@ describe('Validators', () => {
475476
});
476477

477478
it('should normalize and evaluate async validator-directives correctly', () => {
478-
const v = Validators.composeAsync(
479-
[normalizeAsyncValidator(new AsyncValidatorDirective('expected', {'one': true}))])!;
479+
const normalizedValidators = normalizeValidators<AsyncValidatorFn>(
480+
[new AsyncValidatorDirective('expected', {'one': true})]);
481+
const validatorFn = Validators.composeAsync(normalizedValidators)!;
480482

481483
let errorMap: {[key: string]: any}|null = undefined!;
482-
(v(new FormControl('invalid')) as Observable<ValidationErrors|null>)
484+
(validatorFn(new FormControl('invalid')) as Observable<ValidationErrors|null>)
483485
.pipe(first())
484486
.subscribe((errors: {[key: string]: any}|null) => errorMap = errors)!;
485487

0 commit comments

Comments
 (0)