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

Continuation of #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

Author: jacobsfletch

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?: {

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`

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
           },
           {

test/form-state/collections/Posts/ArrayRowLabel.tsx

@@ -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>
 }

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,14 +587,39 @@ 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)',
         initialValue: 'Test Post',
         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',