-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feature - Prep work for new feature in 4736 #4776
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
Feature - Prep work for new feature in 4736 #4776
Conversation
… of properties Fixed rjsf-team#4772 by always genering the outer grid - Updated `chakra-ui`'s `ObjectFieldTemplate` to render the outer `Grid` regardless of the number of properties - Updated the `CHANGELOG.md` accordingly
const { keyedFormData } = this.state; | ||
const fieldTitle = schema.title || title || name; | ||
const { schemaUtils, formContext } = registry; | ||
const { schemaUtils, formContext, globalFormOptions = {} } = registry; |
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 doesn't look like it would be a problem in any of the components in this change, but setting the default value to {}
here would create a new object every render. It could cause performance issues if it later got passed to a hook dependency array, which could happen in a future change or in a custom field.
Maybe Form.getDefaultRegistry
should create one empty object instead and pass it around everywhere?
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'll go with that
The `customValidate` prop requires a function that specifies custom validation rules for the form. | ||
See [Validation](../usage/validation.md) for more information. | ||
|
||
## experimental_componentUpdateStrategy |
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.
Thanks for adding this
Breaking change refactoring in preparation for the feature in 4736 - In `@rjsf/utils`: - Added new `GlobalFormOptions`, refactoring the `experimental_componentUpdateStrategy` from `Registry` and `idPrefix` & `idSeparator` from `FieldProps` - Replaced the `experimental_componentUpdateStrategy` prop in `Registry` with `readonly globalFormOptions?: GlobalFormOptions` - Updated `FieldProps` to remove `idPrefix` and `idSeparator` - In `@rjsf/core`: - Updated `Form` to add the `globalFormOptions` to the registry if there are any `GlobalFormOptions` values provided - Updated `ArrayField`, `LayoutGridField`, `ObjectField` and `SchemaField` to get `idPrefix`, `idSeparator` from the `registry.globalFormOptions`, no longer passing them on `FieldProps` - Updated `SchemaField` to get `experimental_componentUpdateStrategy` from the `registry.globalFormOptions` as well - Updated the `SchemaField` tests as needed - In `@rjsf/daisy` updated the snapshots - Updated the `custom-widget-fields.md` and `v6.x upgrade guide.md` to document the refactor of the `idPrefix` and `idSeparator` refactor - Updated the `CHANGELOG.md` file accordingly
819c8bb
to
baab030
Compare
const globalFormOptions = JSON.parse( | ||
JSON.stringify({ idPrefix, idSeparator, experimental_componentUpdateStrategy }), | ||
); |
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.
This will not work if #4773 adds the nameGenerator
function prop since it couldn't be be stringified/parsed.
Could you use lodash omitBy
/isUndefined
?
const globalFormOptions = JSON.parse( | |
JSON.stringify({ idPrefix, idSeparator, experimental_componentUpdateStrategy }), | |
); | |
const globalFormOptions = omitBy({ idPrefix, idSeparator, experimental_componentUpdateStrategy }, isUndefined) |
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.
Let me try that!
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 went with isNil()
rather than isUndefined()
because we were already including it
Reasons for making this change
Breaking change refactoring in preparation for the feature in 4736
@rjsf/utils
:GlobalFormOptions
, refactoring theexperimental_componentUpdateStrategy
fromRegistry
andidPrefix
&idSeparator
fromFieldProps
experimental_componentUpdateStrategy
prop inRegistry
withreadonly globalFormOptions?: GlobalFormOptions
FieldProps
to removeidPrefix
andidSeparator
@rjsf/core
:Form
to add theglobalFormOptions
to the registry if there are anyGlobalFormOptions
values providedArrayField
,LayoutGridField
,ObjectField
andSchemaField
to getidPrefix
,idSeparator
from theregistry.globalFormOptions
, no longer passing them onFieldProps
SchemaField
to getexperimental_componentUpdateStrategy
from theregistry.globalFormOptions
as wellSchemaField
tests as needed@rjsf/daisy
updated the snapshotscustom-widget-fields.md
andv6.x upgrade guide.md
to document the refactor of theidPrefix
andidSeparator
refactorCHANGELOG.md
file accordinglyChecklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update
to update snapshots, if needed.