Skip to content

fix 3711 anyOf/oneOf remove old fields from previous schema#3714

Closed
cwendtxealth wants to merge 2 commits into
rjsf-team:mainfrom
cwendtxealth:fixes-3711-anyOf-oneOf-remove-old-fields
Closed

fix 3711 anyOf/oneOf remove old fields from previous schema#3714
cwendtxealth wants to merge 2 commits into
rjsf-team:mainfrom
cwendtxealth:fixes-3711-anyOf-oneOf-remove-old-fields

Conversation

@cwendtxealth

@cwendtxealth cwendtxealth commented Jun 1, 2023

Copy link
Copy Markdown
Contributor

Reasons for making this change

Fixes #3711 - removing old undefined fields when switching to different oneOf/anyOf schema

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

});

newFormData = {
...data,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@cwendtxealth These changes are very similar to #3443 (comment). In that PR I requested the developer to ensure that none of the other issues were regressed. Are you willing to do that legwork?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I should be able to validate those issues if they have reproducible steps

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just from some initial testing it does look like removing some undefined fields causes some issues with how getDefaultFormState fills out formData. Might need to look at a different place to remove the undefined fields.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@cwendtxealth I'm on vacation for a week starting today. My thoughts are that, perhaps we add another flag to the experimental_defaultFormStateBehavior that deals with this behavior somehow.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we talk about this at one of the Friday meetings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That sounds good!

@linpc

linpc commented Aug 25, 2023

Copy link
Copy Markdown

Hello, any update on this fix? Thanks!

@heath-freenome

Copy link
Copy Markdown
Member

@linpc This is a very tricky problem that @cwendtxealth hasn't quite figured out how to fix yet.

@michal-kurz

Copy link
Copy Markdown
Contributor

Hey @cwendtxealth, @heath-freenome , any predictions on when this might get merged? No pressure, but our workaround for this is giving us some performance issues, so we need to decide if it's worth optimizing.

@fdallmeier

Copy link
Copy Markdown

Hi @michal-kurz , would you mind sharing your workaround?

@Bonfims

Bonfims commented Apr 24, 2024

Copy link
Copy Markdown

thank you all, just a doubt: it will work on conditionally remove a field with "if" without anyOf or oneOf?

@UsainBloot

Copy link
Copy Markdown

Thanks for contributing this, I believe it would fix my issue raised in #4366

@heath-freenome

Copy link
Copy Markdown
Member

@cwendtxealth Any chance you can figure out 1) whether this PR is still needed given all the changes and features added to v6? and 2) if so, how can we help you get it over the line?

@heath-freenome

Copy link
Copy Markdown
Member

The referenced bug was fixed by another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

anyOf/oneOf keeping undefined property of old option

7 participants