Skip to content
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.

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

Fix incorrect allOf merging (fix #2923) (Reimplement #3025) #3227

Merged
merged 6 commits into from Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Expand Up @@ -17,12 +17,13 @@ should change the heading of the (upcoming) version to include a major version b
-->
# 5.0.0-beta-15

# @rjsf/core
## @rjsf/core
- Pass the `schema` along to the `ArrayFieldItemTemplate` as part of the fix for [#3253](https://github.com/rjsf-team/react-jsonschema-form/issues/3253)
- Tweak Babel configuration to emit ES5-compatible output files, fixing [#3240](https://github.com/rjsf-team/react-jsonschema-form/issues/3240)

# @rjsf/utils
## @rjsf/utils
- Update the `ArrayFieldItemTemplate` to add `schema` as part of the fix for [#3253](https://github.com/rjsf-team/react-jsonschema-form/issues/3253)
- Fix improper merging of nested `allOf`s ([#3025](https://github.com/rjsf-team/react-jsonschema-form/pull/3025), [#3227](https://github.com/rjsf-team/react-jsonschema-form/pull/3227))
nickgros marked this conversation as resolved.
Show resolved Hide resolved

## Dev / docs / playground
- Fixed the documentation for `ArrayFieldItemTemplate` as part of the fix for [#3253](https://github.com/rjsf-team/react-jsonschema-form/issues/3253)
Expand Down
3 changes: 2 additions & 1 deletion packages/utils/src/schema/getDefaultFormState.ts
Expand Up @@ -96,13 +96,14 @@ export function computeDefaults<
S extends StrictRJSFSchema = RJSFSchema
>(
validator: ValidatorType<T, S>,
schema: S,
rawSchema: S,
parentDefaults?: T,
rootSchema: S = {} as S,
rawFormData?: T,
includeUndefinedValues: boolean | "excludeObjectChildren" = false
): T | T[] | undefined {
const formData = isObject(rawFormData) ? rawFormData : {};
let schema: S = isObject(rawSchema) ? rawSchema : ({} as S);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test that passes an invalid schema and expects a specific error message. The test fails without this change because a string ends up getting passed here as rawSchema, and we don't get to the error case that the test does expect.

The best fix would be to actually validate the schema before we get this far, but that would be a large change that would require a lot of effort.

// Compute the defaults recursively: give highest priority to deepest nodes.
let defaults: T | T[] | undefined = parentDefaults;
if (isObject(defaults) && isObject(schema.default)) {
Expand Down
36 changes: 4 additions & 32 deletions packages/utils/src/schema/retrieveSchema.ts
@@ -1,6 +1,6 @@
import get from "lodash/get";
import set from "lodash/set";
import mergeAllOf from "json-schema-merge-allof";
import mergeAllOf, { Options } from "json-schema-merge-allof";

import {
ADDITIONAL_PROPERTIES_KEY,
Expand Down Expand Up @@ -242,40 +242,12 @@ export default function retrieveSchema<
}

const formData: GenericObjectType = rawFormData || {};
// For each level of the dependency, we need to recursively determine the appropriate resolved schema given the current state of formData.
// Otherwise, nested allOf subschemas will not be correctly displayed.
if (resolvedSchema.properties) {
const properties: GenericObjectType = {};

Object.entries(resolvedSchema.properties).forEach((entries) => {
const propName = entries[0];
const propSchema = entries[1] as S;
const rawPropData = formData[propName];
const propData = isObject(rawPropData) ? rawPropData : {};
const resolvedPropSchema = retrieveSchema<T, S>(
validator,
propSchema,
rootSchema,
propData
);

properties[propName] = resolvedPropSchema;

if (
propSchema !== resolvedPropSchema &&
resolvedSchema.properties !== properties
) {
resolvedSchema = { ...resolvedSchema, properties };
}
});
}

if (ALL_OF_KEY in schema) {
try {
resolvedSchema = mergeAllOf({
...resolvedSchema,
allOf: resolvedSchema.allOf,
}) as S;
resolvedSchema = mergeAllOf(resolvedSchema, {
deep: false,
} as Options) as S;
} catch (e) {
console.warn("could not merge subschemas in allOf:\n" + e);
const { allOf, ...resolvedSchemaWithoutAllOf } = resolvedSchema;
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/schema/toPathSchema.ts
Expand Up @@ -54,7 +54,7 @@ export default function toPathSchema<

if (
ADDITIONAL_PROPERTIES_KEY in schema &&
schema[ADDITIONAL_PROPERTIES_KEY] === true
schema[ADDITIONAL_PROPERTIES_KEY] !== false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use the path schema to decide which objects to drop when we use omitExtraData. For some reason, a change here uncovered a case where the additionalProperty fields would be dropped by omitExtraData + liveOmit if the additionalProperties schema was an object. This fixes that regression.

) {
set(pathSchema, RJSF_ADDITONAL_PROPERTIES_FLAG, true);
}
Expand Down
18 changes: 18 additions & 0 deletions packages/utils/test/schema/getDefaultFormStateTest.ts
Expand Up @@ -151,6 +151,24 @@ export default function getDefaultFormStateTest(
requiredProperty: "foo",
});
});
it("test computeDefaults handles an invalid property schema", () => {
const schema: RJSFSchema = {
type: "object",
properties: {
invalidProperty: "not a valid property value",
},
} as RJSFSchema;
expect(
computeDefaults(
testValidator,
schema,
undefined,
schema,
undefined,
"excludeObjectChildren"
)
).toEqual({});
});
});
describe("root default", () => {
it("should map root schema default to form state, if any", () => {
Expand Down
52 changes: 45 additions & 7 deletions packages/utils/test/schema/retrieveSchemaTest.ts
Expand Up @@ -1205,14 +1205,52 @@ export default function retrieveSchemaTest(testValidator: TestValidatorType) {
title: "Breed name",
type: "string",
},
Spots: {
default: "small",
enum: ["large", "small"],
title: "Spots",
type: "string",
},
},
required: ["BreedName", "Spots"],
allOf: [
{
if: {
required: ["BreedName"],
properties: {
BreedName: {
const: "Alsatian",
},
},
},
then: {
properties: {
Fur: {
default: "brown",
enum: ["black", "brown"],
title: "Fur",
type: "string",
},
},
required: ["Fur"],
},
},
{
if: {
required: ["BreedName"],
properties: {
BreedName: {
const: "Dalmation",
},
},
},
then: {
properties: {
Spots: {
default: "small",
enum: ["large", "small"],
title: "Spots",
type: "string",
},
},
required: ["Spots"],
},
},
],
required: ["BreedName"],
title: "Breed",
},
},
Expand Down