diff --git a/packages/payload/src/admin/forms/Form.ts b/packages/payload/src/admin/forms/Form.ts index 97dbc5589e5..729571b9ae8 100644 --- a/packages/payload/src/admin/forms/Form.ts +++ b/packages/payload/src/admin/forms/Form.ts @@ -11,6 +11,7 @@ export type Data = { } export type Row = { + addedByServer?: FieldState['addedByServer'] blockType?: string collapsed?: boolean customComponents?: { diff --git a/packages/ui/src/forms/Form/mergeServerFormState.ts b/packages/ui/src/forms/Form/mergeServerFormState.ts index 09cba80f061..c94d36af2cd 100644 --- a/packages/ui/src/forms/Form/mergeServerFormState.ts +++ b/packages/ui/src/forms/Form/mergeServerFormState.ts @@ -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 - - 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 (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, + ) + + 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` diff --git a/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts b/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts index fe19d6fe613..3ad96b3db06 100644 --- a/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts +++ b/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts @@ -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 }, { diff --git a/test/form-state/collections/Posts/ArrayRowLabel.tsx b/test/form-state/collections/Posts/ArrayRowLabel.tsx index f0d95f3378b..22a5c9813b8 100644 --- a/test/form-state/collections/Posts/ArrayRowLabel.tsx +++ b/test/form-state/collections/Posts/ArrayRowLabel.tsx @@ -1,5 +1,5 @@ import React from 'react' export const ArrayRowLabel = () => { - return

This is a custom component

+ return

This is a custom component

} diff --git a/test/form-state/int.spec.ts b/test/form-state/int.spec.ts index 71a36db3c07..637de0d7c4a 100644 --- a/test/form-state/int.spec.ts +++ b/test/form-state/int.spec.ts @@ -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 = { 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 = { @@ -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',