tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix: deeply merge array and block rows from server form state (#13551)

Browse Source 
Continuation of https://github.com/payloadcms/payload/pull/13501.

When merging server form state with `acceptValues: true`, like on submit
(not autosave), rows are not deeply merged causing custom row
components, like row labels, to disappear. This is because we never
attach components to the form state response unless it has re-rendered
server-side, so unless we merge these rows with the current state, we
lose them.

Instead of allowing `acceptValues` to override all local changes to
rows, we need to flag any newly added rows with `addedByServer` so they
can bypass the merge strategy. Existing rows would continue to be merged
as expected, and new rows are simply appended to the end.

Discovered here:
https://discord.com/channels/967097582721572934/967097582721572937/1408367321797365840

---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1211115023863814
This commit is contained in:
Jacob Fletcher
2025-08-22 15:41:51 -04:00
committed by GitHub
parent 598ff4cf7b
commit 1e13474068
5 changed files with 99 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
packages/payload/src/admin/forms/Form.ts
View File

@@ -11,6 +11,7 @@ export type Data = {
}
export type Row = {
addedByServer?: FieldState['addedByServer']
blockType?: string
collapsed?: boolean
customComponents?: {

42
packages/ui/src/forms/Form/mergeServerFormState.ts
View File

@@ -89,30 +89,38 @@ export const mergeServerFormState = ({
}
/**
* Intelligently merge the rows array to ensure changes to local state are not lost while the request was pending
* Deeply merge the rows array to ensure changes to local state are not lost while the request was pending
* For example, the server response could come back with a row which has been deleted on the client
* Loop over the incoming rows, if it exists in client side form state, merge in any new properties from the server
* Note: read `currentState` and not `newState` here, as the `rows` property have already been merged above
*/
if (Array.isArray(incomingField.rows)) {
if (acceptValues === true) {
newState[path].rows = incomingField.rows
} else if (path in currentState) {
newState[path].rows = [...(currentState[path]?.rows || [])] // shallow copy to avoid mutating the original array
if (Array.isArray(incomingField.rows) && path in currentState) {
newState[path].rows = [...(currentState[path]?.rows || [])] // shallow copy to avoid mutating the original array
incomingField.rows.forEach((row) => {
const indexInCurrentState = currentState[path].rows?.findIndex(
(existingRow) => existingRow.id === row.id,
)
incomingField.rows.forEach((row) => {
const indexInCurrentState = currentState[path].rows?.findIndex(
(existingRow) => existingRow.id === row.id,
)
if (indexInCurrentState > -1) {
newState[path].rows[indexInCurrentState] = {
...currentState[path].rows[indexInCurrentState],
...row,
}
if (indexInCurrentState > -1) {
newState[path].rows[indexInCurrentState] = {
...currentState[path].rows[indexInCurrentState],
...row,
}
})
}
} else if (row.addedByServer) {
/**
* Note: This is a known limitation of computed array and block rows
* If a new row was added by the server, we append it to the _end_ of this array
* This is because the client is the source of truth, and it has arrays ordered in a certain position
* For example, the user may have re-ordered rows client-side while a long running request is processing
* This means that we _cannot_ slice a new row into the second position on the server, for example
* By the time it gets back to the client, its index is stale
*/
const newRow = { ...row }
delete newRow.addedByServer
newState[path].rows.push(newRow)
}
})
}
// If `valid` is `undefined`, mark it as `true`

9
packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts
View File

@@ -352,7 +352,10 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
newRow.lastRenderedPath = previousRow.lastRenderedPath
}
acc.rows.push(newRow)
// add addedByServer flag
if (!previousRow) {
newRow.addedByServer = true
}
const isCollapsed = isRowCollapsed({
collapsedPrefs: preferences?.fields?.[path]?.collapsed,
@@ -362,9 +365,11 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
})
if (isCollapsed) {
acc.rows[acc.rows.length - 1].collapsed = true
newRow.collapsed = true
}
acc.rows.push(newRow)
return acc
},
{

2
test/form-state/collections/Posts/ArrayRowLabel.tsx
View File

@@ -1,5 +1,5 @@
import React from 'react'
export const ArrayRowLabel = () => {
return <p>This is a custom component</p>
return <p id="custom-array-row-label">This is a custom component</p>
}

68
test/form-state/int.spec.ts
View File

@@ -1,4 +1,5 @@
import type { FieldState, FormState, Payload, User } from 'payload'
import type React from 'react'
import { buildFormState } from '@payloadcms/ui/utilities/buildFormState'
import path from 'path'
@@ -10,6 +11,7 @@ import type { NextRESTClient } from '../helpers/NextRESTClient.js'
import { devUser } from '../credentials.js'
import { initPayloadInt } from '../helpers/initPayloadInt.js'
import { postsSlug } from './collections/Posts/index.js'
// eslint-disable-next-line payload/no-relative-monorepo-imports
import { mergeServerFormState } from '../../packages/ui/src/forms/Form/mergeServerFormState.js'
@@ -21,6 +23,14 @@ const { email, password } = devUser
const filename = fileURLToPath(import.meta.url)
const dirname = path.dirname(filename)
const DummyReactComponent: React.ReactNode = {
// @ts-expect-error - can ignore, needs to satisfy `typeof value.$$typeof === 'symbol'`
$$typeof: Symbol.for('react.element'),
type: 'div',
props: {},
key: null,
}
describe('Form State', () => {
beforeAll(async () => {
;({ payload, restClient } = await initPayloadInt(dirname, undefined, true))
@@ -566,7 +576,7 @@ describe('Form State', () => {
expect(newState === currentState).toBe(true)
})
it('should accept all values from the server regardless of local modifications, e.g. on submit', () => {
it('should accept all values from the server regardless of local modifications, e.g. `acceptAllValues` on submit', () => {
const title: FieldState = {
value: 'Test Post (modified on the client)',
initialValue: 'Test Post',
@@ -577,7 +587,7 @@ describe('Form State', () => {
const currentState: Record<string, FieldState> = {
title: {
...title,
isModified: true,
isModified: true, // This is critical, this is what we're testing
},
computedTitle: {
value: 'Test Post (computed on the client)',
@@ -585,6 +595,31 @@ describe('Form State', () => {
valid: true,
passesCondition: true,
},
array: {
rows: [
{
id: '1',
customComponents: {
RowLabel: DummyReactComponent,
},
lastRenderedPath: 'array.0.customTextField',
},
],
valid: true,
passesCondition: true,
},
'array.0.id': {
value: '1',
initialValue: '1',
valid: true,
passesCondition: true,
},
'array.0.customTextField': {
value: 'Test Post (modified on the client)',
initialValue: 'Test Post',
valid: true,
passesCondition: true,
},
}
const incomingStateFromServer: Record<string, FieldState> = {
@@ -600,6 +635,29 @@ describe('Form State', () => {
valid: true,
passesCondition: true,
},
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
// Omit `customComponents` because the server did not re-render this row
},
],
passesCondition: true,
valid: true,
},
'array.0.id': {
value: '1',
initialValue: '1',
valid: true,
passesCondition: true,
},
'array.0.customTextField': {
value: 'Test Post (modified on the client)',
initialValue: 'Test Post',
valid: true,
passesCondition: true,
},
}
const newState = mergeServerFormState({
@@ -614,10 +672,14 @@ describe('Form State', () => {
...incomingStateFromServer.title,
isModified: true,
},
array: {
...incomingStateFromServer.array,
rows: currentState?.array?.rows,
},
})
})
it('should not accept values from the server if they have been modified locally since the request was made, e.g. on autosave', () => {
it('should not accept values from the server if they have been modified locally since the request was made, e.g. `overrideLocalChanges: false` on autosave', () => {
const title: FieldState = {
value: 'Test Post (modified on the client 1)',
initialValue: 'Test Post',