From 7928ecaee7ffefb8c042f0606c662bb390a75a72 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Thu, 2 Jan 2025 14:12:28 -0500 Subject: [PATCH] fix: safely executes form state conditions, validations, and default 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. --- .../addFieldStatePromise.ts | 53 ++++++++++--------- .../calculateDefaultValues/promise.ts | 21 +++++--- .../forms/fieldSchemasToFormState/index.tsx | 1 + .../fieldSchemasToFormState/iterateFields.ts | 32 +++++++++-- .../src/providers/ServerFunctions/index.tsx | 13 +++-- test/_community/payload-types.ts | 2 +- test/joins/payload-types.ts | 6 +++ tsconfig.base.json | 2 +- 8 files changed, 87 insertions(+), 43 deletions(-) diff --git a/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts b/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts index 4e73cef360..d0842b8e10 100644 --- a/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts +++ b/packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts @@ -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 */ @@ -69,6 +70,7 @@ export type AddFieldStatePromiseArgs = { parentPermissions: SanitizedFieldsPermissions parentSchemaPath: string passesCondition: boolean + path: string preferences: DocumentPreferences previousFormState: FormState renderAllFields: boolean @@ -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 */ @@ -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, @@ -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 @@ -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) => { diff --git a/packages/ui/src/forms/fieldSchemasToFormState/calculateDefaultValues/promise.ts b/packages/ui/src/forms/fieldSchemasToFormState/calculateDefaultValues/promise.ts index 6d5aafbe63..d9507d247d 100644 --- a/packages/ui/src/forms/fieldSchemasToFormState/calculateDefaultValues/promise.ts +++ b/packages/ui/src/forms/fieldSchemasToFormState/calculateDefaultValues/promise.ts @@ -30,13 +30,20 @@ export const defaultValuePromise = async ({ 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}`, + }) + } } } diff --git a/packages/ui/src/forms/fieldSchemasToFormState/index.tsx b/packages/ui/src/forms/fieldSchemasToFormState/index.tsx index 15fe1acc12..38f06f2724 100644 --- a/packages/ui/src/forms/fieldSchemasToFormState/index.tsx +++ b/packages/ui/src/forms/fieldSchemasToFormState/index.tsx @@ -57,6 +57,7 @@ export const fieldSchemasToFormState = async (args: Args): Promise => 'clientFieldSchemaMap is not passed to fieldSchemasToFormState - this will reduce performance', ) } + const { id, clientFieldSchemaMap, diff --git a/packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts b/packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts index d25621d75d..c26b4909f6 100644 --- a/packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts +++ b/packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts @@ -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' @@ -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( @@ -126,6 +145,7 @@ export const iterateFields = async ({ forceFullValue, fullData, includeSchema, + indexPath, omitParents, operation, parentIndexPath, @@ -133,11 +153,13 @@ export const iterateFields = async ({ parentPermissions: permissions, parentSchemaPath, passesCondition, + path, preferences, previousFormState, renderAllFields, renderFieldFn, req, + schemaPath, skipConditionChecks, skipValidation, state, diff --git a/packages/ui/src/providers/ServerFunctions/index.tsx b/packages/ui/src/providers/ServerFunctions/index.tsx index 65d596e743..64573a7e9d 100644 --- a/packages/ui/src/providers/ServerFunctions/index.tsx +++ b/packages/ui/src/providers/ServerFunctions/index.tsx @@ -3,6 +3,7 @@ import type { BuildTableStateArgs, Data, DocumentSlots, + ErrorResult, Locale, ServerFunctionClient, } from 'payload' @@ -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: { @@ -107,7 +110,7 @@ export const ServerFunctionsProvider: React.FC<{ const result = (await serverFunction({ name: 'schedule-publish', args: { ...rest }, - })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled + })) as Awaited> // TODO: infer this type when `strictNullChecks` is enabled if (!remoteSignal?.aborted) { return result @@ -137,7 +140,7 @@ export const ServerFunctionsProvider: React.FC<{ const result = (await serverFunction({ name: 'form-state', args: { fallbackLocale: false, ...rest }, - })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled + })) as Awaited> // TODO: infer this type when `strictNullChecks` is enabled if (!remoteSignal?.aborted) { return result @@ -161,7 +164,7 @@ export const ServerFunctionsProvider: React.FC<{ const result = (await serverFunction({ name: 'table-state', args: { fallbackLocale: false, ...rest }, - })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled + })) as Awaited> // TODO: infer this type when `strictNullChecks` is enabled if (!remoteSignal?.aborted) { return result @@ -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> // TODO: infer this type when `strictNullChecks` is enabled return result } catch (_err) { diff --git a/test/_community/payload-types.ts b/test/_community/payload-types.ts index a259f6c7c5..fd6483846a 100644 --- a/test/_community/payload-types.ts +++ b/test/_community/payload-types.ts @@ -336,4 +336,4 @@ export interface Auth { declare module 'payload' { // @ts-ignore export interface GeneratedTypes extends Config {} -} +} \ No newline at end of file diff --git a/test/joins/payload-types.ts b/test/joins/payload-types.ts index 394f98fffc..241b99ec4e 100644 --- a/test/joins/payload-types.ts +++ b/test/joins/payload-types.ts @@ -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; @@ -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; diff --git a/tsconfig.base.json b/tsconfig.base.json index 872fb4ed36..01f5644941 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -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"],