Skip to content

Commit

Permalink
fix: fixing the merging of anyOf/oneOf schemas (#3767)
Browse files Browse the repository at this point in the history
Improved the merging of anyOf/oneOf schemas using the `mergeSchemas()` function instead of object structuring
- In `@rjsf/utils` improved the merging of anyOf/oneOf schemas as follows:
  - Updated `getDefaultFormState()` and `retrieveSchema()` functions to use `mergeSchemas()`
- In `@rjsf/core`, updated `MultiSchemaField` to use `mergeSchemas()`
- Updated the tests accordingly, adding new tests as needed
- Updated the `CHANGELOG.md` accordingly
  • Loading branch information
heath-freenome committed Jul 14, 2023
1 parent ea36b6e commit 2a8b4ca
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 13 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ should change the heading of the (upcoming) version to include a major version b
-->
# 5.10.1

## @rjsf/core

- Updated `MultiSchemaField` to use `mergeSchema()` for merging in the remaining schema for `anyOf`/`oneOf`

## @rjsf/utils

- Updated `retrieveSchemaInternal()` to always resolve allOf schema without merging when `expandAllBranches` is set, fixing compiled schema issue always throwing error with `mergeAllOf`
- Updated `getDefaultFormState()` to use `mergeSchema()` for merging in the remaining schema for `anyOf`/`oneOf`
- Updated `retrieveSchema()` to use `mergeSchema()` for merging in the remaining schema for `anyOf`/`oneOf`

# 5.10.0

Expand All @@ -40,7 +46,6 @@ should change the heading of the (upcoming) version to include a major version b
- Updated the `custom-widgets-fields` documentation to add the new added behaviour of `getFieldComponent()` function. [#3740](https://github.com/rjsf-team/react-jsonschema-form/pull/3740)
- Updated the `playground` to add an example of the new added behaviour of `getFieldComponent()` function. [#3740](https://github.com/rjsf-team/react-jsonschema-form/pull/3740)


# 5.9.0

## @rjsf/utils
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/components/fields/MultiSchemaField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getDiscriminatorFieldFromSchema,
getUiOptions,
getWidget,
mergeSchemas,
RJSFSchema,
StrictRJSFSchema,
TranslatableString,
Expand Down Expand Up @@ -174,7 +175,7 @@ class AnyOfField<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends For
const { oneOf, anyOf, ...remaining } = schema;
// Merge in all the non-oneOf/anyOf properties and also skip the special ADDITIONAL_PROPERTY_FLAG property
unset(remaining, ADDITIONAL_PROPERTY_FLAG);
optionSchema = !isEmpty(remaining) ? { ...remaining, ...option } : option;
optionSchema = !isEmpty(remaining) ? (mergeSchemas(remaining, option) as S) : option;
}

const translateEnum: TranslatableString = title
Expand Down
44 changes: 41 additions & 3 deletions packages/core/test/anyOf.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,48 @@ describe('anyOf', () => {
expect(node.querySelectorAll('select')).to.have.length.of(0);
});

it('should render a select element if the anyOf keyword is present', () => {
it('should render a select element if the anyOf keyword is present, merges top level required', () => {
const schema = {
type: 'object',
title: 'Merges into anyOf',
required: ['baz'],
properties: {
baz: { type: 'number' },
},
anyOf: [
{
properties: {
foo: { type: 'string' },
},
},
{
properties: {
bar: { type: 'string' },
},
},
],
};

const { node } = createFormComponent({
schema,
});

console.log(node.innerHTML);

expect(node.querySelectorAll('select')).to.have.length.of(1);
expect(node.querySelector('select').id).eql('root__anyof_select');
expect(node.querySelectorAll('span.required')).to.have.length.of(2);
});

it('should render a select element if the anyOf keyword is present, merges top level and anyOf required', () => {
const schema = {
type: 'object',
required: ['baz'],
properties: {
baz: { type: 'number' },
},
anyOf: [
{
required: ['foo'],
properties: {
foo: { type: 'string' },
},
Expand All @@ -53,9 +89,11 @@ describe('anyOf', () => {
schema,
});

expect(node.querySelector('legend#root__title').innerHTML).eql(schema.title);
console.log(node.innerHTML);

expect(node.querySelectorAll('select')).to.have.length.of(1);
expect(node.querySelector('select').id).eql('root__anyof_select');
expect(node.querySelectorAll('span.required')).to.have.length.of(3);
});

it('should render a root select element with default value', () => {
Expand Down
44 changes: 41 additions & 3 deletions packages/core/test/oneOf.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,48 @@ describe('oneOf', () => {
expect(node.querySelectorAll('select')).to.have.length.of(0);
});

it('should render a select element if the oneOf keyword is present', () => {
it('should render a select element if the oneOf keyword is present, merges top level required', () => {
const schema = {
type: 'object',
title: 'Merges into oneOf',
required: ['baz'],
properties: {
baz: { type: 'number' },
},
oneOf: [
{
properties: {
foo: { type: 'string' },
},
},
{
properties: {
bar: { type: 'string' },
},
},
],
};

const { node } = createFormComponent({
schema,
});

console.log(node.innerHTML);

expect(node.querySelectorAll('select')).to.have.length.of(1);
expect(node.querySelector('select').id).eql('root__oneof_select');
expect(node.querySelectorAll('span.required')).to.have.length.of(2);
});

it('should render a select element if the oneOf keyword is present, merges top level and oneOf required', () => {
const schema = {
type: 'object',
required: ['baz'],
properties: {
baz: { type: 'number' },
},
oneOf: [
{
required: ['foo'],
properties: {
foo: { type: 'string' },
},
Expand All @@ -54,9 +90,11 @@ describe('oneOf', () => {
schema,
});

expect(node.querySelector('legend#root__title').innerHTML).eql(schema.title);
console.log(node.innerHTML);

expect(node.querySelectorAll('select')).to.have.length.of(1);
expect(node.querySelector('select').id).eql('root__oneof_select');
expect(node.querySelectorAll('span.required')).to.have.length.of(3);
});

it('should assign a default value and set defaults on option change', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/utils/src/schema/getDefaultFormState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import isObject from '../isObject';
import isFixedItems from '../isFixedItems';
import mergeDefaultsWithFormData from '../mergeDefaultsWithFormData';
import mergeObjects from '../mergeObjects';
import mergeSchemas from '../mergeSchemas';
import {
Experimental_DefaultFormStateBehavior,
FormContextType,
Expand Down Expand Up @@ -214,7 +215,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema
discriminator
)
] as S;
schemaToCompute = { ...remaining, ...schemaToCompute };
schemaToCompute = mergeSchemas(remaining, schemaToCompute) as S;
} else if (ANY_OF_KEY in schema) {
const { anyOf, ...remaining } = schema;
if (anyOf!.length === 0) {
Expand All @@ -231,7 +232,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema
discriminator
)
] as S;
schemaToCompute = { ...remaining, ...schemaToCompute };
schemaToCompute = mergeSchemas(remaining, schemaToCompute) as S;
}

if (schemaToCompute) {
Expand Down
4 changes: 2 additions & 2 deletions packages/utils/src/schema/retrieveSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,9 @@ export function resolveAnyOrOneOfSchemas<
// Call this to trigger the set of isValid() calls that the schema parser will need
const option = getFirstMatchingOption<T, S, F>(validator, formData, anyOrOneOf, rootSchema, discriminator);
if (expandAllBranches) {
return anyOrOneOf.map((item) => ({ ...remaining, ...item }));
return anyOrOneOf.map((item) => mergeSchemas(remaining, item) as S);
}
schema = { ...remaining, ...anyOrOneOf[option] } as S;
schema = mergeSchemas(remaining, anyOrOneOf[option]) as S;
}
return [schema];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,9 @@ Object {
"type": "number",
},
},
"required": Array [
"more",
],
"type": "object",
},
"choice2": Object {
Expand Down
60 changes: 60 additions & 0 deletions packages/utils/test/schema/getDefaultFormStateTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1529,6 +1529,36 @@ export default function getDefaultFormStateTest(testValidator: TestValidatorType
},
});
});
it('should populate nested default values merging required fields', () => {
const schema: RJSFSchema = {
type: 'object',
required: ['foo', 'bar'],
properties: {
foo: {
type: 'string',
default: 'fooVal',
},
},
oneOf: [
{
properties: {
bar: {
type: 'number',
},
baz: {
default: 'bazIsRequired',
},
},
required: ['baz'],
},
],
};
expect(
getDefaultFormState(testValidator, schema, {}, undefined, undefined, {
emptyObjectFields: 'populateRequiredDefaults',
})
).toEqual({ foo: 'fooVal', baz: 'bazIsRequired' });
});
it('should populate defaults for oneOf + dependencies', () => {
const schema: RJSFSchema = {
oneOf: [
Expand Down Expand Up @@ -1696,6 +1726,36 @@ export default function getDefaultFormStateTest(testValidator: TestValidatorType
},
});
});
it('should populate nested default values merging required fields', () => {
const schema: RJSFSchema = {
type: 'object',
required: ['foo', 'bar'],
properties: {
foo: {
type: 'string',
default: 'fooVal',
},
},
anyOf: [
{
properties: {
bar: {
type: 'number',
},
baz: {
default: 'bazIsRequired',
},
},
required: ['baz'],
},
],
};
expect(
getDefaultFormState(testValidator, schema, {}, undefined, undefined, {
emptyObjectFields: 'populateRequiredDefaults',
})
).toEqual({ foo: 'fooVal', baz: 'bazIsRequired' });
});
it('should populate defaults for anyOf + dependencies', () => {
const schema: RJSFSchema = {
anyOf: [
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/test/schema/retrieveSchemaTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,7 @@ export default function retrieveSchemaTest(testValidator: TestValidatorType) {
expect(resolveAnyOrOneOfSchemas(testValidator, oneOfSchema, SUPER_SCHEMA, true)).toEqual([
{
...(SUPER_SCHEMA.definitions?.choice1 as RJSFSchema),
required: ['choice'],
required: ['choice', 'more'],
},
{
...(SUPER_SCHEMA.definitions?.choice2 as RJSFSchema),
Expand Down
1 change: 1 addition & 0 deletions packages/utils/test/testUtils/testData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ export const SUPER_SCHEMA: RJSFSchema = {
},
choice1: {
type: 'object',
required: ['more'],
properties: {
choice: {
type: 'string',
Expand Down

0 comments on commit 2a8b4ca

Please sign in to comment.