Skip to content

Commit 4b48add

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 4b48add

File tree

3 files changed

+103
-10
lines changed

3 files changed

+103
-10
lines changed

‎lib/src/schema.js‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,32 @@ 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(
32+
Reflect.ownKeys(foo)
33+
.map((key) => (isDefined(foo[key]) ? key : undefined))
34+
.filter(isDefined),
35+
);
36+
const barKeys = new Set(
37+
Reflect.ownKeys(bar)
38+
.map((key) => (isDefined(bar[key]) ? key : undefined))
39+
.filter(isDefined),
40+
);
41+
if (fooKeys.size !== barKeys.size) {
42+
return false;
43+
}
44+
if ([...fooKeys].some((fooKey) => !barKeys.has(fooKey))) {
45+
return false;
46+
}
47+
return [...fooKeys].every((fooKey) => foo[fooKey] === bar[fooKey]);
48+
}
49+
2450
export function accumulateValues(
2551
obj,
2652
schema,
@@ -76,7 +102,7 @@ export function accumulateValues(
76102
}
77103
return schema.defaultValue;
78104
}
79-
// FIXME: should we instead use (return nullable ? null : undefined)?
105+
// TODO: should we instead use (return nullable ? null : undefined)?
80106
return [];
81107
}
82108
return values;
@@ -216,7 +242,6 @@ export function accumulateErrors(
216242
return error;
217243
}
218244

219-
// FIXME: only copy oldErrors when a change is detected
220245
export function accumulateDifferentialErrors(
221246
oldObj,
222247
newObj,
@@ -279,8 +304,6 @@ export function accumulateDifferentialErrors(
279304
const localMember = member(e.new);
280305
const index = keySelector(e.new);
281306

282-
// console.warn(index, depsChanged);
283-
284307
const fieldError = accumulateDifferentialErrors(
285308
e.old,
286309
e.new,
@@ -296,6 +319,9 @@ export function accumulateDifferentialErrors(
296319
}
297320
});
298321

322+
if (isObjectReasonablyEqual(oldError, errors)) {
323+
return oldError;
324+
}
299325
return hasNoKeys(errors) ? undefined : errors;
300326
}
301327

@@ -324,8 +350,6 @@ export function accumulateDifferentialErrors(
324350
|| (depsChanged && isDefined(dependenciesObj?.[fieldName]))
325351
);
326352

327-
// console.warn(fieldName, depsChangedForField, dependenciesObj?.[fieldName]);
328-
329353
const fieldError = accumulateDifferentialErrors(
330354
oldObj?.[fieldName],
331355
newObj?.[fieldName],
@@ -341,6 +365,9 @@ export function accumulateDifferentialErrors(
341365
}
342366
});
343367

368+
if (isObjectReasonablyEqual(oldError, errors)) {
369+
return oldError;
370+
}
344371
return hasNoKeys(errors) ? undefined : errors;
345372
}
346373

@@ -397,7 +424,7 @@ export function analyzeErrors(errors) {
397424
}
398425

399426
// FIXME: move this to a helper
400-
// FIXME: check conditions for change in context and baseValue
427+
// TODO: check conditions for change in context and baseValue
401428
export function addCondition(
402429
schema,
403430
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)