Skip to content

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

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.

Sign up for GitHub

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

Open

cwendtxealth wants to merge 2 commits into rjsf-team:main

from cwendtxealth:fixes-3711-anyOf-oneOf-remove-old-fields

Open

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

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

+11 −3

Conversation

Copy link

Contributor

@cwendtxealth cwendtxealth commented Jun 1, 2023 •

edited

Loading

Reasons for making this change

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

Checklist

I'm updating documentation
- I've checked the rendering of the Markdown text I've added
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

cwendtxealth and others added 2 commits

June 1, 2023 10:56


 fix 3711 anyOf/oneOf remove old fields from previous schema

bfedb77


 Merge branch 'main' into fixes-3711-anyOf-oneOf-remove-old-fields

cd6d326

heath-freenome reviewed

View reviewed changes

packages/utils/src/schema/sanitizeDataForNewSchema.ts

});

newFormData = {

...data,

Copy link

Member

@heath-freenome heath-freenome Jun 1, 2023

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

Contributor Author

@cwendtxealth cwendtxealth Jun 1, 2023

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

Contributor Author

@cwendtxealth cwendtxealth Jun 3, 2023

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

Member

@heath-freenome heath-freenome Jun 7, 2023

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

Member

@heath-freenome heath-freenome Aug 4, 2023

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

Contributor Author

@cwendtxealth cwendtxealth Aug 4, 2023

Choose a reason for hiding this comment

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

That sounds good!

Copy link

linpc commented Aug 25, 2023

Hello, any update on this fix? Thanks!

@heath-freenome

Copy link

Member

heath-freenome commented Aug 29, 2023

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

@nagy-nabil nagy-nabil mentioned this pull request

is RJSF with zod-to-json-schema/typebox good fit for us? nilefy/nilefy#172

Merged

3 tasks

Copy link

Contributor

michal-kurz commented Feb 8, 2024

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.

Copy link

fdallmeier commented Mar 28, 2024

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

Copy link

Bonfims commented Apr 24, 2024

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

@UsainBloot UsainBloot mentioned this pull request

Unable to add new item for object with additionalProperties within anyOf / oneOf with liveOmit and omitExtraData #4366

Open

4 tasks

Copy link

UsainBloot commented Nov 18, 2024

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

@heath-freenome

Copy link

Member

heath-freenome commented Oct 8, 2025

@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?

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

Labels

None yet

7 participants

@cwendtxealth @linpc @heath-freenome @michal-kurz @fdallmeier @Bonfims @UsainBloot