fix: simplifies field error paths (#5504)

2024-03-28 13:15:44 -04:00
parent 5873dfb731
commit 8636685252
17 changed files with 97 additions and 91 deletions
--- a/packages/payload/src/admin/forms/Form.ts
+++ b/packages/payload/src/admin/forms/Form.ts
@@ -8,7 +8,6 @@ export type Data = {
 export type Row = {
  blockType?: string
  collapsed?: boolean
-  errorPaths?: string[]
  id: string
 }

--- a/packages/payload/src/admin/forms/Label.ts
+++ b/packages/payload/src/admin/forms/Label.ts
@@ -1,5 +1,6 @@
 export type LabelProps = {
  CustomLabel?: React.ReactNode
+  as?: 'label' | 'span'
  htmlFor?: string
  label?: Record<string, string> | false | string
  required?: boolean
--- a/packages/payload/src/fields/validations.ts
+++ b/packages/payload/src/fields/validations.ts
@@ -229,7 +229,7 @@ const validateArrayLength = (
 ) => {
  const { maxRows, minRows, required, t } = options

-  const arrayLength = Array.isArray(value) ? value.length : 0
+  const arrayLength = Array.isArray(value) ? value.length : value || 0

  if (!required && arrayLength === 0) return true

--- a/packages/ui/src/fields/Array/ArrayRow.tsx
+++ b/packages/ui/src/fields/Array/ArrayRow.tsx
@@ -22,6 +22,7 @@ type ArrayRowProps = UseDraggableSortableReturn & {
  CustomRowLabel?: React.ReactNode
  addRow: (rowIndex: number) => void
  duplicateRow: (rowIndex: number) => void
+  errorCount: number
  fieldMap: FieldMap
  forceRender?: boolean
  hasMaxRows?: boolean
@@ -35,6 +36,7 @@ type ArrayRowProps = UseDraggableSortableReturn & {
  row: Row
  rowCount: number
  rowIndex: number
+  schemaPath: string
  setCollapse: (rowID: string, collapsed: boolean) => void
 }

@@ -43,6 +45,7 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
  addRow,
  attributes,
  duplicateRow,
+  errorCount,
  fieldMap,
  forceRender = false,
  hasMaxRows,
@@ -57,6 +60,7 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
  row,
  rowCount,
  rowIndex,
+  schemaPath,
  setCollapse,
  setNodeRef,
  transform,
@@ -70,7 +74,6 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
    '0',
  )}`

-  const errorCount = row.errorPaths?.length
  const fieldHasErrors = errorCount > 0 && hasSubmitted

  const classNames = [
@@ -134,7 +137,7 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
          path={path}
          permissions={permissions?.fields}
          readOnly={readOnly}
-          schemaPath={parentPath}
+          schemaPath={schemaPath}
        />
      </Collapsible>
    </div>
--- a/packages/ui/src/fields/Array/index.tsx
+++ b/packages/ui/src/fields/Array/index.tsx
@@ -112,6 +112,7 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {
  const { path: pathFromContext } = useFieldProps()

  const {
+    errorPaths,
    path,
    rows = [],
    schemaPath,
@@ -180,10 +181,8 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {

  const hasMaxRows = maxRows && rows.length >= maxRows

-  const fieldErrorCount =
-    rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0) + (valid ? 0 : 1)
-
-  const fieldHasErrors = submitted && fieldErrorCount > 0
+  const fieldErrorCount = errorPaths.length
+  const fieldHasErrors = submitted && errorPaths.length > 0

  const showRequired = readOnly && rows.length === 0
  const showMinRows = rows.length < minRows || (required && rows.length === 0)
@@ -209,7 +208,7 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {
        <div className={`${baseClass}__header-wrap`}>
          <div className={`${baseClass}__header-content`}>
            <h3 className={`${baseClass}__title`}>
-              <FieldLabel CustomLabel={CustomLabel} {...(labelProps || {})} />
+              <FieldLabel CustomLabel={CustomLabel} as="span" unstyled {...(labelProps || {})} />
            </h3>
            {fieldHasErrors && fieldErrorCount > 0 && (
              <ErrorPill count={fieldErrorCount} i18n={i18n} withMessage />
@@ -247,32 +246,39 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {
          ids={rows.map((row) => row.id)}
          onDragEnd={({ moveFromIndex, moveToIndex }) => moveRow(moveFromIndex, moveToIndex)}
        >
-          {rows.map((row, i) => (
-            <DraggableSortableItem disabled={readOnly} id={row.id} key={row.id}>
-              {(draggableSortableItemProps) => (
-                <ArrayRow
-                  {...draggableSortableItemProps}
-                  CustomRowLabel={CustomRowLabel}
-                  addRow={addRow}
-                  duplicateRow={duplicateRow}
-                  fieldMap={fieldMap}
-                  forceRender={forceRender}
-                  hasMaxRows={hasMaxRows}
-                  indexPath={indexPath}
-                  labels={labels}
-                  moveRow={moveRow}
-                  path={path}
-                  permissions={permissions}
-                  readOnly={readOnly}
-                  removeRow={removeRow}
-                  row={row}
-                  rowCount={rows.length}
-                  rowIndex={i}
-                  setCollapse={setCollapse}
-                />
-              )}
-            </DraggableSortableItem>
-          ))}
+          {rows.map((row, i) => {
+            const rowErrorCount = errorPaths?.filter((errorPath) =>
+              errorPath.startsWith(`${path}.${i}.`),
+            ).length
+            return (
+              <DraggableSortableItem disabled={readOnly} id={row.id} key={row.id}>
+                {(draggableSortableItemProps) => (
+                  <ArrayRow
+                    {...draggableSortableItemProps}
+                    CustomRowLabel={CustomRowLabel}
+                    addRow={addRow}
+                    duplicateRow={duplicateRow}
+                    errorCount={rowErrorCount}
+                    fieldMap={fieldMap}
+                    forceRender={forceRender}
+                    hasMaxRows={hasMaxRows}
+                    indexPath={indexPath}
+                    labels={labels}
+                    moveRow={moveRow}
+                    path={path}
+                    permissions={permissions}
+                    readOnly={readOnly}
+                    removeRow={removeRow}
+                    row={row}
+                    rowCount={rows.length}
+                    rowIndex={i}
+                    schemaPath={schemaPath}
+                    setCollapse={setCollapse}
+                  />
+                )}
+              </DraggableSortableItem>
+            )
+          })}
          {!valid && (
            <React.Fragment>
              {showRequired && (
--- a/packages/ui/src/fields/Blocks/BlockRow.tsx
+++ b/packages/ui/src/fields/Blocks/BlockRow.tsx
@@ -24,6 +24,7 @@ type BlockFieldProps = UseDraggableSortableReturn & {
  block: ReducedBlock
  blocks: ReducedBlock[]
  duplicateRow: (rowIndex: number) => void
+  errorCount: number
  forceRender?: boolean
  hasMaxRows?: boolean
  indexPath: string
@@ -46,6 +47,7 @@ export const BlockRow: React.FC<BlockFieldProps> = ({
  block,
  blocks,
  duplicateRow,
+  errorCount,
  forceRender,
  hasMaxRows,
  labels,
@@ -67,8 +69,7 @@ export const BlockRow: React.FC<BlockFieldProps> = ({
  const { i18n } = useTranslation()
  const hasSubmitted = useFormSubmitted()

-  const errorCount = row.errorPaths?.length
-  const fieldHasErrors = errorCount > 0 && hasSubmitted
+  const fieldHasErrors = hasSubmitted && errorCount > 0

  const classNames = [
    `${baseClass}__row`,
--- a/packages/ui/src/fields/Blocks/index.tsx
+++ b/packages/ui/src/fields/Blocks/index.tsx
@@ -113,6 +113,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
  const { path: pathFromContext } = useFieldProps()

  const {
+    errorPaths,
    path,
    permissions,
    rows = [],
@@ -192,7 +193,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {

  const hasMaxRows = maxRows && rows.length >= maxRows

-  const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0)
+  const fieldErrorCount = errorPaths.length
  const fieldHasErrors = submitted && fieldErrorCount + (valid ? 0 : 1) > 0

  const showMinRows = rows.length < minRows || (required && rows.length === 0)
@@ -219,7 +220,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
        <div className={`${baseClass}__header-wrap`}>
          <div className={`${baseClass}__heading-with-error`}>
            <h3>
-              <FieldLabel CustomLabel={CustomLabel} {...(labelProps || {})} />
+              <FieldLabel CustomLabel={CustomLabel} as="span" unstyled {...(labelProps || {})} />
            </h3>
            {fieldHasErrors && fieldErrorCount > 0 && (
              <ErrorPill count={fieldErrorCount} i18n={i18n} withMessage />
@@ -262,6 +263,9 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
            const blockToRender = blocks.find((block) => block.slug === blockType)

            if (blockToRender) {
+              const rowErrorCount = errorPaths.filter((errorPath) =>
+                errorPath.startsWith(`${path}.${i}`),
+              ).length
              return (
                <DraggableSortableItem disabled={readOnly} id={row.id} key={row.id}>
                  {(draggableSortableItemProps) => (
@@ -271,6 +275,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
                      block={blockToRender}
                      blocks={blocks}
                      duplicateRow={duplicateRow}
+                      errorCount={rowErrorCount}
                      forceRender={forceRender}
                      hasMaxRows={hasMaxRows}
                      indexPath={indexPath}
--- a/packages/ui/src/fields/Collapsible/index.tsx
+++ b/packages/ui/src/fields/Collapsible/index.tsx
@@ -1,5 +1,5 @@
 'use client'
-import type { DocumentPreferences, FieldBase, RowLabel as RowLabelType } from 'payload/types'
+import type { DocumentPreferences, FieldBase } from 'payload/types'

 import React, { Fragment, useCallback, useEffect, useState } from 'react'

--- a/packages/ui/src/fields/Group/index.tsx
+++ b/packages/ui/src/fields/Group/index.tsx
@@ -12,8 +12,9 @@ import type { FormFieldBase } from '../shared/index.js'
 import { useCollapsible } from '../../elements/Collapsible/provider.js'
 import { ErrorPill } from '../../elements/ErrorPill/index.js'
 import { useFieldProps } from '../../forms/FieldPropsProvider/index.js'
+import { useFormSubmitted } from '../../forms/Form/context.js'
 import { RenderFields } from '../../forms/RenderFields/index.js'
-import { WatchChildErrors } from '../../forms/WatchChildErrors/index.js'
+import { useField } from '../../forms/useField/index.js'
 import { withCondition } from '../../forms/withCondition/index.js'
 import { useTranslation } from '../../providers/Translation/index.js'
 import { useRow } from '../Row/provider.js'
@@ -53,14 +54,15 @@ const GroupField: React.FC<GroupFieldProps> = (props) => {
  const isWithinGroup = useGroup()
  const isWithinRow = useRow()
  const isWithinTab = useTabs()
-  const [errorCount, setErrorCount] = React.useState(undefined)
-  const fieldHasErrors = errorCount > 0
+  const { errorPaths } = useField({ path })
+  const submitted = useFormSubmitted()
+  const errorCount = errorPaths.length
+  const fieldHasErrors = submitted && errorCount > 0

  const isTopLevel = !(isWithinCollapsible || isWithinGroup || isWithinRow)

  return (
    <Fragment>
-      <WatchChildErrors fieldMap={fieldMap} path={path} setErrorCount={setErrorCount} />
      <div
        className={[
          fieldBaseClass,
--- a/packages/ui/src/forms/FieldLabel/index.tsx
+++ b/packages/ui/src/forms/FieldLabel/index.tsx
@@ -13,6 +13,7 @@ import './index.scss'

 const DefaultFieldLabel: React.FC<LabelProps> = (props) => {
  const {
+    as: Element = 'label',
    htmlFor: htmlForFromProps,
    label: labelFromProps,
    required = false,
@@ -27,10 +28,10 @@ const DefaultFieldLabel: React.FC<LabelProps> = (props) => {

  if (labelFromProps) {
    return (
-      <label className={`field-label ${unstyled ? 'unstyled' : ''}`} htmlFor={htmlFor}>
+      <Element className={`field-label ${unstyled ? 'unstyled' : ''}`} htmlFor={htmlFor}>
        {getTranslation(labelFromProps, i18n)}
        {required && !unstyled && <span className="required">*</span>}
-      </label>
+      </Element>
    )
  }

--- a/packages/ui/src/forms/Form/mergeServerFormState.ts
+++ b/packages/ui/src/forms/Form/mergeServerFormState.ts
@@ -52,30 +52,6 @@ export const mergeServerFormState = (
        }
      })

-      /**
-       * 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++
-        }
-      }
-
      // Conditions don't work if we don't memcopy the new state, as the object references would otherwise be the same
      newState[path] = { ...newFieldState }
    })
--- a/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts
+++ b/packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts
@@ -18,13 +18,14 @@ const ObjectId = (ObjectIdImport.default ||
  ObjectIdImport) as unknown as typeof ObjectIdImport.default

 export type AddFieldStatePromiseArgs = {
+  addErrorPathToParent: (path: string) => void
  /**
   * if all parents are localized, then the field is localized
   */
  anyParentLocalized?: boolean
  data: Data
-  errorPaths: string[]
  field: NonPresentationalField
+  fieldIndex: number
  /**
   * 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.
   */
@@ -72,9 +73,9 @@ export type AddFieldStatePromiseArgs = {
 export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Promise<void> => {
  const {
    id,
+    addErrorPathToParent: addErrorPathToParentArg,
    anyParentLocalized = false,
    data,
-    errorPaths: parentErrorPaths = [],
    field,
    filter,
    forceFullValue = false,
@@ -92,7 +93,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
  } = args

  if (fieldAffectsData(field)) {
-    const validate = operation === 'update' ? field.validate : undefined
+    const validate = field.validate

    const fieldState: FormField = {
      errorPaths: [],
@@ -139,14 +140,21 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
      })
    }

+    const addErrorPathToParent = (errorPath: string) => {
+      if (typeof addErrorPathToParentArg === 'function') {
+        addErrorPathToParentArg(errorPath)
+      }
+
+      if (!fieldState.errorPaths.includes(errorPath)) {
+        fieldState.errorPaths.push(errorPath)
+        fieldState.valid = false
+      }
+    }
+
    if (typeof validationResult === 'string') {
      fieldState.errorMessage = validationResult
      fieldState.valid = false
-      // TODO: this is unpredictable, need to figure out why
-      // It will sometimes lead to inconsistencies across re-renders
-      if (!parentErrorPaths.includes(`${path}${field.name}`)) {
-        parentErrorPaths.push(`${path}${field.name}`)
-      }
+      addErrorPathToParent(`${path}${field.name}`)
    } else {
      fieldState.valid = true
    }
@@ -174,9 +182,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
            acc.promises.push(
              iterateFields({
                id,
+                addErrorPathToParent,
                anyParentLocalized: field.localized || anyParentLocalized,
                data: row,
-                errorPaths: fieldState.errorPaths,
                fields: field.fields,
                filter,
                forceFullValue,
@@ -202,7 +210,6 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
                collapsedRowIDs === undefined
                  ? field.admin.initCollapsed
                  : collapsedRowIDs.includes(row.id),
-              errorPaths: fieldState.errorPaths,
            })

            return acc
@@ -287,9 +294,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
              acc.promises.push(
                iterateFields({
                  id,
+                  addErrorPathToParent,
                  anyParentLocalized: field.localized || anyParentLocalized,
                  data: row,
-                  errorPaths: fieldState.errorPaths,
                  fields: block.fields,
                  filter,
                  forceFullValue,
@@ -316,7 +323,6 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
                  collapsedRowIDs === undefined
                    ? field.admin.initCollapsed
                    : collapsedRowIDs.includes(row.id),
-                errorPaths: fieldState.errorPaths,
              })
            }

@@ -356,9 +362,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
      case 'group': {
        await iterateFields({
          id,
+          addErrorPathToParent,
          anyParentLocalized: field.localized || anyParentLocalized,
          data: data?.[field.name] || {},
-          errorPaths: parentErrorPaths,
          fields: field.fields,
          filter,
          forceFullValue,
@@ -491,9 +497,10 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
    // Handle field types that do not use names (row, etc)
    await iterateFields({
      id,
+      // passthrough parent functionality
+      addErrorPathToParent: addErrorPathToParentArg,
      anyParentLocalized: field.localized || anyParentLocalized,
      data,
-      errorPaths: parentErrorPaths,
      fields: field.fields,
      filter,
      forceFullValue,
@@ -513,9 +520,10 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
    const promises = field.tabs.map((tab) =>
      iterateFields({
        id,
+        // passthrough parent functionality
+        addErrorPathToParent: addErrorPathToParentArg,
        anyParentLocalized: tab.localized || anyParentLocalized,
        data: tabHasName(tab) ? data?.[tab.name] : data,
-        errorPaths: parentErrorPaths,
        fields: tab.fields,
        filter,
        forceFullValue,
--- a/packages/ui/src/forms/buildStateFromSchema/index.tsx
+++ b/packages/ui/src/forms/buildStateFromSchema/index.tsx
@@ -39,8 +39,8 @@ export const buildStateFromSchema = async (args: Args): Promise<FormState> => {

    await iterateFields({
      id,
+      addErrorPathToParent: null,
      data: fullData,
-      errorPaths: [],
      fields: fieldSchema,
      fullData,
      operation,
--- a/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts
+++ b/packages/ui/src/forms/buildStateFromSchema/iterateFields.ts
@@ -7,12 +7,12 @@ import type { AddFieldStatePromiseArgs } from './addFieldStatePromise.js'
 import { addFieldStatePromise } from './addFieldStatePromise.js'

 type Args = {
+  addErrorPathToParent: (path: string) => void
  /**
   * if any parents is localized, then the field is localized. @default false
   */
  anyParentLocalized?: boolean
  data: Data
-  errorPaths: string[]
  fields: FieldSchema[]
  filter?: (args: AddFieldStatePromiseArgs) => boolean
  /**
@@ -58,9 +58,9 @@ type Args = {
 */
 export const iterateFields = async ({
  id,
+  addErrorPathToParent: addErrorPathToParentArg,
  anyParentLocalized = false,
  data,
-  errorPaths,
  fields,
  filter,
  forceFullValue = false,
@@ -78,7 +78,7 @@ export const iterateFields = async ({
 }: Args): Promise<void> => {
  const promises = []

-  fields.forEach((field) => {
+  fields.forEach((field, fieldIndex) => {
    if (!fieldIsPresentationalOnly(field) && !field?.admin?.disabled) {
      let passesCondition = true
      if (!skipConditionChecks) {
@@ -92,10 +92,11 @@ export const iterateFields = async ({
      promises.push(
        addFieldStatePromise({
          id,
+          addErrorPathToParent: addErrorPathToParentArg,
          anyParentLocalized,
          data,
-          errorPaths,
          field,
+          fieldIndex,
          filter,
          forceFullValue,
          fullData,
--- a/packages/ui/src/forms/useField/index.tsx
+++ b/packages/ui/src/forms/useField/index.tsx
@@ -105,6 +105,7 @@ export const useField = <T,>(options: Options): FieldType<T> => {
  const result: FieldType<T> = useMemo(
    () => ({
      errorMessage: field?.errorMessage,
+      errorPaths: field?.errorPaths || [],
      filterOptions,
      formProcessing: processing,
      formSubmitted: submitted,
@@ -123,6 +124,7 @@ export const useField = <T,>(options: Options): FieldType<T> => {
      field?.errorMessage,
      field?.rows,
      field?.valid,
+      field?.errorPaths,
      processing,
      setValue,
      showError,
--- a/packages/ui/src/forms/useField/types.ts
+++ b/packages/ui/src/forms/useField/types.ts
@@ -12,6 +12,7 @@ export type Options = {

 export type FieldType<T> = {
  errorMessage?: string
+  errorPaths?: string[]
  filterOptions?: FilterOptionsResult
  formProcessing: boolean
  formSubmitted: boolean