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

oneOf doesn't work with objects #3742

Closed
4 tasks done
gabor-meresz-epam opened this issue Jun 26, 2023 · 20 comments
Closed
4 tasks done

oneOf doesn't work with objects #3742

gabor-meresz-epam opened this issue Jun 26, 2023 · 20 comments
Labels
awaiting response bug possibly close To confirm if this issue can be closed

Comments

@gabor-meresz-epam
Copy link

Prerequisites

What theme are you using?

core

Version

5.x

Current Behavior

The form shows wrong error messages in case the schema contains oneOf with objects inside and the user changes between the available options.

The unexpected errors:

  • must NOT have additional properties
  • must match exactly one schema in oneOf

Expected Behavior

The form should work fine with all kinds of oneOf schemas.

Steps To Reproduce

  1. Go to Playground
  2. Enable Live Validation
  3. Select 'Option 2'
  4. Select 'Option 1'

Environment

RJSF playground

Anything else?

Screenshot 2023-06-26 at 9 18 22

@gabor-meresz-epam gabor-meresz-epam added bug needs triage Initial label given, to be assigned correct labels and assigned labels Jun 26, 2023
@heath-freenome
Copy link
Member

heath-freenome commented Jun 30, 2023

@gabor-meresz-epam Even AJV believes that this is not a valid schema. Have you tried something like this playground instead?

@heath-freenome heath-freenome added awaiting response and removed needs triage Initial label given, to be assigned correct labels and assigned labels Jun 30, 2023
@tamas-grosz-epam
Copy link

tamas-grosz-epam commented Jul 11, 2023

Hi @heath-freenome , sorry I get 404 on the link you provided
https://rjsf-team.github.io/react-jsonschema-form/

actually, we got that if the oneOf contains an object, then after changing the Option - the validation fails always

@gabor-meresz-epam
Copy link
Author

@gabor-meresz-epam Even AJV believes that this is not a valid schema. Have you tried something like this playground instead?

Sorry the link you provided is broken (404)

The schema looks valid for me, I tried out with https://www.jsonschemavalidator.net/
Screenshot 2023-07-11 at 14 12 47

@gabor-meresz-epam
Copy link
Author

@heath-freenome did you have a chance to check the replies?

@heath-freenome
Copy link
Member

@gabor-meresz-epam Sorry, it's been a busy week at work. I'm trying to figure out what the example I sent was and why the link is broken

@heath-freenome
Copy link
Member

Here's the proper Playground link

@tamas-grosz-epam
Copy link

hi, @heath-freenome thanks for the new link, it's working now - and it does not give error, but :)
if both branches of oneOf are objects (I added "type": "object" to "Adam" in your example), then pressed Reset, then Validate (it's ok now), then changed the option - and now pressing Validate again brings the error

@heath-freenome
Copy link
Member

hi, @heath-freenome thanks for the new link, it's working now - and it does not give error, but :) if both branches of oneOf are objects (I added "type": "object" to "Adam" in your example), then pressed Reset, then Validate (it's ok now), then changed the option - and now pressing Validate again brings the error

Can you share the playground link for the situation you are describing? You can use the Share button and paste what is put into your clipboard

@tamas-grosz-epam
Copy link

tamas-grosz-epam commented Jul 24, 2023

hi, @heath-freenome , sure - but it's really only that one line
link
It behaves a bit strange: if you play with clicking on reset, validate and changing the option - then sometimes it says ok, sometimes gives errors. To me it seems either a cache like issue, or like the validator wouldn't fall back from the wrong branch.
And is the discriminator always necessary? Couldn't the validator differentiate between branches by their simple content?

@heath-freenome
Copy link
Member

The discriminator provides a performance increase with the oneOf computations

@tamas-grosz-epam
Copy link

hi, @heath-freenome, back to the original issue: could you reproduce the error from the playground link provided?

@heath-freenome
Copy link
Member

@tamas-grosz-epam I could when I removed the trainer prop from the formData. If you change the object fields default behavior then things work fine. See this link

@tamas-grosz-epam
Copy link

Hi, @heath-freenome - maybe I didn't notice something, but your link seems to be the same as the first one.
The problem is when both branches are objects, like in the link I gave. I paste it again:
link
It behaves a bit strange: if you play with clicking on reset, validate and changing the option - then sometimes it says ok, sometimes gives errors. To me it seems either a cache like issue, or like the validator wouldn't fall back from the wrong branch.

@heath-freenome
Copy link
Member

The link is different, not in the schema, but in the options, specifically the Object fields default behavior

@tamas-grosz-epam
Copy link

Hi, sorry for the late reaction. We introduced and tested your idea, it works. This issue may be closed, still I'd prefer to keep it and convert to show that the setting is incompatible with the oneOf part

@heath-freenome
Copy link
Member

@tamas-grosz-epam I don't understand your comment. Perhaps you could either update this issue or write a new one with a more specific cause/effect bug for the incompatible oneOf part?

@tamas-grosz-epam
Copy link

With your suggestion to change Object fields default behavior option - it works. But without changing it, it does not - that's what I mean incompatible

@heath-freenome
Copy link
Member

Ah... Well we provide the options on Form to change the various behaviors of RJSF. If using one of these behavior changes solves your problem, then I believe the issue is solved. As for it being incompatible with a different option setting, I think that is to be expected, No?

@heath-freenome heath-freenome added the possibly close To confirm if this issue can be closed label Oct 16, 2023
@tamas-grosz-epam
Copy link

Yeah, I'd flag this issue solved and add a line in documentation that if oneOf is needed, then use only the working option

Copy link

stale bot commented Nov 16, 2023

This issue was closed because of lack of recent activity. Reopen if you still need assistance.

@stale stale bot closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response bug possibly close To confirm if this issue can be closed
Projects
None yet
Development

No branches or pull requests

3 participants