Rewrite mergeSchemas to fix schema dependencies merging (rjsf-team#1476)

* rewrite mergeSchemas to fix duplicate error * fix: don't include duplicate values when merging "required" fields * fix: merge required attribute only for object types * fix logic for calculating required keyword

Author: epicfaace

src/utils.js

@@ -2,6 +2,7 @@ import React from "react";
 import * as ReactIs from "react-is";
 import fill from "core-js/library/fn/array/fill";
 import validateFormData, { isValid } from "./validate";
+import { union } from "lodash";
 
 export const ADDITIONAL_PROPERTY_FLAG = "__additional_property";
 
@@ -66,6 +67,7 @@ export function getDefaultRegistry() {
  };
 }
 
+/* Gets the type of a given schema. */
 export function getSchemaType(schema) {
  let { type } = schema;
 
@@ -760,8 +762,34 @@ function withExactlyOneSubschema(
  );
 }
 
-function mergeSchemas(schema1, schema2) {
- return mergeObjects(schema1, schema2, true);
+// Recursively merge deeply nested schemas.
+// The difference between mergeSchemas and mergeObjects
+// is that mergeSchemas only concats arrays for
+// values under the "required" keyword, and when it does,
+// it doesn't include duplicate values.
+export function mergeSchemas(obj1, obj2) {
+ var acc = Object.assign({}, obj1); // Prevent mutation of source object.
+ return Object.keys(obj2).reduce((acc, key) => {
+ const left = obj1 ? obj1[key] : {},
+ right = obj2[key];
+ if (obj1 && obj1.hasOwnProperty(key) && isObject(right)) {
+ acc[key] = mergeSchemas(left, right);
+ } else if (
+ obj1 &&
+ obj2 &&
+ (getSchemaType(obj1) === "object" || getSchemaType(obj2) === "object") &&
+ key === "required" &&
+ Array.isArray(left) &&
+ Array.isArray(right)
+ ) {
+ // Don't include duplicate values when merging
+ // "required" fields.
+ acc[key] = union(left, right);
+ } else {
+ acc[key] = right;
+ }
+ return acc;
+ }, acc);
 }
 
 function isArguments(object) {

test/Form_test.js

@@ -2325,7 +2325,44 @@ describeRepeated("Form common", createFormComponent => {
  });
  });
 
- describe("Dependency defaults", () => {
+ describe("Dependencies", () => {
+ it("should not give a validation error by duplicating enum values in dependencies", () => {
+ const schema = {
+ title: "A registration form",
+ description: "A simple form example.",
+ type: "object",
+ properties: {
+ type1: {
+ type: "string",
+ title: "Type 1",
+ enum: ["FOO", "BAR", "BAZ"],
+ },
+ type2: {
+ type: "string",
+ title: "Type 2",
+ enum: ["GREEN", "BLUE", "RED"],
+ },
+ },
+ dependencies: {
+ type1: {
+ properties: {
+ type1: {
+ enum: ["FOO"],
+ },
+ type2: {
+ enum: ["GREEN"],
+ },
+ },
+ },
+ },
+ };
+ const formData = {
+ type1: "FOO",
+ };
+ const { node, comp } = createFormComponent({ schema, formData });
+ Simulate.submit(node);
+ expect(comp.state.errors).to.have.length.of(0);
+ });
  it("should show dependency defaults for uncontrolled components", () => {
  const schema = {
  type: "object",

test/utils_test.js

@@ -23,6 +23,7 @@ import {
  toIdSchema,
  toPathSchema,
  guessType,
+ mergeSchemas,
 } from "../src/utils";
 
 describe("utils", () => {
@@ -1058,6 +1059,123 @@ describe("utils", () => {
  });
  });
 
+ describe("mergeSchemas()", () => {
+ it("shouldn't mutate the provided objects", () => {
+ const obj1 = { a: 1 };
+ mergeSchemas(obj1, { b: 2 });
+ expect(obj1).eql({ a: 1 });
+ });
+
+ it("should merge two one-level deep objects", () => {
+ expect(mergeSchemas({ a: 1 }, { b: 2 })).eql({ a: 1, b: 2 });
+ });
+
+ it("should override the first object with the values from the second", () => {
+ expect(mergeSchemas({ a: 1 }, { a: 2 })).eql({ a: 2 });
+ });
+
+ it("should override non-existing values of the first object with the values from the second", () => {
+ expect(mergeSchemas({ a: { b: undefined } }, { a: { b: { c: 1 } } })).eql(
+ { a: { b: { c: 1 } } }
+ );
+ });
+
+ it("should recursively merge deeply nested objects", () => {
+ const obj1 = {
+ a: 1,
+ b: {
+ c: 3,
+ d: [1, 2, 3],
+ e: { f: { g: 1 } },
+ },
+ c: 2,
+ };
+ const obj2 = {
+ a: 1,
+ b: {
+ d: [3, 2, 1],
+ e: { f: { h: 2 } },
+ g: 1,
+ },
+ c: 3,
+ };
+ const expected = {
+ a: 1,
+ b: {
+ c: 3,
+ d: [3, 2, 1],
+ e: { f: { g: 1, h: 2 } },
+ g: 1,
+ },
+ c: 3,
+ };
+ expect(mergeSchemas(obj1, obj2)).eql(expected);
+ });
+
+ it("should recursively merge File objects", () => {
+ const file = new File(["test"], "test.txt");
+ const obj1 = {
+ a: {},
+ };
+ const obj2 = {
+ a: file,
+ };
+ expect(mergeSchemas(obj1, obj2).a).instanceOf(File);
+ });
+
+ describe("arrays", () => {
+ it("should not concat arrays", () => {
+ const obj1 = { a: [1] };
+ const obj2 = { a: [2] };
+
+ expect(mergeSchemas(obj1, obj2)).eql({ a: [2] });
+ });
+
+ it("should concat arrays under 'required' keyword", () => {
+ const obj1 = { type: "object", required: [1] };
+ const obj2 = { type: "object", required: [2] };
+
+ expect(mergeSchemas(obj1, obj2)).eql({
+ type: "object",
+ required: [1, 2],
+ });
+ });
+
+ it("should concat arrays under 'required' keyword when one of the schemas is an object type", () => {
+ const obj1 = { type: "object", required: [1] };
+ const obj2 = { required: [2] };
+
+ expect(mergeSchemas(obj1, obj2)).eql({
+ type: "object",
+ required: [1, 2],
+ });
+ });
+
+ it("should concat nested arrays under 'required' keyword", () => {
+ const obj1 = { a: { type: "object", required: [1] } };
+ const obj2 = { a: { type: "object", required: [2] } };
+
+ expect(mergeSchemas(obj1, obj2)).eql({
+ a: { type: "object", required: [1, 2] },
+ });
+ });
+
+ it("should not include duplicate values when concatting arrays under 'required' keyword", () => {
+ const obj1 = { type: "object", required: [1] };
+ const obj2 = { type: "object", required: [1] };
+
+ expect(mergeSchemas(obj1, obj2)).eql({ type: "object", required: [1] });
+ });
+
+ it("should not concat arrays under 'required' keyword that are not under an object type", () => {
+ const obj1 = { required: [1] };
+ const obj2 = { required: [2] };
+
+ expect(mergeSchemas(obj1, obj2)).eql({ required: [2] });
+ });
+ });
+ });
+
  describe("retrieveSchema()", () => {
  it("should 'resolve' a schema which contains definitions", () => {
  const schema = { $ref: "#/definitions/address" };
@@ -1320,6 +1438,35 @@ describe("utils", () => {
  required: ["a", "b"],
  });
  });
+ it("should not concat enum properties, but should concat 'required' properties", () => {
+ const schema = {
+ type: "object",
+ properties: {
+ a: { type: "string", enum: ["FOO", "BAR", "BAZ"] },
+ b: { type: "string", enum: ["GREEN", "BLUE", "RED"] },
+ },
+ required: ["a"],
+ dependencies: {
+ a: {
+ properties: {
+ a: { enum: ["FOO"] },
+ b: { enum: ["BLUE"] },
+ },
+ required: ["a", "b"],
+ },
+ },
+ };
+ const definitions = {};
+ const formData = { a: "FOO" };
+ expect(retrieveSchema(schema, definitions, formData)).eql({
+ type: "object",
+ properties: {
+ a: { type: "string", enum: ["FOO"] },
+ b: { type: "string", enum: ["BLUE"] },
+ },
+ required: ["a", "b"],
+ });
+ });
  });
 
  describe("with $ref in dependency", () => {