Skip to content

Commit

Permalink
fix: safely executes form state conditions, validations, and default …
Browse files Browse the repository at this point in the history
…values (#10275)

Whenever form state fails, like when field conditions, validations, or
default value functions throw errors, blocks and array rows are stuck
within an infinite loading state. Examples of this might be when
accessing properties of undefined within these functions, etc. Although
these errors are logged to the server console, the UI is be misleading,
where the user often waits for the request to resolve rather than
understanding that an underlying API error has occurred. Now, we safely
execute these functions within a `try...catch` block and handle their
failures accordingly. On the client, form state will resolve as expected
using the default return values for these functions.
  • Loading branch information
jacobsfletch authored Jan 2, 2025
1 parent 2ae7d8e commit 7928eca
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export type AddFieldStatePromiseArgs = {
* Whether the field schema should be included in the state
*/
includeSchema?: boolean
indexPath: string
/**
* Whether to omit parent fields in the state. @default false
*/
Expand All @@ -69,6 +70,7 @@ export type AddFieldStatePromiseArgs = {
parentPermissions: SanitizedFieldsPermissions
parentSchemaPath: string
passesCondition: boolean
path: string
preferences: DocumentPreferences
previousFormState: FormState
renderAllFields: boolean
Expand All @@ -78,6 +80,7 @@ export type AddFieldStatePromiseArgs = {
* just create your own req and pass in the locale and the user
*/
req: PayloadRequest
schemaPath: string
/**
* Whether to skip checking the field's condition. @default false
*/
Expand All @@ -102,24 +105,25 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
collectionSlug,
data,
field,
fieldIndex,
fieldSchemaMap,
filter,
forceFullValue = false,
fullData,
includeSchema = false,
indexPath,
omitParents = false,
operation,
parentIndexPath,
parentPath,
parentPermissions,
parentSchemaPath,
passesCondition,
path,
preferences,
previousFormState,
renderAllFields,
renderFieldFn,
req,
schemaPath,
skipConditionChecks = false,
skipValidation = false,
state,
Expand All @@ -131,14 +135,6 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
)
}

const { indexPath, path, schemaPath } = getFieldPaths({
field,
index: fieldIndex,
parentIndexPath: 'name' in field ? '' : parentIndexPath,
parentPath,
parentSchemaPath,
})

const requiresRender = renderAllFields || previousFormState?.[path]?.requiresRender

let fieldPermissions: SanitizedFieldPermissions = true
Expand Down Expand Up @@ -187,20 +183,29 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
}
}

validationResult = await validate(
data?.[field.name] as never,
{
...field,
id,
collectionSlug,
data: fullData,
jsonError,
operation,
preferences,
req,
siblingData: data,
} as any,
)
try {
validationResult = await validate(
data?.[field.name] as never,
{
...field,
id,
collectionSlug,
data: fullData,
jsonError,
operation,
preferences,
req,
siblingData: data,
} as any,
)
} catch (err) {
validationResult = `Error validating field at path: ${path}`

req.payload.logger.error({
err,
msg: validationResult,
})
}
}

const addErrorPathToParent = (errorPath: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,20 @@ export const defaultValuePromise = async <T>({
typeof siblingData[field.name] === 'undefined' &&
typeof field.defaultValue !== 'undefined'
) {
siblingData[field.name] = await getDefaultValue({
defaultValue: field.defaultValue,
locale,
req,
user,
value: siblingData[field.name],
})
try {
siblingData[field.name] = await getDefaultValue({
defaultValue: field.defaultValue,
locale,
req,
user,
value: siblingData[field.name],
})
} catch (err) {
req.payload.logger.error({
err,
msg: `Error calculating default value for field: ${field.name}`,
})
}
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/forms/fieldSchemasToFormState/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const fieldSchemasToFormState = async (args: Args): Promise<FormState> =>
'clientFieldSchemaMap is not passed to fieldSchemasToFormState - this will reduce performance',
)
}

const {
id,
clientFieldSchemaMap,
Expand Down
32 changes: 27 additions & 5 deletions packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import type {
SanitizedFieldsPermissions,
} from 'payload'

import { getFieldPaths } from 'payload/shared'

import type { AddFieldStatePromiseArgs } from './addFieldStatePromise.js'
import type { RenderFieldMethod } from './types.js'

Expand Down Expand Up @@ -103,12 +105,29 @@ export const iterateFields = async ({
fields.forEach((field, fieldIndex) => {
let passesCondition = true

const { indexPath, path, schemaPath } = getFieldPaths({
field,
index: fieldIndex,
parentIndexPath: 'name' in field ? '' : parentIndexPath,
parentPath,
parentSchemaPath,
})

if (!skipConditionChecks) {
passesCondition = Boolean(
(field?.admin?.condition
? Boolean(field.admin.condition(fullData || {}, data || {}, { user: req.user }))
: true) && parentPassesCondition,
)
try {
passesCondition = Boolean(
(field?.admin?.condition
? Boolean(field.admin.condition(fullData || {}, data || {}, { user: req.user }))
: true) && parentPassesCondition,
)
} catch (err) {
passesCondition = false

req.payload.logger.error({
err,
msg: `Error evaluating field condition at path: ${path}`,
})
}
}

promises.push(
Expand All @@ -126,18 +145,21 @@ export const iterateFields = async ({
forceFullValue,
fullData,
includeSchema,
indexPath,
omitParents,
operation,
parentIndexPath,
parentPath,
parentPermissions: permissions,
parentSchemaPath,
passesCondition,
path,
preferences,
previousFormState,
renderAllFields,
renderFieldFn,
req,
schemaPath,
skipConditionChecks,
skipValidation,
state,
Expand Down
13 changes: 8 additions & 5 deletions packages/ui/src/providers/ServerFunctions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
BuildTableStateArgs,
Data,
DocumentSlots,
ErrorResult,
Locale,
ServerFunctionClient,
} from 'payload'
Expand Down Expand Up @@ -46,7 +47,9 @@ type RenderDocument = (args: {
redirectAfterDelete?: boolean
redirectAfterDuplicate?: boolean
signal?: AbortSignal
}) => Promise<{ data: Data; Document: React.ReactNode }>
}) => Promise<
{ data: Data; Document: React.ReactNode } | ({ data: never; Document: never } & ErrorResult)
>

type CopyDataFromLocaleClient = (
args: {
Expand Down Expand Up @@ -107,7 +110,7 @@ export const ServerFunctionsProvider: React.FC<{
const result = (await serverFunction({
name: 'schedule-publish',
args: { ...rest },
})) as ReturnType<typeof schedulePublishHandler> // TODO: infer this type when `strictNullChecks` is enabled
})) as Awaited<ReturnType<typeof schedulePublishHandler>> // TODO: infer this type when `strictNullChecks` is enabled

if (!remoteSignal?.aborted) {
return result
Expand Down Expand Up @@ -137,7 +140,7 @@ export const ServerFunctionsProvider: React.FC<{
const result = (await serverFunction({
name: 'form-state',
args: { fallbackLocale: false, ...rest },
})) as ReturnType<typeof buildFormStateHandler> // TODO: infer this type when `strictNullChecks` is enabled
})) as Awaited<ReturnType<typeof buildFormStateHandler>> // TODO: infer this type when `strictNullChecks` is enabled

if (!remoteSignal?.aborted) {
return result
Expand All @@ -161,7 +164,7 @@ export const ServerFunctionsProvider: React.FC<{
const result = (await serverFunction({
name: 'table-state',
args: { fallbackLocale: false, ...rest },
})) as ReturnType<typeof buildTableStateHandler> // TODO: infer this type when `strictNullChecks` is enabled
})) as Awaited<ReturnType<typeof buildTableStateHandler>> // TODO: infer this type when `strictNullChecks` is enabled

if (!remoteSignal?.aborted) {
return result
Expand All @@ -184,7 +187,7 @@ export const ServerFunctionsProvider: React.FC<{
const result = (await serverFunction({
name: 'render-document',
args: { fallbackLocale: false, ...rest },
})) as { data: Data; Document: React.ReactNode }
})) as Awaited<ReturnType<typeof renderDocument>> // TODO: infer this type when `strictNullChecks` is enabled

return result
} catch (_err) {
Expand Down
2 changes: 1 addition & 1 deletion test/_community/payload-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,4 +336,4 @@ export interface Auth {
declare module 'payload' {
// @ts-ignore
export interface GeneratedTypes extends Config {}
}
}
6 changes: 6 additions & 0 deletions test/joins/payload-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ export interface UserAuthOperations {
export interface Post {
id: string;
title?: string | null;
/**
* Hides posts for the `filtered` join field in categories
*/
isFiltered?: boolean | null;
restrictedField?: string | null;
upload?: (string | null) | Upload;
Expand Down Expand Up @@ -228,6 +231,9 @@ export interface Category {
docs?: (string | Post)[] | null;
hasNextPage?: boolean | null;
} | null;
/**
* Static Description
*/
hasManyPosts?: {
docs?: (string | Post)[] | null;
hasNextPage?: boolean | null;
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
}
],
"paths": {
"@payload-config": ["./test/admin/config.ts"],
"@payload-config": ["./test/_community/config.ts"],
"@payloadcms/live-preview": ["./packages/live-preview/src"],
"@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"],
"@payloadcms/live-preview-vue": ["./packages/live-preview-vue/src/index.ts"],
Expand Down

0 comments on commit 7928eca

Please sign in to comment.