Field hook `afterChange` arg `previousValue` is only set correctly for fields in root of collection

[ysfaran]

Link to reproduction

https://github.com/ysfaran/payload/tree/reproduction-after-change-bug

Describe the Bug

When you create an afterChange hook for any nested field type like type: 'group' the hook will set the value for previousValue wrongly:

{
  name: 'submenu',
  type: 'group',
  fields: [
    {
      name: 'name',
      type: 'text',
      defaultValue: 'initialValueSub',
      hooks: {
        afterChange: [
          ({ value, previousValue }) => {
            // previousValue will store an unexpected value here
          },
        ],
      },
    },
  ],
}

This happens because in the code the previousValue of a field is always checked against the root of a collection/global. For example if you have this kind of structure defined in your collection/global:

{ 
  name: 'rootValue',
  group: {
    name: 'subValue'
  }
}

And you change the value of group.name, then the previousValue in the afterChange hook would NOT be subValue but rootValue. This only happens because they have the same field name (here literally name). If there is no root field with the same name previousValue will always be undefined.

I already applied a yarn patch for my case and could work arround this with following diff:

diff --git a/dist/fields/hooks/afterChange/promise.js b/dist/fields/hooks/afterChange/promise.js
index 011ef42ed988077b15a636d93d582867764bb3fc..18ce7c068430f34a91fab83c6d69e106eae9e70c 100644
--- a/dist/fields/hooks/afterChange/promise.js
+++ b/dist/fields/hooks/afterChange/promise.js
@@ -26,7 +26,7 @@ const promise = async ({ collection, context, data, doc, field, global, operatio
                     originalDoc: doc,
                     previousDoc,
                     previousSiblingDoc,
-                    previousValue: previousDoc[field.name],
+                    previousValue: previousSiblingDoc[field.name],
                     req,
                     siblingData,
                     value: siblingData[field.name]
diff --git a/dist/fields/hooks/afterChange/promise.js b/dist/fields/hooks/afterChange/promise.js
index 18ce7c068430f34a91fab83c6d69e106eae9e70c..d7b3b797a17ec43c2188c6505e2292f1492f60f5 100644
--- a/dist/fields/hooks/afterChange/promise.js
+++ b/dist/fields/hooks/afterChange/promise.js
@@ -50,7 +50,7 @@ const promise = async ({ collection, context, data, doc, field, global, operatio
                     global,
                     operation,
                     previousDoc,
-                    previousSiblingDoc: previousDoc[field.name],
+                    previousSiblingDoc: (previousSiblingDoc?.[field.name]) || {},
                     req,
                     siblingData: (siblingData?.[field.name]) || {},
                     siblingDoc: siblingDoc[field.name]

But this would only fix it for the group field type, not for other nested types like blocks or array.

I happy to file a PR the upcoming days, if no one takes it right away :)

To Reproduce

I will just use the example of the reproduction link here:

1. Create a Menu Global like this:

import type { GlobalConfig } from '../../../../packages/payload/src/globals/config/types'

export const menuSlug = 'menu'

export const MenuGlobal: GlobalConfig = {
  slug: menuSlug,
  fields: [
    {
      name: 'name',
      type: 'text',
      defaultValue: 'initialValueRoot',
      hooks: {
        afterChange: [
          ({ value, previousValue }) => {
            if (value !== previousValue) {
              return `${previousValue} ${value}`
            }
          },
        ],
      },
    },
    {
      name: 'submenu',
      type: 'group',
      fields: [
        {
          name: 'name',
          type: 'text',
          defaultValue: 'initialValueSub',
          hooks: {
            afterChange: [
              ({ value, previousValue }) => {
                if (value !== previousValue) {
                  return `${previousValue} ${value}`
                }
              },
            ],
          },
        },
      ],
    },
  ],
}

2. Change the value of the root field name in the admin panel and save.
3. This will now change the root field name to initialValueRoot <the text you entered>, but also unexpectedly the submenu.name field to initialValueRoot initialValueSub.

Payload Version

2.5.0

Adapters and Plugins

No response

[ysfaran]

I just noticed something else: Isn't it also a bug that you can change the stored data by returning a value in the afterChange hook?

The test i wrote actually proves that you can change the data even afterChange, which is confusing 🤔 The docs also don't state that you should return something.

In my original case I used the afterChange hook for a side effect to delete an item of another collection, which IMO is the use case for such a hook. If you want to change the data, which the afterChange hook was triggered for you should probably use beforeValidate or beforeChange I would assume.

[jmikrut]

@JessChowdhury let's review this PR together when you have bandwidth.

[ysfaran]

Can some one clarify if it is really intended, that you can change data when a value is returned from afterChange?

If yes, there should be some explanation in the docs.

If no, this is another bug and should be fixed.