Skip to content

Commit 373f6d1

Browse files
jacobsfletchjacobsfletch
authored
fix(ui): nested fields disappear when manipulating rows in form state (#11906)
Continuation of #11867. When rendering custom fields nested within arrays or blocks, such as the Lexical rich text editor which is treated as a custom field, these fields will sometimes disappear when form state requests are invoked sequentially. This is especially reproducible on slow networks. This is different from the previous PR in that this issue is caused by adding _rows_ back-to-back, whereas the previous issue was caused when adding a single row followed by a change to another field. Here's a screen recording demonstrating the issue: https://github.com/user-attachments/assets/5ecfa9ec-b747-49ed-8618-df282e64519d The problem is that `requiresRender` is never sent in the form state request for row 2. This is because the [task queue](#11579) processes tasks within a single `useEffect`. This forces React to batch the results of these tasks into a single rendering cycle. So if request 1 sets state that request 2 relies on, request 2 will never use that state since they'll execute within the same lifecycle. Here's a play-by-play of the current behavior: 1. The "add row" event is dispatched a. This sets `requiresRender: true` in form state 1. A form state request is sent with `requiresRender: true` 1. While that request is processing, another "add row" event is dispatched a. This sets `requiresRender: true` in form state b. This adds a form state request into the queue 1. The initial form state request finishes a. This sets `requiresRender: false` in form state 1. The next form state request that was queued up in 3b is sent with `requiresRender: false` a. THIS IS EXPECTED, BUT SHOULD ACTUALLY BE `true`!! To fix this this, we need to ensure that the `requiresRender` property is persisted into the second request instead of overridden. To do this, we can add a new `serverPropsToIgnore` to form state which is read when the processing results from the server. So if `requiresRender` exists in `serverPropsToIgnore`, we do not merge it. This works because we actually mutate form state in between requests. So request 2 can read the results from request 1 without going through an additional rendering cycle. Here's a play-by-play of the fix: 1. The "add row" event is dispatched a. This sets `requiresRender: true` in form state b. This adds a task in the queue to mutate form state with `requiresRender: true` 1. A form state request is sent with `requiresRender: true` 1. While that request is processing, another "add row" event is dispatched a. This sets `requiresRender: true` in form state AND `serverPropsToIgnore: [ "requiresRender" ]` c. This adds a form state request into the queue 1. The initial form state request finishes a. This returns `requiresRender: false` from the form state endpoint BUT IS IGNORED 1. The next form state request that was queued up in 3c is sent with `requiresRender: true`
1 parent 329cd0b commit 373f6d1

File tree

13 files changed

+253
-42
lines changed

13 files changed

+253
-42
lines changed

‎packages/payload/src/admin/forms/Form.ts

+12
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ export type FieldState = {
4949
passesCondition?: boolean
5050
requiresRender?: boolean
5151
rows?: Row[]
52+
/**
53+
* The `serverPropsToIgnore` obj is used to prevent the various properties from being overridden across form state requests.
54+
* This can happen when queueing a form state request with `requiresRender: true` while the another is already processing.
55+
* For example:
56+
* 1. One "add row" action will set `requiresRender: true` and dispatch a form state request
57+
* 2. Another "add row" action will set `requiresRender: true` and queue a form state request
58+
* 3. The first request will return with `requiresRender: false`
59+
* 4. The second request will be dispatched with `requiresRender: false` but should be `true`
60+
* To fix this, only merge the `requiresRender` property if the previous state has not set it to `true`.
61+
* See the `mergeServerFormState` function for implementation details.
62+
*/
63+
serverPropsToIgnore?: Array<keyof FieldState>
5264
valid?: boolean
5365
validate?: Validate
5466
value?: unknown

‎packages/ui/src/fields/Array/index.tsx

+9-7
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const ArrayFieldComponent: ArrayFieldClientComponent = (props) => {
5858
const minRows = (minRowsProp ?? required) ? 1 : 0
5959

6060
const { setDocFieldPreferences } = useDocumentInfo()
61-
const { addFieldRow, dispatchFields, setModified } = useForm()
61+
const { addFieldRow, dispatchFields, moveFieldRow, removeFieldRow, setModified } = useForm()
6262
const submitted = useFormSubmitted()
6363
const { code: locale } = useLocale()
6464
const { i18n, t } = useTranslation()
@@ -153,18 +153,20 @@ export const ArrayFieldComponent: ArrayFieldClientComponent = (props) => {
153153

154154
const removeRow = useCallback(
155155
(rowIndex: number) => {
156-
dispatchFields({ type: 'REMOVE_ROW', path, rowIndex })
157-
setModified(true)
156+
removeFieldRow({ path, rowIndex })
158157
},
159-
[dispatchFields, path, setModified],
158+
[removeFieldRow, path],
160159
)
161160

162161
const moveRow = useCallback(
163162
(moveFromIndex: number, moveToIndex: number) => {
164-
dispatchFields({ type: 'MOVE_ROW', moveFromIndex, moveToIndex, path })
165-
setModified(true)
163+
moveFieldRow({
164+
moveFromIndex,
165+
moveToIndex,
166+
path,
167+
})
166168
},
167-
[dispatchFields, path, setModified],
169+
[path, moveFieldRow],
168170
)
169171

170172
const toggleCollapseAll = useCallback(

‎packages/ui/src/fields/Blocks/index.tsx

+7-9
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ const BlocksFieldComponent: BlocksFieldClientComponent = (props) => {
6060
const minRows = (minRowsProp ?? required) ? 1 : 0
6161

6262
const { setDocFieldPreferences } = useDocumentInfo()
63-
const { addFieldRow, dispatchFields, setModified } = useForm()
63+
const { addFieldRow, dispatchFields, moveFieldRow, removeFieldRow, setModified } = useForm()
6464
const { code: locale } = useLocale()
6565
const {
6666
config: { localization },
@@ -141,23 +141,19 @@ const BlocksFieldComponent: BlocksFieldClientComponent = (props) => {
141141

142142
const removeRow = useCallback(
143143
(rowIndex: number) => {
144-
dispatchFields({
145-
type: 'REMOVE_ROW',
144+
removeFieldRow({
146145
path,
147146
rowIndex,
148147
})
149-
150-
setModified(true)
151148
},
152-
[path, dispatchFields, setModified],
149+
[path, removeFieldRow],
153150
)
154151

155152
const moveRow = useCallback(
156153
(moveFromIndex: number, moveToIndex: number) => {
157-
dispatchFields({ type: 'MOVE_ROW', moveFromIndex, moveToIndex, path })
158-
setModified(true)
154+
moveFieldRow({ moveFromIndex, moveToIndex, path })
159155
},
160-
[dispatchFields, path, setModified],
156+
[moveFieldRow, path],
161157
)
162158

163159
const toggleCollapseAll = useCallback(
@@ -166,6 +162,7 @@ const BlocksFieldComponent: BlocksFieldClientComponent = (props) => {
166162
collapsed,
167163
rows,
168164
})
165+
169166
dispatchFields({ type: 'SET_ALL_ROWS_COLLAPSED', path, updatedRows })
170167
setDocFieldPreferences(path, { collapsed: collapsedIDs })
171168
},
@@ -179,6 +176,7 @@ const BlocksFieldComponent: BlocksFieldClientComponent = (props) => {
179176
rowID,
180177
rows,
181178
})
179+
182180
dispatchFields({ type: 'SET_ROW_COLLAPSED', path, updatedRows })
183181
setDocFieldPreferences(path, { collapsed: collapsedIDs })
184182
},

‎packages/ui/src/forms/Form/fieldReducer.ts

+42-3
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
6363
requiresRender: true,
6464
rows: withNewRow,
6565
value: siblingRows.length,
66+
...(state[path]?.requiresRender === true
67+
? {
68+
serverPropsToIgnore: [
69+
...(state[path]?.serverPropsToIgnore || []),
70+
'requiresRender',
71+
],
72+
}
73+
: state[path]?.serverPropsToIgnore || []),
6674
},
6775
}
6876

@@ -172,6 +180,14 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
172180
requiresRender: true,
173181
rows: rowsMetadata,
174182
value: rows.length,
183+
...(state[path]?.requiresRender === true
184+
? {
185+
serverPropsToIgnore: [
186+
...(state[path]?.serverPropsToIgnore || []),
187+
'requiresRender',
188+
],
189+
}
190+
: state[path]?.serverPropsToIgnore || ([] as any)),
175191
},
176192
}
177193

@@ -200,6 +216,14 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
200216
...state[path],
201217
requiresRender: true,
202218
rows: rowsWithinField,
219+
...(state[path]?.requiresRender === true
220+
? {
221+
serverPropsToIgnore: [
222+
...(state[path]?.serverPropsToIgnore || []),
223+
'requiresRender',
224+
],
225+
}
226+
: state[path]?.serverPropsToIgnore || ([] as any)),
203227
},
204228
}
205229

@@ -218,9 +242,8 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
218242

219243
const copyOfMovingLabel = customComponents.RowLabels[moveFromIndex]
220244

221-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
222245
customComponents.RowLabels.splice(moveFromIndex, 1)
223-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
246+
224247
customComponents.RowLabels.splice(moveToIndex, 0, copyOfMovingLabel)
225248

226249
newState[path].customComponents = customComponents
@@ -253,6 +276,14 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
253276
requiresRender: true,
254277
rows: rowsMetadata,
255278
value: rows.length,
279+
...(state[path]?.requiresRender === true
280+
? {
281+
serverPropsToIgnore: [
282+
...(state[path]?.serverPropsToIgnore || []),
283+
'requiresRender',
284+
],
285+
}
286+
: state[path]?.serverPropsToIgnore || []),
256287
},
257288
...flattenRows(path, rows),
258289
}
@@ -292,6 +323,14 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
292323
disableFormData: true,
293324
rows: rowsMetadata,
294325
value: siblingRows.length,
326+
...(state[path]?.requiresRender === true
327+
? {
328+
serverPropsToIgnore: [
329+
...(state[path]?.serverPropsToIgnore || []),
330+
'requiresRender',
331+
],
332+
}
333+
: state[path]?.serverPropsToIgnore || []),
295334
},
296335
}
297336

@@ -327,7 +366,7 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
327366
return newState
328367
}
329368

330-
//TODO: Remove this in 4.0 - this is a temporary fix to prevent a breaking change
369+
// TODO: Remove this in 4.0 - this is a temporary fix to prevent a breaking change
331370
if (action.sanitize) {
332371
for (const field of Object.values(action.state)) {
333372
if (field.valid !== false) {

‎packages/ui/src/forms/Form/index.tsx

+26-9
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,11 @@ export const Form: React.FC<FormProps> = (props) => {
121121

122122
const fieldsReducer = useReducer(fieldReducer, {}, () => initialState)
123123

124-
const [fields, dispatchFields] = fieldsReducer
124+
const [formState, dispatchFields] = fieldsReducer
125125

126-
contextRef.current.fields = fields
126+
contextRef.current.fields = formState
127127

128-
const prevFields = useRef(fields)
128+
const prevFormState = useRef(formState)
129129

130130
const validateForm = useCallback(async () => {
131131
const validatedFieldState = {}
@@ -611,9 +611,25 @@ export const Form: React.FC<FormProps> = (props) => {
611611
[dispatchFields, getDataByPath],
612612
)
613613

614+
const moveFieldRow: FormContextType['moveFieldRow'] = useCallback(
615+
({ moveFromIndex, moveToIndex, path }) => {
616+
dispatchFields({
617+
type: 'MOVE_ROW',
618+
moveFromIndex,
619+
moveToIndex,
620+
path,
621+
})
622+
623+
setModified(true)
624+
},
625+
[dispatchFields],
626+
)
627+
614628
const removeFieldRow: FormContextType['removeFieldRow'] = useCallback(
615629
({ path, rowIndex }) => {
616630
dispatchFields({ type: 'REMOVE_ROW', path, rowIndex })
631+
632+
setModified(true)
617633
},
618634
[dispatchFields],
619635
)
@@ -672,6 +688,7 @@ export const Form: React.FC<FormProps> = (props) => {
672688
contextRef.current.dispatchFields = dispatchFields
673689
contextRef.current.addFieldRow = addFieldRow
674690
contextRef.current.removeFieldRow = removeFieldRow
691+
contextRef.current.moveFieldRow = moveFieldRow
675692
contextRef.current.replaceFieldRow = replaceFieldRow
676693
contextRef.current.uuid = uuid
677694
contextRef.current.initializing = initializing
@@ -710,7 +727,7 @@ export const Form: React.FC<FormProps> = (props) => {
710727
refreshCookie()
711728
},
712729
15000,
713-
[fields],
730+
[formState],
714731
)
715732

716733
useEffect(() => {
@@ -743,7 +760,7 @@ export const Form: React.FC<FormProps> = (props) => {
743760
})
744761

745762
if (changed) {
746-
prevFields.current = newState
763+
prevFormState.current = newState
747764

748765
dispatchFields({
749766
type: 'REPLACE_STATE',
@@ -757,14 +774,14 @@ export const Form: React.FC<FormProps> = (props) => {
757774

758775
useDebouncedEffect(
759776
() => {
760-
if ((isFirstRenderRef.current || !dequal(fields, prevFields.current)) && modified) {
777+
if ((isFirstRenderRef.current || !dequal(formState, prevFormState.current)) && modified) {
761778
executeOnChange(submitted)
762779
}
763780

764-
prevFields.current = fields
781+
prevFormState.current = formState
765782
isFirstRenderRef.current = false
766783
},
767-
[modified, submitted, fields],
784+
[modified, submitted, formState],
768785
250,
769786
)
770787

@@ -793,7 +810,7 @@ export const Form: React.FC<FormProps> = (props) => {
793810
<FormContext value={contextRef.current}>
794811
<FormWatchContext
795812
value={{
796-
fields,
813+
fields: formState,
797814
...contextRef.current,
798815
}}
799816
>

‎packages/ui/src/forms/Form/initContextState.ts

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const initContextState: Context = {
4141
getSiblingData,
4242
initializing: undefined,
4343
isValid: true,
44+
moveFieldRow: () => undefined,
4445
removeFieldRow: () => undefined,
4546
replaceFieldRow: () => undefined,
4647
replaceState: () => undefined,

‎packages/ui/src/forms/Form/mergeServerFormState.ts

+27-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
'use client'
2+
import type { FieldState } from 'payload'
3+
24
import { dequal } from 'dequal/lite' // lite: no need for Map and Set support
35
import { type FormState } from 'payload'
46

@@ -27,7 +29,7 @@ export const mergeServerFormState = ({
2729
const newState = {}
2830

2931
if (existingState) {
30-
const serverPropsToAccept = [
32+
const serverPropsToAccept: Array<keyof FieldState> = [
3133
'passesCondition',
3234
'valid',
3335
'errorMessage',
@@ -46,6 +48,7 @@ export const mergeServerFormState = ({
4648
if (!incomingState[path]) {
4749
continue
4850
}
51+
4952
let fieldChanged = false
5053

5154
/**
@@ -55,6 +58,7 @@ export const mergeServerFormState = ({
5558
newFieldState.errorPaths,
5659
incomingState[path].errorPaths as unknown as string[],
5760
)
61+
5862
if (errorPathsResult.result) {
5963
if (errorPathsResult.changed) {
6064
changed = errorPathsResult.changed
@@ -76,25 +80,41 @@ export const mergeServerFormState = ({
7680
/**
7781
* Handle adding all the remaining props that should be updated in the local form state from the server form state
7882
*/
79-
serverPropsToAccept.forEach((prop) => {
80-
if (!dequal(incomingState[path]?.[prop], newFieldState[prop])) {
83+
serverPropsToAccept.forEach((propFromServer) => {
84+
if (!dequal(incomingState[path]?.[propFromServer], newFieldState[propFromServer])) {
8185
changed = true
8286
fieldChanged = true
83-
if (!(prop in incomingState[path])) {
87+
88+
if (newFieldState?.serverPropsToIgnore?.includes(propFromServer)) {
89+
// Remove the ignored prop for the next request
90+
newFieldState.serverPropsToIgnore = newFieldState.serverPropsToIgnore.filter(
91+
(prop) => prop !== propFromServer,
92+
)
93+
94+
// if no keys left, remove the entire object
95+
if (!newFieldState.serverPropsToIgnore.length) {
96+
delete newFieldState.serverPropsToIgnore
97+
}
98+
99+
return
100+
}
101+
102+
if (!(propFromServer in incomingState[path])) {
84103
// Regarding excluding the customComponents prop from being deleted: the incoming state might not have been rendered, as rendering components for every form onchange is expensive.
85104
// Thus, we simply re-use the initial render state
86-
if (prop !== 'customComponents') {
87-
delete newFieldState[prop]
105+
if (propFromServer !== 'customComponents') {
106+
delete newFieldState[propFromServer]
88107
}
89108
} else {
90-
newFieldState[prop] = incomingState[path][prop]
109+
newFieldState[propFromServer as any] = incomingState[path][propFromServer]
91110
}
92111
}
93112
})
94113

95114
if (newFieldState.valid !== false) {
96115
newFieldState.valid = true
97116
}
117+
98118
if (newFieldState.passesCondition !== false) {
99119
newFieldState.passesCondition = true
100120
}
@@ -106,7 +126,6 @@ export const mergeServerFormState = ({
106126
// Now loop over values that are part of incoming state but not part of existing state, and add them to the new state.
107127
// This can happen if a new array row was added. In our local state, we simply add out stubbed `array` and `array.[index].id` entries to the local form state.
108128
// However, all other array sub-fields are not added to the local state - those will be added by the server and may be incoming here.
109-
110129
for (const [path, field] of Object.entries(incomingState)) {
111130
if (!existingState[path]) {
112131
changed = true

0 commit comments

Comments
 (0)