-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add the ability to parse defaults from allOf
schema
#3969
Conversation
experimental_defaultFormStateBehavior?.allOf === 'populateDefaults' && ALL_OF_KEY in schema | ||
? retrieveSchema<T, S, F>(validator, schema, rootSchema, formData) | ||
: schema; | ||
const objectDefaults = Object.keys(retrievedSchema.properties || {}).reduce( |
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 forget if it is possible for allOf
to be in something other than the object type?
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.
@benjdlambert Do you know whether this is possible? I can imagine if it is, then this solution may be incomplete
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.
Hmm, so I guess you mean something like this right?
const schema = { anyOf: [{ type: 'string' }, { type: 'number' }] }
Or do you have an example schema in mind that I can test with?
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.
That's an anyOf
. I'm guessing that you probably haven't seen an JSON Schema that has an allOf
that isn't part of an object
type?
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.
Anyway, this is better than what we had before, so I'm going to approve
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.
Yeah, actually i'm not sure. I've never seen one, not sure if there's any other datatypes where it would be possible to use allOf
like that. Happy to provide a followup if there are!
Can you rebase and update the |
anyOf
schemaallOf
schema
@heath-freenome updated 🎉 |
CHANGELOG.md
Outdated
## @rjsf/utils | ||
|
||
- Added an experimental flag `allOf` to `experimental_defaultFormStateBehavior` for populating defaults when using `allOf` schemas [#3969](https://github.com/rjsf-team/react-jsonschema-form/pull/3969) | ||
|
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 move this after @rjsf/fluentui-rc
since we like to keep the packages in alphabetical order. And there are also conflicts due to another merged PR. Finally, it seems like you did a major reformat of the file (your IDE?). Can you revert all of those changes to previous versions?
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.
Woops, yep I thought I had saved without formatting for that file. Updated and made the changes 🎉
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.
@heath-freenome not sure about the build failures here, seemed to have rebased but they look unrelated?
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.
@zxbodya Could this be related to your recent PR?
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.
it indeed looks to be because of project references misconfiguration -- added fix here #3982
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.
@benjdlambert I just realized that you are adding a new feature here. I just released 5.14.3, so can you make your changes be in version 5.15.0 instead?
CHANGELOG.md
Outdated
- update tsconfigs: | ||
- `"importHelpers": false` to remove need for tslib dependency [#3958](https://github.com/rjsf-team/react-jsonschema-form/issues/3958) | ||
- increase compilation target level from es6 to es2018 (so there are no need for transpiling object spread/rest feature) | ||
- add missing typescript project reference for `snapshot-tests` in a root tsconfig, update it to also use es modules | ||
- Added a dropdown for chaning the `experimental_defaultFormStateBehavior.allOf` behaviour in the playground |
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.
- Added a dropdown for chaning the `experimental_defaultFormStateBehavior.allOf` behaviour in the playground | |
- Added a dropdown for changing the `experimental_defaultFormStateBehavior.allOf` behaviour in the playground |
222a9dd
to
73899d2
Compare
@benjdlambert sorry for the extra work... Can you resolve |
@heath-freenome no worries, done! 🎉 |
* feat: support default values in anyOf format * feat: add `skipDefaults` and `populateDefaults` options for `allOf` parsing * docs: add examples * chore: ammending chanelog
Reasons for making this change
Fixes #3892.
This adds an opt in config option to be able to derive the default values from
allOf
schema including theif-then-else
syntax. Followed the implementation options listed in the PR. Maybe it would make sense to follow the same config types asarrayMinItems
and haveallOf.populate
as thedefaultFormOptions
, but will leave that up to the reviewer.Checklist
npm run test:update
to update snapshots, if needed.