-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix 3759 - uneditable & permanent defaults with additional properties #4490
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Heath C <51679588+heath-freenome@users.noreply.github.com>
@Piotr-Debicki I hope all is well... Still waiting for you to update your PR to avoid breaking API changes |
@heath-freenome Hey, sorry! I got caught up with work and this completely slipped my mind. I'll revisit this when I get some free time this week |
@Piotr-Debicki Hey, seems you didn't get the time you were hoping for. Any chance you can get it done by the end of September? |
@heath-freenome Hey, sorry (again). I unfortunately had a bunch of stuff added to my plate at once, but I should be able to finish this up by then, yeah |
@heath-freenome Hey, finally got to updating the PR. Let me know if you need any other changes here. Wasn't sure about versioning, so I just kept the convention + .01 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Piotr-Debicki Thanks for updating your PR... I hope you can respond to this feedback before we release v6 in less than a month.
- [includeUndefinedValues=false]: boolean | "excludeObjectChildren" - Optional flag, if true, cause undefined values to be added as defaults. If "excludeObjectChildren", cause undefined values for this object and pass `includeUndefinedValues` as false when computing defaults for any nested object properties. | ||
- [experimental_defaultFormStateBehavior]: Experimental_DefaultFormStateBehavior - See `Form` documentation for the [experimental_defaultFormStateBehavior](./form-props.md#experimental_defaultFormStateBehavior) prop | ||
- [experimental_customMergeAllOf]: Experimental_CustomMergeAllOf<S> - See `Form` documentation for the [experimental_customMergeAllOf](./form-props.md#experimental_custommergeallof) prop | ||
- [initialDefaultsGenerated]: boolean - Optional flag, indicates whether or not initial defaults have been generated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also document this new feature in the v6.x upgrade guide.md
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can delete this file as we've removed validator-ajv6
from V6. did you need to make any changes to the validator-ajv8
instead?
## @rjsf/validator-ajv6 | ||
|
||
- Updated `getDefaultFormState` calls to reflect addition of `initialDefaultsGenerated` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this since we deleted that package in v6.
|
||
## @rjsf/validator-ajv8 | ||
|
||
- Updated `getDefaultFormState` calls to reflect addition of `initialDefaultsGenerated` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't actually see any changes for this validator in your PR
Reasons for making this change
Fixes #3759
Additional properties with defaults set currently creates a permanent key-value pair that is essentially uneditable.
Trying to edit the key creates an entirely new key value entry on blur while still keeping the default key-value pair. The default key-value pair is also not deletable. Solution here generates defaults for additional properties once on form initialization, and then skips regeneration on any form update
This solution works for root & nested additionalProperties (see tests)
Allows for 'reset' form to regenerate additionalProperties defaults
Also fixes the issue I mentioned in the Closed PR with custom formData not being applied when generating additionalProperties defaults
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update
to update snapshots, if needed.