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

Required property inside of allOf doesn't seem to be populated with populateAllDefaults #3832

Closed
1 task done
MarekBodingerBA opened this issue Aug 16, 2023 · 3 comments
Closed
1 task done
Labels
any-one-all-of Related to fixing anyOf, oneOf or allOf bug defaults help wanted

Comments

@MarekBodingerBA
Copy link
Contributor

MarekBodingerBA commented Aug 16, 2023

Prerequisites

What theme are you using?

core

What is your question?

Hello,

in this case, I cannot find a reason why the property fourth is not populated. I had similar examples running correctly, but in this case I am not able to say what's the reason and our form logic heavily relies on this.

Here is the code sample with the output:
https://runkit.com/marekbodingerba/64dd19e7e6504a0008fcb575

Playground link

Thank you for looking at it, it might be something simple that I am missing.

Marek

@MarekBodingerBA MarekBodingerBA added needs triage Initial label given, to be assigned correct labels and assigned question labels Aug 16, 2023
@nickgros
Copy link
Contributor

This definitely looks like a bug, thanks for the report @MarekBodingerBA . I think this is a duplicate of #3599. Unfortunately, current maintainers' time is limited so we could use any help we could get on this issues. Would you be willing to investigate?

@nickgros nickgros added bug help wanted defaults any-one-all-of Related to fixing anyOf, oneOf or allOf and removed question needs triage Initial label given, to be assigned correct labels and assigned labels Aug 18, 2023
@MarekBodingerBA
Copy link
Contributor Author

I've managed to find the underlying issue.

I've set up multiple console logs through the code.
1:

(console logs "theSchema" before retrieval)
2:
const schema = retrieveSchema<T, S, F>(validator, theSchema, rootSchema, formData);
(console logs "schema" after retrieval)
3:
const objectDefaults = Object.keys(schema.properties || {}).reduce((acc: GenericObjectType, key: string) => {
(console logs "schema" when object type is detected in "computeDefaults").

For the correctly working schema:

{
    type: 'object',
    allOf: [
        {
            type: 'object',
            properties: {
                bar: {
                    type: 'object',
                },
            },
            required: ['bar'],
        },
    ],
};

it console logs:

// 1
{
  "type": "object",
  "allOf": [
    {
      "type": "object",
      "properties": {
        "bar": {
          "type": "object"
        }
      },
      "required": [
        "bar"
      ]
    }
  ]
}
// 2
{
  "type": "object",
  "required": [
    "bar"
  ],
  "properties": {
    "bar": {
      "type": "object"
    }
  }
}
// 3
{
  "type": "object",
  "required": [
    "bar"
  ],
  "properties": {
    "bar": {
      "type": "object"
    }
  }
}
// 3
{
  "type": "object"
}

// Result
{ bar: {} }

As we can see, all allOf subschemas are at the root and are resolved at the first step (by retrieveSchema). Therefore it works correctly.

If we nest the subschema containing allOf deeper:

{
    type: 'object',
    properties: {
        foo: {
            type: 'object',
            allOf: [
                {
                    type: 'object',
                    properties: {
                        bar: {
                            type: 'object',
                        },
                    },
                    required: ['bar'],
                },
            ],
        },
    },
    required: ['foo'],
};
// 1
{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "allOf": [
        {
          "type": "object",
          "properties": {
            "bar": {
              "type": "object"
            }
          },
          "required": [
            "bar"
          ]
        }
      ]
    }
  },
  "required": [
    "foo"
  ]
}
// 2
{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "allOf": [
        {
          "type": "object",
          "properties": {
            "bar": {
              "type": "object"
            }
          },
          "required": [
            "bar"
          ]
        }
      ]
    }
  },
  "required": [
    "foo"
  ]
}
// 3
{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "allOf": [
        {
          "type": "object",
          "properties": {
            "bar": {
              "type": "object"
            }
          },
          "required": [
            "bar"
          ]
        }
      ]
    }
  },
  "required": [
    "foo"
  ]
}
// 3
{
  "type": "object",
  "allOf": [
    {
      "type": "object",
      "properties": {
        "bar": {
          "type": "object"
        }
      },
      "required": [
        "bar"
      ]
    }
  ]
}

// Result
{ foo: {} }

We can see the deeply nested allOfs are not resolved (probably correct) as the output of 1 and 2 is the same. Then, computeDefaults is called recursively. The last output of 3 shows that it contains the allOf which is ignored as the inner procedure of the switch condition only works with properties and additionalProperties, but not with allOf.

As an easy fix, I tried to call retrieveSchema where 3rd console log is and use the retrieved one. It seems to be working with my use case but fails to resolve the references in #3599 (however, I cannot assure I've implemented it correctly). Another possibility might be to implement if (ALL_OF_KEY in schema) in computeDefaults, but I am not sure about the implications of doing this.

What do you think @nickgros?

MarekBodingerBA pushed a commit to bratislava/react-jsonschema-form that referenced this issue Oct 10, 2023
@MarekBodingerBA
Copy link
Contributor Author

Fixed in #3892.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
any-one-all-of Related to fixing anyOf, oneOf or allOf bug defaults help wanted
Projects
None yet
Development

No branches or pull requests

2 participants