Chore: Adopt SJSF omitExtraData algorithm#5065
Conversation
|
@x0k Per your suggestion, I've adapted your code to this library. Feel free to take a look |
| new RegExp(pattern), | ||
| schemaDef as S | boolean, | ||
| ]); | ||
| const knownProperties = new Set(Object.keys(properties ?? {})); |
There was a problem hiding this comment.
Removing getKnownProperties will result in bar being preserved in the following case (if this was intentional, it would make sense to add this test to the test suite)
const schema: RJSFSchema = {
type: 'object',
oneOf: [
{
properties: {
kind: { const: 'a' },
foo: { type: 'string' },
},
},
{
properties: {
kind: { const: 'b' },
bar: { type: 'string' },
},
},
],
additionalProperties: {
type: 'string',
},
};
const value = {
kind: 'a',
bar: 'hello',
extra: 'keep me',
};
const result = omitExtraData(testValidator, schema, schema, value);
expect(result).toEqual({
kind: 'a',
bar: 'hello',
extra: 'keep me',
});| if (isObject(value)) { | ||
| return Object.values(value as GenericObjectType).every(isValueEmpty); | ||
| } |
There was a problem hiding this comment.
This function should not be recursive, or setProperty should call a different function. Since omitExtraData performs a depth-first traversal, empty objects could be collapsed when returning the result from omit, for example. It may even make sense to integrate all isValueEmpty logic directly into omit.
There was a problem hiding this comment.
Updated setProperty() per your recommendation
| const options = (oneOf as Array<S | boolean>).map((d) => { | ||
| if (!isSchemaObj(d)) { | ||
| return (d ? {} : { not: {} }) as S; | ||
| } | ||
| return d.additionalProperties === false ? ({ ...d, additionalProperties: true } as S) : d; | ||
| }); |
There was a problem hiding this comment.
Oh, it looks like this will not work with precompiled validators.
There was a problem hiding this comment.
This is the same limitation that already exists elsewhere in the codebase (e.g. MultiSchemaField) when getClosestMatchingOption is used with dynamic schemas
I wasn’t aware of this. Perhaps precompile module (usage) could also be adapted at some point.
There was a problem hiding this comment.
@heath-freenome did the prior omitExtraData handle (or not handle) oneOf matching in a way that worked for precompiled validators? Does this represent a functional regression?
| } | ||
| } | ||
|
|
||
| if (additionalProperties !== undefined && additionalProperties !== false) { |
There was a problem hiding this comment.
If the additionalProperties schema is not explicitly specified, its value is treated as false, which differs from the behavior defined by JSON Schema. It might be worth adding a note about this peculiarity.
Adopted the `omitExtraData()` processing algorithm from `svelte-jsonschema-form`, adding in the `removeOptionalEmptyObject()` behavior - In `@rjsf/utils`: - Rewrote `omitExtraData()` basing it on the algorithm from `SJSF`, with improvements to pick up the `removeOptionalEmptyObject()` behavior - Refactored the `isValueEmpty()` function from `removeOptionalEmptyObject()` for use by `omitExtraData()` exporting it from utils - Deprecated the `removeOptionalEmptyObject()` and `toPathSchema()` functions, `toPathSchema()` on `SchemaUtilsType` and the `PathSchema` type - Updated the tests for `toPathSchema()` to add missing coverage after the rewrite - Updated the tests for `omitExtraData()` to cover `isValueEmpty()`, the ported `removeOptionalEmptyObject()` behavior and keeping coverage 100% - In `@rjsf/core`: - Updated `Form` to stop using `removeOptionalEmptyObjects()` and deprecating `removeEmptyOptionalObjects` prop. - Added tests for `Form` to cover the remove empty objects cases - Updated the `form-props.md` and `utility-functions.md` to document the deprecations - Updated the `uiSchema.md` to improve the `enumNames` documents slightly - Updated the `CHANGELOG.md` accordingly
heath-freenome
force-pushed
the
adopt-SJSF-omitExtraData-algorithm
branch
from
42a648e to
cb4235a
Compare
3 participants
Reasons for making this change
Adopted the
omitExtraData()processing algorithm fromsvelte-jsonschema-form, adding in theremoveOptionalEmptyObject()behavior@rjsf/utils:omitExtraData()basing it on the algorithm fromSJSF, with improvements to pick up theremoveOptionalEmptyObject()behaviorisValueEmpty()function fromremoveOptionalEmptyObject()for use byomitExtraData()exporting it from utilsremoveOptionalEmptyObject()andtoPathSchema()functions,toPathSchema()onSchemaUtilsTypeand thePathSchematypetoPathSchema()to add missing coverage after the rewriteomitExtraData()to coverisValueEmpty(), the portedremoveOptionalEmptyObject()behavior and keeping coverage 100%@rjsf/core:Formto stop usingremoveOptionalEmptyObjects()and deprecatingremoveEmptyOptionalObjectsprop.Formto cover the remove empty objects casesform-props.mdandutility-functions.mdto document the deprecationsuiSchema.mdto improve theenumNamesdocuments slightlyCHANGELOG.mdaccordinglyChecklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.