Skip to content

Commit 53a06e2

Browse files
tnagorratnagorra
committed
Add optimization so error object reference does not change
- Error object was recreated even if value change did not change the error object
1 parent b1a4a60 commit 53a06e2

File tree

3 files changed

+96
-11
lines changed

3 files changed

+96
-11
lines changed

‎lib/src/schema.js‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ import {
2121
hasNoValues,
2222
} from './utils';
2323

24+
function isObjectReasonablyEqual(foo, bar) {
25+
if (isNotDefined(foo) && isNotDefined(bar)) {
26+
return true;
27+
}
28+
if (isNotDefined(foo) || isNotDefined(bar)) {
29+
return false;
30+
}
31+
const fooKeys = new Set(Reflect.ownKeys(foo).map((key) => isDefined(foo[key]) ? key : undefined).filter(isDefined));
32+
const barKeys = new Set(Reflect.ownKeys(bar).map((key) => isDefined(bar[key]) ? key : undefined).filter(isDefined));
33+
if (fooKeys.size !== barKeys.size) {
34+
return false;
35+
}
36+
if ([...fooKeys].some((fooKey) => !barKeys.has(fooKey))) {
37+
return false;
38+
}
39+
return [...fooKeys].every((fooKey) => foo[fooKey] === bar[fooKey]);
40+
}
41+
2442
export function accumulateValues(
2543
obj,
2644
schema,
@@ -76,7 +94,7 @@ export function accumulateValues(
7694
}
7795
return schema.defaultValue;
7896
}
79-
// FIXME: should we instead use (return nullable ? null : undefined)?
97+
// TODO: should we instead use (return nullable ? null : undefined)?
8098
return [];
8199
}
82100
return values;
@@ -216,7 +234,6 @@ export function accumulateErrors(
216234
return error;
217235
}
218236

219-
// FIXME: only copy oldErrors when a change is detected
220237
export function accumulateDifferentialErrors(
221238
oldObj,
222239
newObj,
@@ -246,7 +263,7 @@ export function accumulateDifferentialErrors(
246263
const isSchemaForObject = !!fields;
247264

248265
if (isSchemaForArray) {
249-
const errors = {};
266+
let errors = {};
250267
// FIXME: make this more performant? deps?
251268
if (validation) {
252269
const validationErrors = validation(newObj, baseValue, context);
@@ -279,8 +296,6 @@ export function accumulateDifferentialErrors(
279296
const localMember = member(e.new);
280297
const index = keySelector(e.new);
281298

282-
// console.warn(index, depsChanged);
283-
284299
const fieldError = accumulateDifferentialErrors(
285300
e.old,
286301
e.new,
@@ -296,6 +311,9 @@ export function accumulateDifferentialErrors(
296311
}
297312
});
298313

314+
if (isObjectReasonablyEqual(oldError, errors)) {
315+
return oldError;
316+
}
299317
return hasNoKeys(errors) ? undefined : errors;
300318
}
301319

@@ -324,8 +342,6 @@ export function accumulateDifferentialErrors(
324342
|| (depsChanged && isDefined(dependenciesObj?.[fieldName]))
325343
);
326344

327-
// console.warn(fieldName, depsChangedForField, dependenciesObj?.[fieldName]);
328-
329345
const fieldError = accumulateDifferentialErrors(
330346
oldObj?.[fieldName],
331347
newObj?.[fieldName],
@@ -341,6 +357,9 @@ export function accumulateDifferentialErrors(
341357
}
342358
});
343359

360+
if (isObjectReasonablyEqual(oldError, errors)) {
361+
return oldError;
362+
}
344363
return hasNoKeys(errors) ? undefined : errors;
345364
}
346365

@@ -397,7 +416,7 @@ export function analyzeErrors(errors) {
397416
}
398417

399418
// FIXME: move this to a helper
400-
// FIXME: check conditions for change in context and baseValue
419+
// TODO: check conditions for change in context and baseValue
401420
export function addCondition(
402421
schema,
403422
value,

‎lib/src/schemaAccumulateDifferentialErrors.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,3 +622,44 @@ test('accumulate differential errors with nested conditional', () => {
622622
},
623623
});
624624
});
625+
626+
test('accumulate differential errors with nested conditional and reference preserve', () => {
627+
const oldValue: PartialForm<FormType> = {
628+
name: 'Hari Bahadur',
629+
email: '[email protected]',
630+
clients: [
631+
{
632+
clientId: '1',
633+
strength: -10,
634+
},
635+
{
636+
clientId: '2',
637+
strength: -10,
638+
},
639+
],
640+
};
641+
const newValue: PartialForm<FormType> = {
642+
...oldValue,
643+
name: 'Madan Krishna',
644+
};
645+
const oldError = {
646+
clients: {
647+
1: {
648+
strength: 'The field must be greater than or equal to 0',
649+
},
650+
2: {
651+
strength: 'The field must be greater than or equal to 0',
652+
},
653+
},
654+
};
655+
const newError = accumulateDifferentialErrors(
656+
oldValue,
657+
newValue,
658+
oldError,
659+
errorFormTypeSchema,
660+
undefined,
661+
undefined,
662+
)
663+
expect(newError).toStrictEqual(oldError);
664+
expect(newError).toBe(oldError);
665+
});

‎yarn.lock‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14361,7 +14361,16 @@ string-length@^4.0.1:
1436114361
char-regex "^1.0.2"
1436214362
strip-ansi "^6.0.0"
1436314363

14364-
"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
14364+
"string-width-cjs@npm:string-width@^4.2.0":
14365+
version "4.2.3"
14366+
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
14367+
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
14368+
dependencies:
14369+
emoji-regex "^8.0.0"
14370+
is-fullwidth-code-point "^3.0.0"
14371+
strip-ansi "^6.0.1"
14372+
14373+
"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
1436514374
version "4.2.3"
1436614375
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
1436714376
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@@ -14485,7 +14494,7 @@ string_decoder@~1.1.1:
1448514494
dependencies:
1448614495
safe-buffer "~5.1.0"
1448714496

14488-
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
14497+
"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
1448914498
version "6.0.1"
1449014499
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
1449114500
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@@ -14499,6 +14508,13 @@ strip-ansi@^3.0.1:
1449914508
dependencies:
1450014509
ansi-regex "^2.0.0"
1450114510

14511+
strip-ansi@^6.0.0, strip-ansi@^6.0.1:
14512+
version "6.0.1"
14513+
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
14514+
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
14515+
dependencies:
14516+
ansi-regex "^5.0.1"
14517+
1450214518
strip-ansi@^7.0.1:
1450314519
version "7.1.0"
1450414520
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
@@ -15840,7 +15856,16 @@ worker-rpc@^0.1.0:
1584015856
dependencies:
1584115857
microevent.ts "~0.1.1"
1584215858

15843-
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
15859+
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
15860+
version "7.0.0"
15861+
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
15862+
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
15863+
dependencies:
15864+
ansi-styles "^4.0.0"
15865+
string-width "^4.1.0"
15866+
strip-ansi "^6.0.0"
15867+
15868+
wrap-ansi@^7.0.0:
1584415869
version "7.0.0"
1584515870
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
1584615871
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==

0 commit comments

Comments
 (0)