ngneat / reactive-forms

(Angular Reactive) Forms with Benefits 😉
https://www.netbasal.com
611 stars 56 forks source link

Consider using something like `fast-deep-equal` to check if errors are equal in `controlErrorChanges$` #131

Closed jafaircl closed 2 years ago

jafaircl commented 2 years ago

Description

Comparing two objects using JSON.stringify has 2 drawbacks:

1) It's much slower than it needs to be. Using a library that checks if two objects are equivalent based on their values can be about an order of magnitude faster. I forked fast-deep-equal and added equality functions using JSON.stringify and the stringify function from fast-json-stable-stringify to the benchmark. You can see some of the results below:

fast-deep-equal x 275,971 ops/sec ±0.57% (92 runs sampled)
fast-deep-equal/es6 x 241,026 ops/sec ±0.59% (92 runs sampled)
fast-equals x 282,790 ops/sec ±0.57% (88 runs sampled)
nano-equal x 199,804 ops/sec ±0.71% (93 runs sampled)
shallow-equal-fuzzy x 155,549 ops/sec ±0.50% (91 runs sampled)
JSON.stringify x 29,999 ops/sec ±0.73% (93 runs sampled)
fast-json-stable-stringify x 19,918 ops/sec ±0.55% (90 runs sampled)
The fastest is fast-equals

2) If two objects have equivalent values but the keys are in different orders, they will be not be considered equivalent. For example, { required: true, min: true } and { min: true, required: true } are considered different values. With a library like fast-deep-equal or fast-equals, they would be considered the same.

Proposed solution

Instead of using the current equality function which uses JSON.stringify to check if two values are equal, use something like fast-deep-equal to check.

import fastDeepEqual from 'fast-deep-equal'

export function controlErrorChanges$(
  control: AbstractControl,
  errors$: Observable<ValidationErrors | null>
): Observable<ValidationErrors | null> {
  return merge(
    defer(() => of(control.errors)),
    errors$,
    control.valueChanges.pipe(
      map(() => control.errors),
      distinctUntilChanged((a, b) => fastDeepEqual(a, b))
    )
  );
}

I can create a PR to do this. But, it will add a dependency. So, I wanted to reach out and see if this is something you would be interested in changing before going any further.

Alternatives considered

We could pipe the errors$ observable like errors$.pipe(distinctUntilChanged(fastDeepEqual). But, that would still run them through the JSON.stringify equality function, negating the speed benefit.

Do you want to create a pull request?

Yes