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

trim empty objects to allow correct validation of optional objects with mandatory fields #1228

Closed

Conversation

numical
Copy link

@numical numical commented Mar 19, 2019

Reasons for making this change

NOT FOR MERGE YET!
Test failures remain - see discussion below.

This PR offers a potential solution for the issue of allowing optional objects with required fields to pass validation when none of their values are defined.

The approach is similar to that discussed in Issue 675 and proposed, then withdrawn, in PR 682.

Prior to validation, a clone of formData is generated where all objects with no defined values are removed (note: empty arrays are not removed).

This works nicely - but changes the semantics of the errors. Primarily 'is a required property' errors become 'should be object'. Currently I have not changed the tests so as to display that.

I have raised this PR to essentially ask whether this approach could be used and, if so, to seek advice on what to about the changed errors (I note the conversation here).

If this is related to existing tickets, include links to them as well:
#675
#682

Checklist

  • I'm updating documentation
  • [X ] I'm adding or updating code
    • [ X] I've added and/or updated tests
    • I've updated docs if needed
    • [X ] I've run npm run cs-format on my branch to conform my code to prettier coding style
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@epicfaace
Copy link
Member

Hi @numical , thanks for the PR.

I see that the tests are failing due to the errors saying "should be object"; however, when I try to replicate this error on my local playground, I still get the expected error, "is a required property". Do you know why this is the case?

image

Also, your changes work, but if HTML5 validation is enabled, then I still get the "required" dialog. Do you think there might be a way to programmatically change this in this PR as well (to conditionally add the required attribute)?

image

@epicfaace
Copy link
Member

@numical just checking on this.

@Morriz
Copy link

Morriz commented Jan 21, 2022

any progress here? It would be reallllly beneficial to not get errors on empty nested props with required values

azertyfun added a commit to azertyfun/react-jsonschema-form that referenced this pull request Jun 10, 2022
When a nested value is also undefined, it would go up the trimming chain
and return undefined instead of {}.
azertyfun added a commit to azertyfun/react-jsonschema-form that referenced this pull request Jun 13, 2022
When a nested value is also undefined, it would go up the trimming chain
and return undefined instead of {}.
@heath-freenome heath-freenome added the v5 refactor Needs refactor due to v5 breaking changes label Jan 8, 2023
@heath-freenome
Copy link
Member

The issue attempted to be fixed by this PR was actually fixed in #3436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response v5 refactor Needs refactor due to v5 breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants