-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Speed up ArrayInput #10926
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: next
Are you sure you want to change the base?
Speed up ArrayInput #10926
Changes from all commits
22d7e09
8a57a0c
7289cb0
39792b7
bcfceee
68129be
18cb162
8bbcec5
4fe3ee4
1eccc44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { useEffect } from 'react'; | ||
| import { useEffect, useRef } from 'react'; | ||
| import { | ||
| FieldValues, | ||
| UseFieldArrayReturn, | ||
|
|
@@ -36,12 +36,21 @@ export const useApplyInputDefaultValues = ({ | |
| const finalSource = useWrappedSource(source); | ||
|
|
||
| const record = useRecordContext(inputProps); | ||
| const { getValues, resetField, formState, reset } = useFormContext(); | ||
| const { getValues, resetField, reset, subscribe } = useFormContext(); | ||
| const recordValue = get(record, finalSource); | ||
| const formValue = get(getValues(), finalSource); | ||
| const { dirtyFields } = formState; | ||
| const isDirty = Object.keys(dirtyFields).includes(finalSource); | ||
| const isDirty = useRef<boolean | undefined>(undefined); | ||
|
|
||
| useEffect(() => { | ||
| return subscribe({ | ||
| formState: { dirtyFields: true }, | ||
| callback: ({ dirtyFields }) => { | ||
| isDirty.current = Object.keys(dirtyFields ?? {}).includes( | ||
| finalSource | ||
| ); | ||
| }, | ||
|
Comment on lines
+47
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we say in #10997 that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }); | ||
| }, [finalSource, subscribe]); | ||
| useEffect(() => { | ||
| if ( | ||
| defaultValue == null || | ||
|
|
@@ -52,7 +61,7 @@ export const useApplyInputDefaultValues = ({ | |
| // We check strictly for undefined to avoid setting default value | ||
| // when the field is null | ||
| recordValue !== undefined || | ||
| isDirty | ||
| isDirty.current === true | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,44 @@ export const ReadOnly = () => ( | |
| </TestMemoryRouter> | ||
| ); | ||
|
|
||
| export const DefaultValues = () => ( | ||
| <TestMemoryRouter initialEntries={['/books/1']}> | ||
| <Admin dataProvider={dataProvider}> | ||
| <Resource | ||
| name="books" | ||
| edit={() => { | ||
| return ( | ||
| <Edit | ||
| mutationMode="pessimistic" | ||
| mutationOptions={{ | ||
| onSuccess: data => { | ||
| console.log(data); | ||
| }, | ||
| }} | ||
| > | ||
| <SimpleForm> | ||
| <TextInput source="title" /> | ||
| <ArrayInput source="authors"> | ||
| <SimpleFormIterator> | ||
| <TextInput | ||
| source="name" | ||
| defaultValue="John Doe" | ||
| /> | ||
| <TextInput | ||
| source="role" | ||
| defaultValue="Author" | ||
| /> | ||
| </SimpleFormIterator> | ||
| </ArrayInput> | ||
| </SimpleForm> | ||
| </Edit> | ||
|
Comment on lines
+176
to
+206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found a weird issue while testing this story: However it seems this is already the case on So, this is not a regression from this PR, but this is something we should fix IMHO, either in this PR on in another one. Wdyt? |
||
| ); | ||
| }} | ||
| /> | ||
| </Admin> | ||
| </TestMemoryRouter> | ||
| ); | ||
|
|
||
| const BookEditWithAutocomplete = () => { | ||
| return ( | ||
| <Edit | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lastly, I tried building the EE packages with this version and ran into a TS issue. Still, I figured you may want to fix it in this PR, before the 5.13.0 is released. PR #10955 added a |
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'm pretty sure we already went through this issue, but apparently we did not cover it with a unit test (and apparently we should!): form validity has to be checked when the form is actually submitted. Checking it before triggering the debounced submission is not enough, because it doesn't work in the following scenario: