fix(ui): deleted array item reappears after reorder with autosave

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

@@ -24,6 +24,49 @@ type Args = {
   incomingState: FormState
 }
 
+/**
+ * Parses an array field path to extract the array path, row index, and field path.
+ * Handles nested arrays by finding the deepest numeric index.
+ *
+ * @param path - The field path to parse (e.g., 'array.1.text' or 'blocks.0.items.1.title')
+ * @returns Object with arrayPath, rowIndex, and fieldPath, or null if not an array field
+ *
+ * @example
+ * parseArrayFieldPath('array.1.text')
+ * // Returns: { arrayPath: 'array', rowIndex: 1, fieldPath: 'text' }
+ *
+ * @example
+ * parseArrayFieldPath('blocks.0.items.1.title')
+ * // Returns: { arrayPath: 'blocks.0.items', rowIndex: 1, fieldPath: 'title' }
+ */
+function parseArrayFieldPath(path: string): {
+  arrayPath: string
+  fieldPath: string
+  rowIndex: number
+} | null {
+  const segments = path.split('.')
+
+  // Find the last numeric index (indicates array row)
+  let lastNumericIndex = -1
+  for (let i = segments.length - 1; i >= 0; i--) {
+    if (/^\d+$/.test(segments[i])) {
+      lastNumericIndex = i
+      break
+    }
+  }
+
+  // Not an array field if no numeric index or nothing after it
+  if (lastNumericIndex === -1 || lastNumericIndex === segments.length - 1) {
+    return null
+  }
+
+  return {
+    arrayPath: segments.slice(0, lastNumericIndex).join('.'),
+    fieldPath: segments.slice(lastNumericIndex + 1).join('.'),
+    rowIndex: parseInt(segments[lastNumericIndex], 10),
+  }
+}
+
 /**
  * This function receives form state from the server and intelligently merges it into the client state.
  * The server contains extra properties that the client may not have, e.g. custom components and error states.
@@ -54,7 +97,7 @@ export const mergeServerFormState = ({
      *   a. accept all values when explicitly requested, e.g. on submit
      *   b. only accept values for unmodified fields, e.g. on autosave
      */
-    const shouldAcceptValue =
+    let shouldAcceptValue =
       incomingField.addedByServer ||
       acceptValues === true ||
       (typeof acceptValues === 'object' &&
@@ -63,6 +106,28 @@ export const mergeServerFormState = ({
         acceptValues.overrideLocalChanges === false &&
         !currentState[path]?.isModified)
 
+    /**
+     * For array row fields, verify the row IDs match at the given index before accepting
+     * server values. If rows were reordered or deleted while the request was in-flight,
+     * the same index may refer to different rows, and accepting the server value would
+     * overwrite the wrong row's data.
+     *
+     * This guard only applies during autosave. On explicit save (acceptValues === true),
+     * the server response is authoritative and this check is bypassed.
+     */
+    if (shouldAcceptValue && !incomingField.addedByServer && acceptValues !== true) {
+      const parsed = parseArrayFieldPath(path)
+      if (parsed) {
+        const { arrayPath, rowIndex } = parsed
+        const clientRowId = currentState[arrayPath]?.rows?.[rowIndex]?.id
+        const serverRowId = incomingState[arrayPath]?.rows?.[rowIndex]?.id
+
+        if (clientRowId === undefined || (serverRowId && clientRowId !== serverRowId)) {
+          shouldAcceptValue = false
+        }
+      }
+    }
+
     let sanitizedIncomingField = incomingField
 
     if (!shouldAcceptValue) {
@@ -93,34 +158,54 @@ export const mergeServerFormState = ({
      * 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
+     *
+     * On explicit save (acceptValues === true), the server is authoritative - accept server row order.
+     * On autosave (acceptValues !== true), the client is source of truth - use ID-based matching.
      */
     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,
+      if (acceptValues === true) {
+        // Explicit save: server is authoritative, use index-based merging
+        newState[path].rows = incomingField.rows.map((serverRow, index) => ({
+          ...(currentState[path]?.rows?.[index] || {}),
+          ...serverRow,
+        }))
+      } else {
+        // Autosave: client is source of truth, use ID-based matching
+        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)
           }
-        } 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)
+        })
+
+        /**
+         * Sync the value field to match the actual row count after merging.
+         * Client is source of truth for row count when rows were added/removed during the request.
+         */
+        if ('value' in incomingField && newState[path].rows.length !== incomingField.value) {
+          newState[path].value = newState[path].rows.length
         }
-      })
+      }
     }
 
     // If `valid` is `undefined`, mark it as `true`