Skip to content

Commit

Permalink
fix(forms): change Array.reduce usage to Array.forEach (#35349)
Browse files Browse the repository at this point in the history
There is currently a bug in Chrome 80 that makes Array.reduce
not work according to spec. The functionality in forms that
retrieves controls from FormGroups and FormArrays (`form.get`)
relied on Array.reduce, so the Chrome bug broke forms for
many users.

This commit refactors our forms code to rely on Array.forEach
instead of Array.reduce to fix forms while we are waiting
for the Chrome fix to go live.

See https://bugs.chromium.org/p/chromium/issues/detail?id=1049982.

PR Close #35349
  • Loading branch information
kara committed Feb 12, 2020
1 parent a245e9d commit 554c2cb
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
24 changes: 14 additions & 10 deletions packages/forms/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,21 @@ function _find(control: AbstractControl, path: Array<string|number>| string, del
}
if (Array.isArray(path) && path.length === 0) return null;

return path.reduce((v: AbstractControl | null, name) => {
if (v instanceof FormGroup) {
return v.controls.hasOwnProperty(name as string) ? v.controls[name] : null;
}

if (v instanceof FormArray) {
return v.at(<number>name) || null;
// Not using Array.reduce here due to a Chrome 80 bug
// https://bugs.chromium.org/p/chromium/issues/detail?id=1049982
let controlToFind: AbstractControl|null = control;
path.forEach((name: string | number) => {
if (controlToFind instanceof FormGroup) {
controlToFind = controlToFind.controls.hasOwnProperty(name as string) ?
controlToFind.controls[name] :
null;
} else if (controlToFind instanceof FormArray) {
controlToFind = controlToFind.at(<number>name) || null;
} else {
controlToFind = null;
}

return null;
}, control);
});
return controlToFind;
}

function coerceToValidator(
Expand Down
12 changes: 8 additions & 4 deletions packages/forms/src/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,13 @@ function _executeAsyncValidators(control: AbstractControl, validators: AsyncVali
}

function _mergeErrors(arrayOfErrors: ValidationErrors[]): ValidationErrors|null {
const res: {[key: string]: any} =
arrayOfErrors.reduce((res: ValidationErrors | null, errors: ValidationErrors | null) => {
return errors != null ? {...res !, ...errors} : res !;
}, {});
let res: {[key: string]: any} = {};

// Not using Array.reduce here due to a Chrome 80 bug
// https://bugs.chromium.org/p/chromium/issues/detail?id=1049982
arrayOfErrors.forEach((errors: ValidationErrors | null) => {
res = errors != null ? {...res !, ...errors} : res !;
});

return Object.keys(res).length === 0 ? null : res;
}

0 comments on commit 554c2cb

Please sign in to comment.