fix(ui): properly reflects hook changes in ui (#10268)

Fixes #9882 and #9691

In 2.0, we would accept data coming back from an update operation and
then reflect those changes in UI.

However, in 3.0, we did not do that anymore - meaning you could change a
document with hooks in `beforeChange` or `afterChange`, but then not see
the changes made on the server.

This PR updates the way that `mergeServerFormState` works, and adds a
property to optionally allow values from server form state - which can
then be used in the `onSuccess` form handler which may need to define
new field values.

Author: jmikrut

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

@@ -12,8 +12,8 @@ import { useConfig } from '../../providers/Config/index.js'
 import { useLocale } from '../../providers/Locale/index.js'
 import { mergeFieldStyles } from '../mergeFieldStyles.js'
 import { isFieldRTL } from '../shared/index.js'
-import './index.scss'
 import { TextInput } from './Input.js'
+import './index.scss'
 
 export { TextInput, TextInputProps }
 

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

@@ -334,10 +334,11 @@ export const Form: React.FC<FormProps> = (props) => {
           if (typeof onSuccess === 'function') {
             const newFormState = await onSuccess(json)
             if (newFormState) {
-              const { newState: mergedFormState } = mergeServerFormState(
-                contextRef.current.fields || {},
-                newFormState,
-              )
+              const { newState: mergedFormState } = mergeServerFormState({
+                acceptValues: true,
+                existingState: contextRef.current.fields || {},
+                incomingState: newFormState,
+              })
 
               dispatchFields({
                 type: 'REPLACE_STATE',
@@ -666,10 +667,10 @@ export const Form: React.FC<FormProps> = (props) => {
             return
           }
 
-          const { changed, newState } = mergeServerFormState(
-            contextRef.current.fields || {},
-            revalidatedFormState,
-          )
+          const { changed, newState } = mergeServerFormState({
+            existingState: contextRef.current.fields || {},
+            incomingState: revalidatedFormState,
+          })
 
           if (changed) {
             dispatchFields({

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

@@ -4,14 +4,11 @@ import { type FormState } from 'payload'
 
 import { mergeErrorPaths } from './mergeErrorPaths.js'
 
-const serverPropsToAccept = [
-  'passesCondition',
-  'valid',
-  'errorMessage',
-  'rows',
-  'customComponents',
-  'requiresRender',
-]
+type Args = {
+  acceptValues?: boolean
+  existingState: FormState
+  incomingState: FormState
+}
 
 /**
  * Merges certain properties from the server state into the client state. These do not include values,
@@ -20,10 +17,24 @@ const serverPropsToAccept = [
  * We want to use this to update the error state, and other properties that are not user input, as the error state
  * is the thing we want to keep in sync with the server (where it's calculated) on the client.
  */
-export const mergeServerFormState = (
-  existingState: FormState,
-  incomingState: FormState,
-): { changed: boolean; newState: FormState } => {
+export const mergeServerFormState = ({
+  acceptValues,
+  existingState,
+  incomingState,
+}: Args): { changed: boolean; newState: FormState } => {
+  const serverPropsToAccept = [
+    'passesCondition',
+    'valid',
+    'errorMessage',
+    'rows',
+    'customComponents',
+    'requiresRender',
+  ]
+
+  if (acceptValues) {
+    serverPropsToAccept.push('value')
+  }
+
   let changed = false
 
   const newState = {}

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

@@ -38,10 +38,8 @@ export const useField = <TValue,>(options: Options): FieldType<TValue> => {
   const { id, collectionSlug } = useDocumentInfo()
   const operation = useOperation()
 
-  const { dispatchField, field } = useFormFields(([fields, dispatch]) => ({
-    dispatchField: dispatch,
-    field: (fields && fields?.[path]) || null,
-  }))
+  const dispatchField = useFormFields(([_, dispatch]) => dispatch)
+  const field = useFormFields(([fields]) => (fields && fields?.[path]) || null)
 
   const { t } = useTranslation()
   const { config } = useConfig()

packages/ui/src/views/Edit/index.tsx

@@ -291,32 +291,32 @@ export const DefaultEditView: React.FC<ClientSideEditViewProps> = ({
       }
     },
     [
-      adminRoute,
+      reportUpdate,
+      id,
+      entitySlug,
+      user,
       collectionSlug,
+      userSlug,
+      incrementVersionCount,
+      updateSavedDocumentData,
+      onSaveFromContext,
+      isEditing,
       depth,
-      docPermissions,
-      entitySlug,
       getDocPermissions,
-      getDocPreferences,
-      getFormState,
       globalSlug,
-      id,
-      incrementVersionCount,
-      isEditing,
-      isLockingEnabled,
-      locale,
-      onSaveFromContext,
-      operation,
+      autosaveEnabled,
       refreshCookieAsync,
-      reportUpdate,
-      resetUploadEdits,
+      adminRoute,
+      locale,
       router,
+      resetUploadEdits,
+      getDocPreferences,
+      getFormState,
+      docPermissions,
+      operation,
       schemaPathSegments,
+      isLockingEnabled,
       setDocumentIsLocked,
-      updateSavedDocumentData,
-      user,
-      userSlug,
-      autosaveEnabled,
     ],
   )
 

test/hooks/collections/BeforeChange/index.ts

@@ -0,0 +1,22 @@
+import type { CollectionConfig } from 'payload'
+
+export const BeforeChangeHooks: CollectionConfig = {
+  slug: 'before-change-hooks',
+  hooks: {
+    beforeChange: [
+      ({ data }) => {
+        data.title = 'hi from hook'
+
+        return data
+      },
+    ],
+  },
+  fields: [
+    {
+      name: 'title',
+      label: 'Title',
+      type: 'text',
+      required: true,
+    },
+  ],
+}

test/hooks/config.ts

@@ -8,6 +8,7 @@ import { APIError } from 'payload'
 
 import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
 import { AfterOperationCollection } from './collections/AfterOperation/index.js'
+import { BeforeChangeHooks } from './collections/BeforeChange/index.js'
 import { BeforeValidateCollection } from './collections/BeforeValidate/index.js'
 import ChainingHooks from './collections/ChainingHooks/index.js'
 import ContextHooks from './collections/ContextHooks/index.js'
@@ -25,6 +26,7 @@ export const HooksConfig: Promise<SanitizedConfig> = buildConfigWithDefaults({
     },
   },
   collections: [
+    BeforeChangeHooks,
     BeforeValidateCollection,
     AfterOperationCollection,
     ContextHooks,

test/hooks/e2e.spec.ts

@@ -23,6 +23,7 @@ let payload: PayloadTestSDK<Config>
 
 describe('Hooks', () => {
   let url: AdminUrlUtil
+  let beforeChangeURL: AdminUrlUtil
   let page: Page
   let serverURL: string
 
@@ -31,6 +32,7 @@ describe('Hooks', () => {
     ;({ payload, serverURL } = await initPayloadE2ENoConfig<Config>({ dirname }))
 
     url = new AdminUrlUtil(serverURL, 'before-validate')
+    beforeChangeURL = new AdminUrlUtil(serverURL, 'before-change-hooks')
 
     const context = await browser.newContext()
     page = await context.newPage()
@@ -58,6 +60,18 @@ describe('Hooks', () => {
 
     await expect(page.locator('#field-title')).toHaveValue('reset in beforeValidate')
   })
+
+  test('should reflect changes made in beforeChange collection hooks within ui after save', async () => {
+    await page.goto(beforeChangeURL.create)
+    await page.locator('#field-title').fill('should replace value with before change response')
+    await saveDocAndAssert(page)
+
+    await expect(page.locator('#field-title')).toHaveValue('hi from hook')
+    await page.locator('#field-title').fill('helllooooooooo')
+    await saveDocAndAssert(page)
+
+    await expect(page.locator('#field-title')).toHaveValue('hi from hook')
+  })
 })
 
 async function clearAllDocs(): Promise<void> {

test/hooks/payload-types.ts

@@ -11,6 +11,7 @@ export interface Config {
     'hooks-users': HooksUserAuthOperations;
   };
   collections: {
+    'before-change-hooks': BeforeChangeHook;
     'before-validate': BeforeValidate;
     afterOperation: AfterOperation;
     'context-hooks': ContextHook;
@@ -27,6 +28,7 @@ export interface Config {
   };
   collectionsJoins: {};
   collectionsSelect: {
+    'before-change-hooks': BeforeChangeHooksSelect<false> | BeforeChangeHooksSelect<true>;
     'before-validate': BeforeValidateSelect<false> | BeforeValidateSelect<true>;
     afterOperation: AfterOperationSelect<false> | AfterOperationSelect<true>;
     'context-hooks': ContextHooksSelect<false> | ContextHooksSelect<true>;
@@ -77,6 +79,16 @@ export interface HooksUserAuthOperations {
     password: string;
   };
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "before-change-hooks".
+ */
+export interface BeforeChangeHook {
+  id: string;
+  title: string;
+  updatedAt: string;
+  createdAt: string;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "before-validate".
@@ -231,6 +243,10 @@ export interface DataHook {
 export interface PayloadLockedDocument {
   id: string;
   document?:
+    | ({
+        relationTo: 'before-change-hooks';
+        value: string | BeforeChangeHook;
+      } | null)
     | ({
         relationTo: 'before-validate';
         value: string | BeforeValidate;
@@ -313,6 +329,15 @@ export interface PayloadMigration {
   updatedAt: string;
   createdAt: string;
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "before-change-hooks_select".
+ */
+export interface BeforeChangeHooksSelect<T extends boolean = true> {
+  title?: T;
+  updatedAt?: T;
+  createdAt?: T;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "before-validate_select".