Skip to content

Add optimization so error object reference does not change #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add optimization so error object reference does not change #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions lib/src/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -216,7 +242,6 @@ export function accumulateErrors(
return error;
}

// FIXME: only copy oldErrors when a change is detected
export function accumulateDifferentialErrors(
oldObj,
newObj,
Expand Down Expand Up @@ -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,
Expand All @@ -296,6 +319,9 @@ export function accumulateDifferentialErrors(
}
});

if (isObjectReasonablyEqual(oldError, errors)) {
return oldError;
}
return hasNoKeys(errors) ? undefined : errors;
}

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

// console.warn(fieldName, depsChangedForField, dependenciesObj?.[fieldName]);

const fieldError = accumulateDifferentialErrors(
oldObj?.[fieldName],
newObj?.[fieldName],
Expand All @@ -341,6 +365,9 @@ export function accumulateDifferentialErrors(
}
});

if (isObjectReasonablyEqual(oldError, errors)) {
return oldError;
}
return hasNoKeys(errors) ? undefined : errors;
}

Expand Down Expand Up @@ -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,
Expand Down
41 changes: 41 additions & 0 deletions lib/src/schemaAccumulateDifferentialErrors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,3 +622,44 @@ test('accumulate differential errors with nested conditional', () => {
},
});
});

test('accumulate differential errors with nested conditional and reference preserve', () => {
const oldValue: PartialForm<FormType> = {
name: 'Hari Bahadur',
email: '[email protected]',
clients: [
{
clientId: '1',
strength: -10,
},
{
clientId: '2',
strength: -10,
},
],
};
const newValue: PartialForm<FormType> = {
...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);
});
31 changes: 28 additions & 3 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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==
Expand Down Expand Up @@ -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==
Expand All @@ -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"
Expand Down Expand Up @@ -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==
Expand Down
Loading