fix: improve error path merging from server, make sure no new or removed rows/values coming from the server are being considered outside addFieldRow

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

@@ -8,7 +8,7 @@ export type Data = {
 export type Row = {
   blockType?: string
   collapsed?: boolean
-  errorPaths?: Set<string>
+  errorPaths?: string[]
   id: string
 }
 
@@ -19,7 +19,7 @@ export type FilterOptionsResult = {
 export type FormField = {
   disableFormData?: boolean
   errorMessage?: string
-  errorPaths?: Set<string>
+  errorPaths?: string[]
   fieldSchema?: Field
   filterOptions?: FilterOptionsResult
   initialValue: unknown

packages/ui/src/fields/Array/ArrayRow.tsx

@@ -70,7 +70,7 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
     '0',
   )}`
 
-  const errorCount = row.errorPaths?.size
+  const errorCount = row.errorPaths?.length
   const fieldHasErrors = errorCount > 0 && hasSubmitted
 
   const classNames = [

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

@@ -181,7 +181,7 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {
   const hasMaxRows = maxRows && rows.length >= maxRows
 
   const fieldErrorCount =
-    rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0) + (valid ? 0 : 1)
+    rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0) + (valid ? 0 : 1)
 
   const fieldHasErrors = submitted && fieldErrorCount > 0
 

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

@@ -192,7 +192,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
 
   const hasMaxRows = maxRows && rows.length >= maxRows
 
-  const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0)
+  const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0)
   const fieldHasErrors = submitted && fieldErrorCount + (valid ? 0 : 1) > 0
 
   const showMinRows = rows.length < minRows || (required && rows.length === 0)

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

@@ -17,14 +17,13 @@ const ObjectId = (ObjectIdImport.default ||
 export function fieldReducer(state: FormState, action: FieldAction): FormState {
   switch (action.type) {
     case 'REPLACE_STATE': {
-      const newState = {}
-
       if (action.optimize !== false) {
         // Only update fields that have changed
         // by comparing old value / initialValue to new
         // ..
         // This is a performance enhancement for saving
         // large documents with hundreds of fields
+        const newState = {}
 
         Object.entries(action.state).forEach(([path, field]) => {
           const oldField = state[path]
@@ -36,12 +35,10 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
             newState[path] = oldField
           }
         })
-      } else {
-        // If we're not optimizing, just set the state to the new state
-        return action.state
+        return newState
       }
-
-      return newState
+      // If we're not optimizing, just set the state to the new state
+      return action.state
     }
 
     case 'REMOVE': {

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

@@ -5,7 +5,7 @@ import type { FormState } from 'payload/types'
 import isDeepEqual from 'deep-equal'
 import { useRouter } from 'next/navigation.js'
 import { serialize } from 'object-to-formdata'
-import { wait } from 'payload/utilities'
+import { deepCopyObject, wait } from 'payload/utilities'
 import QueryString from 'qs'
 import React, { useCallback, useEffect, useReducer, useRef, useState } from 'react'
 import { toast } from 'react-toastify'
@@ -544,7 +544,11 @@ export const Form: React.FC<FormProps> = (props) => {
           )
 
           if (changed) {
-            dispatchFields({ type: 'REPLACE_STATE', optimize: false, state: newState })
+            dispatchFields({
+              type: 'REPLACE_STATE',
+              optimize: false,
+              state: newState,
+            })
           }
         }
       }

packages/ui/src/forms/Form/mergeErrorPaths.ts

@@ -0,0 +1,38 @@
+import { arraysHaveSameStrings } from '@payloadcms/ui/utilities/arraysHaveSameStrings'
+
+export const mergeErrorPaths = (
+  existing?: string[],
+  incoming?: string[],
+): {
+  changed: boolean
+  result?: string[]
+} => {
+  if (!existing) {
+    return {
+      changed: false,
+      result: undefined,
+    }
+  }
+
+  const existingErrorPaths: string[] = []
+  const incomingErrorPaths: string[] = []
+
+  if (Array.isArray(incoming) && incoming?.length) {
+    incoming.forEach((path) => incomingErrorPaths.push(path))
+  }
+
+  if (Array.isArray(existing) && existing?.length) {
+    existing.forEach((path) => existingErrorPaths.push(path))
+  }
+
+  if (!arraysHaveSameStrings(existingErrorPaths, incomingErrorPaths)) {
+    return {
+      changed: true,
+      result: incomingErrorPaths,
+    }
+  }
+  return {
+    changed: false,
+    result: existing,
+  }
+}

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

@@ -1,10 +1,8 @@
 import type { FormState } from 'payload/types'
 
-import isDeepEqual from 'deep-equal'
+import { mergeErrorPaths } from './mergeErrorPaths.js'
 
-import { arraysHaveSameStrings } from '../../utilities/arraysHaveSameStrings.js'
-
-const propsToCheck = ['passesCondition', 'valid', 'errorMessage']
+const serverPropsToAccept = ['passesCondition', 'valid', 'errorMessage']
 
 /**
  * Merges certain properties from the server state into the client state. These do not include values,
@@ -14,51 +12,70 @@ const propsToCheck = ['passesCondition', 'valid', 'errorMessage']
  * is the thing we want to keep in sync with the server (where it's calculated) on the client.
  */
 export const mergeServerFormState = (
-  oldState: FormState,
-  newState: FormState,
+  existingState: FormState,
+  incomingState: FormState,
 ): { changed: boolean; newState: FormState } => {
   let changed = false
 
-  if (oldState) {
-    Object.entries(newState).forEach(([path, newFieldState]) => {
-      if (!oldState[path]) return
+  if (existingState) {
+    Object.entries(existingState).forEach(([path, newFieldState]) => {
+      if (!incomingState[path]) return
 
-      newFieldState.initialValue = oldState[path].initialValue
-      newFieldState.value = oldState[path].value
+      /**
+       * Handle error paths
+       */
 
-      const oldErrorPaths: string[] = []
-      const newErrorPaths: string[] = []
-
-      if (oldState[path].errorPaths instanceof Set) {
-        oldState[path].errorPaths.forEach((path) => oldErrorPaths.push(path))
+      const errorPathsResult = mergeErrorPaths(
+        newFieldState.errorPaths,
+        incomingState[path].errorPaths as unknown as string[],
+      )
+      if (errorPathsResult.result) {
+        if (errorPathsResult.changed) {
+          changed = errorPathsResult.changed
+        }
+        newFieldState.errorPaths = errorPathsResult.result
       }
 
-      if (
-        newFieldState.errorPaths &&
-        !Array.isArray(newFieldState.errorPaths) &&
-        typeof newFieldState.errorPaths
-      ) {
-        Object.values(newFieldState.errorPaths).forEach((path) => newErrorPaths.push(path))
-      }
-
-      if (!arraysHaveSameStrings(oldErrorPaths, newErrorPaths)) {
-        changed = true
-      }
-
-      propsToCheck.forEach((prop) => {
-        if (newFieldState[prop] != oldState[path][prop]) {
+      /**
+       * Handle the rest which is in serverPropsToAccept
+       */
+      serverPropsToAccept.forEach((prop) => {
+        if (incomingState[path]?.[prop] !== newFieldState[prop]) {
           changed = true
+          if (!(prop in incomingState[path])) {
+            delete newFieldState[prop]
+          } else {
+            newFieldState[prop] = incomingState[path][prop]
+          }
         }
       })
 
-      if (
-        Array.isArray(newFieldState.rows) &&
-        !isDeepEqual(newFieldState.rows, oldState[path].rows)
-      ) {
-        changed = true
+      /**
+       * Handle rows
+       */
+      if (Array.isArray(newFieldState.rows) && newFieldState?.rows.length) {
+        let i = 0
+        for (const row of newFieldState.rows) {
+          const incomingRow = incomingState[path]?.rows?.[i]
+          if (incomingRow) {
+            const errorPathsRowResult = mergeErrorPaths(
+              row.errorPaths,
+              incomingRow.errorPaths as unknown as string[],
+            )
+            if (errorPathsRowResult.result) {
+              if (errorPathsResult.changed) {
+                changed = true
+              }
+              incomingRow.errorPaths = errorPathsRowResult.result
+            }
+          }
+
+          i++
+        }
       }
     })
   }
 
-  return { changed, newState }
+  // Conditions don't work if we don't memcopy the new state, as the object references would otherwise be the same
+  return { changed, newState: { ...existingState } }
 }

packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts

@@ -23,7 +23,7 @@ export type AddFieldStatePromiseArgs = {
    */
   anyParentLocalized?: boolean
   data: Data
-  errorPaths: Set<string>
+  errorPaths: string[]
   field: NonPresentationalField
   /**
    * You can use this to filter down to only `localized` fields that require translation (type: text, textarea, etc.). Another plugin might want to look for only `point` type fields to do some GIS function. With the filter function you can go in like a surgeon.
@@ -74,7 +74,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
     id,
     anyParentLocalized = false,
     data,
-    errorPaths: parentErrorPaths,
+    errorPaths: parentErrorPaths = [],
     field,
     filter,
     forceFullValue = false,
@@ -95,7 +95,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
     const validate = operation === 'update' ? field.validate : undefined
 
     const fieldState: FormField = {
-      errorPaths: new Set(),
+      errorPaths: [],
       fieldSchema: includeSchema ? field : undefined,
       initialValue: undefined,
       passesCondition,
@@ -144,7 +144,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
       fieldState.valid = false
       // TODO: this is unpredictable, need to figure out why
       // It will sometimes lead to inconsistencies across re-renders
-      parentErrorPaths.add(`${path}${field.name}`)
+      if (!parentErrorPaths.includes(`${path}${field.name}`)) {
+        parentErrorPaths.push(`${path}${field.name}`)
+      }
     } else {
       fieldState.valid = true
     }

packages/ui/src/forms/buildStateFromSchema/index.tsx

@@ -40,7 +40,7 @@ export const buildStateFromSchema = async (args: Args): Promise<FormState> => {
     await iterateFields({
       id,
       data: fullData,
-      errorPaths: new Set(),
+      errorPaths: [],
       fields: fieldSchema,
       fullData,
       operation,

packages/ui/src/forms/buildStateFromSchema/iterateFields.ts

@@ -12,7 +12,7 @@ type Args = {
    */
   anyParentLocalized?: boolean
   data: Data
-  errorPaths: Set<string>
+  errorPaths: string[]
   fields: FieldSchema[]
   filter?: (args: AddFieldStatePromiseArgs) => boolean
   /**

test/access-control/e2e.spec.ts

@@ -9,7 +9,6 @@ import type { ReadOnlyCollection, RestrictedVersion } from './payload-types.js'
 
 import {
   closeNav,
-  delayNetwork,
   exactText,
   initPageConsoleErrorCatch,
   openDocControls,
@@ -60,7 +59,6 @@ describe('access control', () => {
     const context = await browser.newContext()
     page = await context.newPage()
     initPageConsoleErrorCatch(page)
-    await delayNetwork({ context, page, delay: 'Slow 4G' })
   })
 
   test('field without read access should not show', async () => {

test/fields/lexical.e2e.spec.ts

@@ -32,7 +32,7 @@ async function navigateToLexicalFields() {
   const url: AdminUrlUtil = new AdminUrlUtil(serverURL, 'lexical-fields')
   await page.goto(url.list)
   const linkToDoc = page.locator('tbody tr:first-child .cell-title a').first()
-  await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: 45000 })
+  await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: POLL_TOPASS_TIMEOUT })
   const linkDocHref = await linkToDoc.getAttribute('href')
 
   await linkToDoc.click()