Skip to content

Commit

Permalink
fix: field paths within hooks (#10638)
Browse files Browse the repository at this point in the history
Field paths within hooks are not correct.

For example, an unnamed tab containing a group field and nested text
field should have the path:
- `myGroupField.myTextField`

However, within hooks that path is formatted as:
- `_index-1.myGroupField.myTextField`

The leading index shown above should not exist, as this field is
considered top-level since it is located within an unnamed tab.

This discrepancy is only evident through the APIs themselves, such as
when creating a request with invalid data and reading the validation
errors in the response. Form state contains proper field paths, which is
ultimately why this issue was never caught. This is because within the
admin panel we merge the API response with the current form state,
obscuring the underlying issue. This becomes especially obvious in
#10580, where we no longer initialize validation errors within form
state until the form has been submitted, and instead rely solely on the
API response for the initial error state.

Here's comprehensive example of how field paths _should_ be formatted:

```
{
  // ...
  fields: [
    {
      // path: 'topLevelNamedField'
      // schemaPath: 'topLevelNamedField'
      // indexPath: ''
      name: 'topLevelNamedField',
      type: 'text',
    },
    {
      // path: 'array'
      // schemaPath: 'array'
      // indexPath: ''
      name: 'array',
      type: 'array',
      fields: [
        {
          // path: 'array.[n].fieldWithinArray'
          // schemaPath: 'array.fieldWithinArray'
          // indexPath: ''
          name: 'fieldWithinArray',
          type: 'text',
        },
        {
          // path: 'array.[n].nestedArray'
          // schemaPath: 'array.nestedArray'
          // indexPath: ''
          name: 'nestedArray',
          type: 'array',
          fields: [
            {
              // path: 'array.[n].nestedArray.[n].fieldWithinNestedArray'
              // schemaPath: 'array.nestedArray.fieldWithinNestedArray'
              // indexPath: ''
              name: 'fieldWithinNestedArray',
              type: 'text',
            },
          ],
        },
        {
          // path: 'array.[n]._index-2'
          // schemaPath: 'array._index-2'
          // indexPath: '2'
          type: 'row',
          fields: [
            {
              // path: 'array.[n].fieldWithinRowWithinArray'
              // schemaPath: 'array._index-2.fieldWithinRowWithinArray'
              // indexPath: ''
              name: 'fieldWithinRowWithinArray',
              type: 'text',
            },
          ],
        },
      ],
    },
    {
      // path: '_index-2'
      // schemaPath: '_index-2'
      // indexPath: '2'
      type: 'row',
      fields: [
        {
          // path: 'fieldWithinRow'
          // schemaPath: '_index-2.fieldWithinRow'
          // indexPath: ''
          name: 'fieldWithinRow',
          type: 'text',
        },
      ],
    },
    {
      // path: '_index-3'
      // schemaPath: '_index-3'
      // indexPath: '3'
      type: 'tabs',
      tabs: [
        {
          // path: '_index-3-0'
          // schemaPath: '_index-3-0'
          // indexPath: '3-0'
          label: 'Unnamed Tab',
          fields: [
            {
              // path: 'fieldWithinUnnamedTab'
              // schemaPath: '_index-3-0.fieldWithinUnnamedTab'
              // indexPath: ''
              name: 'fieldWithinUnnamedTab',
              type: 'text',
            },
            {
              // path: '_index-3-0-1'
              // schemaPath: '_index-3-0-1'
              // indexPath: '3-0-1'
              type: 'tabs',
              tabs: [
                {
                  // path: '_index-3-0-1-0'
                  // schemaPath: '_index-3-0-1-0'
                  // indexPath: '3-0-1-0'
                  label: 'Nested Unnamed Tab',
                  fields: [
                    {
                      // path: 'fieldWithinNestedUnnamedTab'
                      // schemaPath: '_index-3-0-1-0.fieldWithinNestedUnnamedTab'
                      // indexPath: ''
                      name: 'fieldWithinNestedUnnamedTab',
                      type: 'text',
                    },
                  ],
                },
              ],
            },
          ],
        },
        {
          // path: 'namedTab'
          // schemaPath: '_index-3.namedTab'
          // indexPath: ''
          label: 'Named Tab',
          name: 'namedTab',
          fields: [
            {
              // path: 'namedTab.fieldWithinNamedTab'
              // schemaPath: '_index-3.namedTab.fieldWithinNamedTab'
              // indexPath: ''
              name: 'fieldWithinNamedTab',
              type: 'text',
            },
          ],
        },
      ],
    },
  ]
}
```
  • Loading branch information
jacobsfletch authored Jan 27, 2025
1 parent 9f9919d commit 0acaf8a
Show file tree
Hide file tree
Showing 43 changed files with 1,224 additions and 318 deletions.
2 changes: 1 addition & 1 deletion packages/payload/src/admin/RichText.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export type BaseRichTextHookArgs<
field: FieldAffectingData
/** The global which the field belongs to. If the field belongs to a collection, this will be null. */
global: null | SanitizedGlobalConfig

indexPath: number[]
/** The full original document in `update` operations. In the `afterChange` hook, this is the resulting document of the operation. */
originalDoc?: TData
/**
Expand Down
2 changes: 1 addition & 1 deletion packages/payload/src/collections/operations/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ export const findOperation = async <
doc._isLocked = !!lockedDoc
doc._userEditing = lockedDoc ? lockedDoc?.user?.value : null
}
} catch (error) {
} catch (_err) {
for (const doc of result.docs) {
doc._isLocked = false
doc._userEditing = null
Expand Down
1 change: 1 addition & 0 deletions packages/payload/src/fields/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ export type FieldHookArgs<TData extends TypeWithID = any, TValue = any, TSibling
findMany?: boolean
/** The global which the field belongs to. If the field belongs to a collection, this will be null. */
global: null | SanitizedGlobalConfig
indexPath: number[]
/** A string relating to which operation the field type is currently executing within. Useful within beforeValidate, beforeChange, and afterChange hooks to differentiate between create and update operations. */
operation?: 'create' | 'delete' | 'read' | 'update'
/** The full original document in `update` operations. In the `afterChange` hook, this is the resulting document of the operation. */
Expand Down
46 changes: 40 additions & 6 deletions packages/payload/src/fields/getFieldPaths.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
import type { ClientField, Field, TabAsField, TabAsFieldClient } from './config/types.js'
import type { ClientField, Field, Tab, TabAsFieldClient } from './config/types.js'

type Args = {
field: ClientField | Field | TabAsField | TabAsFieldClient
field: ClientField | Field | Tab | TabAsFieldClient
index: number
parentIndexPath: string
parentPath: string
parentSchemaPath: string
}

type Result = {
type FieldPaths = {
/**
* A string of '-' separated indexes representing where
* to find this field in a given field schema array.
* It will always be complete and accurate.
*/
indexPath: string
/**
* Path for this field specifically.
* Path for this field relative to its position in the data.
*/
path: string
/**
* Schema path for this field specifically.
* Path for this field relative to its position in the schema.
*/
schemaPath: string
}
Expand All @@ -31,7 +31,7 @@ export function getFieldPaths({
parentIndexPath,
parentPath,
parentSchemaPath,
}: Args): Result {
}: Args): FieldPaths {
if ('name' in field) {
return {
indexPath: `${parentIndexPath ? parentIndexPath + '-' : ''}${index}`,
Expand All @@ -48,3 +48,37 @@ export function getFieldPaths({
schemaPath: `${parentSchemaPath ? parentSchemaPath + '.' : ''}${indexSuffix}`,
}
}

export function getFieldPathsModified({
field,
index,
parentIndexPath,
parentPath,
parentSchemaPath,
}: Args): FieldPaths {
const parentPathSegments = parentPath.split('.')

const parentIsUnnamed = parentPathSegments[parentPathSegments.length - 1].startsWith('_index-')

const parentWithoutIndex = parentIsUnnamed
? parentPathSegments.slice(0, -1).join('.')
: parentPath

const parentPathToUse = parentIsUnnamed ? parentWithoutIndex : parentPath

if ('name' in field) {
return {
indexPath: '',
path: `${parentPathToUse ? parentPathToUse + '.' : ''}${field.name}`,
schemaPath: `${parentSchemaPath ? parentSchemaPath + '.' : ''}${field.name}`,
}
}

const indexSuffix = `_index-${`${parentIndexPath ? parentIndexPath + '-' : ''}${index}`}`

return {
indexPath: `${parentIndexPath ? parentIndexPath + '-' : ''}${index}`,
path: `${parentPathToUse ? parentPathToUse + '.' : ''}${indexSuffix}`,
schemaPath: `${!parentIsUnnamed && parentSchemaPath ? parentSchemaPath + '.' : ''}${indexSuffix}`,
}
}
5 changes: 3 additions & 2 deletions packages/payload/src/fields/hooks/afterChange/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,12 @@ export const afterChange = async <T extends JsonObject>({
fields: collection?.fields || global?.fields,
global,
operation,
path: [],
parentIndexPath: '',
parentPath: '',
parentSchemaPath: '',
previousDoc,
previousSiblingDoc: previousDoc,
req,
schemaPath: [],
siblingData: data,
siblingDoc: incomingDoc,
})
Expand Down
90 changes: 52 additions & 38 deletions packages/payload/src/fields/hooks/afterChange/promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { Field, TabAsField } from '../../config/types.js'

import { MissingEditorProp } from '../../../errors/index.js'
import { fieldAffectsData, tabHasName } from '../../config/types.js'
import { getFieldPaths } from '../../getFieldPaths.js'
import { getFieldPathsModified as getFieldPaths } from '../../getFieldPaths.js'
import { traverseFields } from './traverseFields.js'

type Args = {
Expand All @@ -19,14 +19,9 @@ type Args = {
fieldIndex: number
global: null | SanitizedGlobalConfig
operation: 'create' | 'update'
/**
* The parent's path
*/
parentPath: (number | string)[]
/**
* The parent's schemaPath (path without indexes).
*/
parentSchemaPath: string[]
parentIndexPath: string
parentPath: string
parentSchemaPath: string
previousDoc: JsonObject
previousSiblingDoc: JsonObject
req: PayloadRequest
Expand All @@ -46,6 +41,7 @@ export const promise = async ({
fieldIndex,
global,
operation,
parentIndexPath,
parentPath,
parentSchemaPath,
previousDoc,
Expand All @@ -54,15 +50,17 @@ export const promise = async ({
siblingData,
siblingDoc,
}: Args): Promise<void> => {
const { path: _fieldPath, schemaPath: _fieldSchemaPath } = getFieldPaths({
const { indexPath, path, schemaPath } = getFieldPaths({
field,
index: fieldIndex,
parentIndexPath: '', // Doesn't matter, as unnamed fields do not affect data, and hooks are only run on fields that affect data
parentPath: parentPath.join('.'),
parentSchemaPath: parentSchemaPath.join('.'),
parentIndexPath,
parentPath,
parentSchemaPath,
})
const fieldPath = _fieldPath ? _fieldPath.split('.') : []
const fieldSchemaPath = _fieldSchemaPath ? _fieldSchemaPath.split('.') : []

const pathSegments = path ? path.split('.') : []
const schemaPathSegments = schemaPath ? schemaPath.split('.') : []
const indexPathSegments = indexPath ? indexPath.split('-').filter(Boolean)?.map(Number) : []

if (fieldAffectsData(field)) {
// Execute hooks
Expand All @@ -76,14 +74,15 @@ export const promise = async ({
data,
field,
global,
indexPath: indexPathSegments,
operation,
originalDoc: doc,
path: fieldPath,
path: pathSegments,
previousDoc,
previousSiblingDoc,
previousValue: previousDoc[field.name],
req,
schemaPath: fieldSchemaPath,
schemaPath: schemaPathSegments,
siblingData,
value: siblingDoc[field.name],
})
Expand All @@ -102,7 +101,7 @@ export const promise = async ({

if (Array.isArray(rows)) {
const promises = []
rows.forEach((row, i) => {
rows.forEach((row, rowIndex) => {
promises.push(
traverseFields({
collection,
Expand All @@ -112,18 +111,20 @@ export const promise = async ({
fields: field.fields,
global,
operation,
path: [...fieldPath, i],
parentIndexPath: '',
parentPath: path + '.' + rowIndex,
parentSchemaPath: schemaPath,
previousDoc,
previousSiblingDoc: previousDoc?.[field.name]?.[i] || ({} as JsonObject),
previousSiblingDoc: previousDoc?.[field.name]?.[rowIndex] || ({} as JsonObject),
req,
schemaPath: fieldSchemaPath,
siblingData: siblingData?.[field.name]?.[i] || {},
siblingData: siblingData?.[field.name]?.[rowIndex] || {},
siblingDoc: row ? { ...row } : {},
}),
)
})
await Promise.all(promises)
}

break
}

Expand All @@ -132,7 +133,8 @@ export const promise = async ({

if (Array.isArray(rows)) {
const promises = []
rows.forEach((row, i) => {

rows.forEach((row, rowIndex) => {
const block = field.blocks.find(
(blockType) => blockType.slug === (row as JsonObject).blockType,
)
Expand All @@ -147,17 +149,19 @@ export const promise = async ({
fields: block.fields,
global,
operation,
path: [...fieldPath, i],
parentIndexPath: '',
parentPath: path + '.' + rowIndex,
parentSchemaPath: schemaPath + '.' + block.slug,
previousDoc,
previousSiblingDoc: previousDoc?.[field.name]?.[i] || ({} as JsonObject),
previousSiblingDoc: previousDoc?.[field.name]?.[rowIndex] || ({} as JsonObject),
req,
schemaPath: fieldSchemaPath,
siblingData: siblingData?.[field.name]?.[i] || {},
siblingData: siblingData?.[field.name]?.[rowIndex] || {},
siblingDoc: row ? { ...row } : {},
}),
)
}
})

await Promise.all(promises)
}

Expand All @@ -174,17 +178,19 @@ export const promise = async ({
fields: field.fields,
global,
operation,
path: fieldPath,
parentIndexPath: indexPath,
parentPath,
parentSchemaPath: schemaPath,
previousDoc,
previousSiblingDoc: { ...previousSiblingDoc },
req,
schemaPath: fieldSchemaPath,
siblingData: siblingData || {},
siblingDoc: { ...siblingDoc },
})

break
}

case 'group': {
await traverseFields({
collection,
Expand All @@ -194,11 +200,12 @@ export const promise = async ({
fields: field.fields,
global,
operation,
path: fieldPath,
parentIndexPath: '',
parentPath: path,
parentSchemaPath: schemaPath,
previousDoc,
previousSiblingDoc: previousDoc[field.name] as JsonObject,
req,
schemaPath: fieldSchemaPath,
siblingData: (siblingData?.[field.name] as JsonObject) || {},
siblingDoc: siblingDoc[field.name] as JsonObject,
})
Expand All @@ -210,6 +217,7 @@ export const promise = async ({
if (!field?.editor) {
throw new MissingEditorProp(field) // while we allow disabling editor functionality, you should not have any richText fields defined if you do not have an editor
}

if (typeof field?.editor === 'function') {
throw new Error('Attempted to access unsanitized rich text editor.')
}
Expand All @@ -226,14 +234,15 @@ export const promise = async ({
data,
field,
global,
indexPath: indexPathSegments,
operation,
originalDoc: doc,
path: fieldPath,
path: pathSegments,
previousDoc,
previousSiblingDoc,
previousValue: previousDoc[field.name],
req,
schemaPath: fieldSchemaPath,
schemaPath: schemaPathSegments,
siblingData,
value: siblingDoc[field.name],
})
Expand All @@ -251,7 +260,9 @@ export const promise = async ({
let tabSiblingDoc = siblingDoc
let tabPreviousSiblingDoc = siblingDoc

if (tabHasName(field)) {
const isNamedTab = tabHasName(field)

if (isNamedTab) {
tabSiblingData = (siblingData[field.name] as JsonObject) ?? {}
tabSiblingDoc = (siblingDoc[field.name] as JsonObject) ?? {}
tabPreviousSiblingDoc = (previousDoc[field.name] as JsonObject) ?? {}
Expand All @@ -265,11 +276,12 @@ export const promise = async ({
fields: field.fields,
global,
operation,
path: fieldPath,
parentIndexPath: isNamedTab ? '' : indexPath,
parentPath: isNamedTab ? path : parentPath,
parentSchemaPath: schemaPath,
previousDoc,
previousSiblingDoc: tabPreviousSiblingDoc,
req,
schemaPath: fieldSchemaPath,
siblingData: tabSiblingData,
siblingDoc: tabSiblingDoc,
})
Expand All @@ -286,14 +298,16 @@ export const promise = async ({
fields: field.tabs.map((tab) => ({ ...tab, type: 'tab' })),
global,
operation,
path: fieldPath,
parentIndexPath: indexPath,
parentPath: path,
parentSchemaPath: schemaPath,
previousDoc,
previousSiblingDoc: { ...previousSiblingDoc },
req,
schemaPath: fieldSchemaPath,
siblingData: siblingData || {},
siblingDoc: { ...siblingDoc },
})

break
}

Expand Down
Loading

0 comments on commit 0acaf8a

Please sign in to comment.