From 4b48addf5a630e4e8d42c96919181504b958934f Mon Sep 17 00:00:00 2001 From: tnagorra Date: Tue, 30 Jul 2024 11:47:23 +0545 Subject: [PATCH] Add optimization so error object reference does not change - Error object was recreated even if value change did not change the error object --- lib/src/schema.js | 41 +++++++++++++++---- ...schemaAccumulateDifferentialErrors.test.ts | 41 +++++++++++++++++++ yarn.lock | 31 ++++++++++++-- 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/lib/src/schema.js b/lib/src/schema.js index 6693cca..c051b99 100644 --- a/lib/src/schema.js +++ b/lib/src/schema.js @@ -21,6 +21,32 @@ import { hasNoValues, } from './utils'; +function isObjectReasonablyEqual(foo, bar) { + if (isNotDefined(foo) && isNotDefined(bar)) { + return true; + } + if (isNotDefined(foo) || isNotDefined(bar)) { + return false; + } + const fooKeys = new Set( + Reflect.ownKeys(foo) + .map((key) => (isDefined(foo[key]) ? key : undefined)) + .filter(isDefined), + ); + const barKeys = new Set( + Reflect.ownKeys(bar) + .map((key) => (isDefined(bar[key]) ? key : undefined)) + .filter(isDefined), + ); + if (fooKeys.size !== barKeys.size) { + return false; + } + if ([...fooKeys].some((fooKey) => !barKeys.has(fooKey))) { + return false; + } + return [...fooKeys].every((fooKey) => foo[fooKey] === bar[fooKey]); +} + export function accumulateValues( obj, schema, @@ -76,7 +102,7 @@ export function accumulateValues( } return schema.defaultValue; } - // FIXME: should we instead use (return nullable ? null : undefined)? + // TODO: should we instead use (return nullable ? null : undefined)? return []; } return values; @@ -216,7 +242,6 @@ export function accumulateErrors( return error; } -// FIXME: only copy oldErrors when a change is detected export function accumulateDifferentialErrors( oldObj, newObj, @@ -279,8 +304,6 @@ export function accumulateDifferentialErrors( const localMember = member(e.new); const index = keySelector(e.new); - // console.warn(index, depsChanged); - const fieldError = accumulateDifferentialErrors( e.old, e.new, @@ -296,6 +319,9 @@ export function accumulateDifferentialErrors( } }); + if (isObjectReasonablyEqual(oldError, errors)) { + return oldError; + } return hasNoKeys(errors) ? undefined : errors; } @@ -324,8 +350,6 @@ export function accumulateDifferentialErrors( || (depsChanged && isDefined(dependenciesObj?.[fieldName])) ); - // console.warn(fieldName, depsChangedForField, dependenciesObj?.[fieldName]); - const fieldError = accumulateDifferentialErrors( oldObj?.[fieldName], newObj?.[fieldName], @@ -341,6 +365,9 @@ export function accumulateDifferentialErrors( } }); + if (isObjectReasonablyEqual(oldError, errors)) { + return oldError; + } return hasNoKeys(errors) ? undefined : errors; } @@ -397,7 +424,7 @@ export function analyzeErrors(errors) { } // FIXME: move this to a helper -// FIXME: check conditions for change in context and baseValue +// TODO: check conditions for change in context and baseValue export function addCondition( schema, value, diff --git a/lib/src/schemaAccumulateDifferentialErrors.test.ts b/lib/src/schemaAccumulateDifferentialErrors.test.ts index 9964ba1..683918f 100644 --- a/lib/src/schemaAccumulateDifferentialErrors.test.ts +++ b/lib/src/schemaAccumulateDifferentialErrors.test.ts @@ -622,3 +622,44 @@ test('accumulate differential errors with nested conditional', () => { }, }); }); + +test('accumulate differential errors with nested conditional and reference preserve', () => { + const oldValue: PartialForm = { + name: 'Hari Bahadur', + email: 'hari.bahadur@gmail.com', + clients: [ + { + clientId: '1', + strength: -10, + }, + { + clientId: '2', + strength: -10, + }, + ], + }; + const newValue: PartialForm = { + ...oldValue, + name: 'Madan Krishna', + }; + const oldError = { + clients: { + 1: { + strength: 'The field must be greater than or equal to 0', + }, + 2: { + strength: 'The field must be greater than or equal to 0', + }, + }, + }; + const newError = accumulateDifferentialErrors( + oldValue, + newValue, + oldError, + errorFormTypeSchema, + undefined, + undefined, + ); + expect(newError).toStrictEqual(oldError); + expect(newError).toBe(oldError); +}); diff --git a/yarn.lock b/yarn.lock index d0d93fb..b07bd4a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14361,7 +14361,16 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"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: +"string-width-cjs@npm:string-width@^4.2.0": + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + +"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: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -14485,7 +14494,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -14499,6 +14508,13 @@ strip-ansi@^3.0.1: dependencies: ansi-regex "^2.0.0" +strip-ansi@^6.0.0, strip-ansi@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -15840,7 +15856,16 @@ worker-rpc@^0.1.0: dependencies: microevent.ts "~0.1.1" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + +wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==