-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feature - replaced IdSchema with FieldPathId #4787
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
Conversation
In order to support an upcoming feature as well as eliminate a performance issue, `IdSchema` was replace with `FieldPathId` - In @rjsf/utils: - Added new `FieldPathList` and `FieldPathId` types and `DEFAULT_ID_PREFIX` and `DEFAULT_ID_SEPARATOR` to `constants.ts` - Added the new `toFieldPathId()` function to generate `FieldPathId`s, exporting it from the library - BREAKING CHANGES - Removed the `IdSchema` type, replacing `idSchema: IdSchema<T>` in all types with `fieldPathId: FieldPathId` - Updated the `idGenerators` to replace `id: IdSchema<T> | string` with `id: FieldPathId | string` removing the need for the `<T = any>` generic on the functions - Removed the `toIdSchema()` function in the `schema` directory - Updated the `SchemaUtilsType` and `createSchemaUtils()` to remove the `toIdSchema()` function - Deleted the `ui:rootFieldId` from the `UiSchema` since `idPrefix` does the same exact thing - Updated the tests accordingly - In `@rjsf/antd`, `@rjsf/chakra-ui`, `@rjsf/fluent-ui`, `@rjsf/mantine`, `@rjsf/mui`, `@rjsf/primereact`, `@rjsf/react-bootstrap`, `@rjsf/semantic-ui` and `@rjsf/shadcn`: - BREAKING CHANGES - Updated all of the templates and widgets to change `idSchema` to `fieldPathId` or to remove the `<T>` off of the idGenerator functions - In @rjsf/core: - BREAKING CHANGES - Updated all of the fields, templates and widgets to change `idSchema` to `fieldPathId` or to remove the `<T>` off of the idGenerator functions - `ObjectField` and `ArrayField` to use `toFieldPathId` instead of `toIdSchema()` to generate the `fieldPathId`s of all its children - Updated the `onChange` handling of fields to make `path` required and either pass it straight through, or use the `fieldPathId.path` instead of using an empty array or appending path information - Updated `Form` to use `toFieldPathId()` to generate `fieldPathId` instead of `idSchema`, always providing the `idPrefix` and `idSeparator` in the `globalFormOptions` and make the `path: FieldPathList` required - Updated `LayoutGridField` to remove the `IdSchema` related code in favor of `FieldPathId` code - Also exported the `getTestRegistry()` function from the main `index.ts` to assist developers in creating `registry` object for tests - Updated all of the test to deal with the `idSchema` -> `fieldPathId` changes - In @rjsf/daisyui: - BREAKING CHANGES - Updated all of the templates and widgets to change `idSchema` to `fieldPathId` or to remove the `<T>` off of the idGenerator functions - Also fixed the `FieldTemplate` to extract the `description` element so that it was not spread onto the `div`, fixing the snapshots - In @rjsf/validator-ajv8: - Updated the test to no longer try to test the delete `toIdSchema` function - In docs: - Updated `custom-templates.md`, `custom-widgets-fields.md` and `layout-grid.md` to change the `idSchema` documentation to `fieldPathId` - Updated `uiSchema.md` to remove the `ui:rootFieldId` documentation - Updated `utility-functions.md` delete `toIdSchema()`, add `toFieldPathId()` and to remove the `<T>` from the id generator functions - Updated `v6.x upgrade guide.md` to document all the breaking changes made in this release - Updated the `CHANGELOG.md` accordingly
e2c246a
to
cc5b4f9
Compare
displayLabel, | ||
classNames, | ||
// Destructure props we don't want to pass to div | ||
description, |
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.
nice subtle fix, the only snapshot changes are related to this (so the ID Schema replacement is clean! 🙌 )
uiSchema: { ...gridFormUISchema, [UI_GLOBAL_OPTIONS_KEY]: globalUiOptions }, | ||
formData: {}, | ||
errorSchema: { employment: {} }, | ||
// IdSchema is weirdly recursive and it's easier to just ignore the error |
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.
not anymore!
idPrefix: props.idPrefix || DEFAULT_ID_PREFIX, | ||
idSeparator: props.idSeparator || DEFAULT_ID_SEPARATOR, |
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.
Are we able to call this.getDefaultRegistry().globalFormOptions
here to get the prefix/separator?
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.
No, it blew things up because this is called in the constructor, before the this.props
has been setup properly.
unset(copiedFormData, key); | ||
// drop property will pass the name in `path` array | ||
onChange(copiedFormData, []); | ||
onChange(copiedFormData, fieldPathId.path); |
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.
Need to ensure no regression in #4763 since we didn't add a test for 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 manually tested additional properties and arrays in my local playground. All works
Reasons for making this change
In order to support an upcoming feature as well as eliminate a performance issue,
IdSchema
was replace withFieldPathId
FieldPathList
andFieldPathId
types andDEFAULT_ID_PREFIX
andDEFAULT_ID_SEPARATOR
toconstants.ts
toFieldPathId()
function to generateFieldPathId
s, exporting it from the libraryIdSchema
type, replacingidSchema: IdSchema<T>
in all types withfieldPathId: FieldPathId
idGenerators
to replaceid: IdSchema<T> | string
withid: FieldPathId | string
removing the need for the<T = any>
generic on the functionstoIdSchema()
function in theschema
directorySchemaUtilsType
andcreateSchemaUtils()
to remove thetoIdSchema()
functionui:rootFieldId
from theUiSchema
sinceidPrefix
does the same exact thing@rjsf/antd
,@rjsf/chakra-ui
,@rjsf/fluent-ui
,@rjsf/mantine
,@rjsf/mui
,@rjsf/primereact
,@rjsf/react-bootstrap
,@rjsf/semantic-ui
and@rjsf/shadcn
:idSchema
tofieldPathId
or to remove the<T>
off of the idGenerator functionsidSchema
tofieldPathId
or to remove the<T>
off of the idGenerator functionsObjectField
andArrayField
to usetoFieldPathId
instead oftoIdSchema()
to generate thefieldPathId
s of all its childrenonChange
handling of fields to makepath
required and either pass it straight through, or use thefieldPathId.path
instead of using an empty array or appending path informationForm
to usetoFieldPathId()
to generatefieldPathId
instead ofidSchema
, always providing theidPrefix
andidSeparator
in theglobalFormOptions
and make thepath: FieldPathList
requiredLayoutGridField
to remove theIdSchema
related code in favor ofFieldPathId
codegetTestRegistry()
function from the mainindex.ts
to assist developers in creatingregistry
object for testsidSchema
->fieldPathId
changesidSchema
tofieldPathId
or to remove the<T>
off of the idGenerator functionsFieldTemplate
to extract thedescription
element so that it was not spread onto thediv
, fixing the snapshotstoIdSchema
functioncustom-templates.md
,custom-widgets-fields.md
andlayout-grid.md
to change theidSchema
documentation tofieldPathId
uiSchema.md
to remove theui:rootFieldId
documentationutility-functions.md
deletetoIdSchema()
, addtoFieldPathId()
and to remove the<T>
from the id generator functionsv6.x upgrade guide.md
to document all the breaking changes made in this releaseCHANGELOG.md
accordinglyChecklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update
to update snapshots, if needed.