Skip to content

Commit 285fc8c

Browse files
DallasOGermanJablo
andauthored
fix: presentational-field types incorrectly exposing hooks (#11514)
### What? Presentational Fields such as [Row](https://payloadcms.com/docs/fields/row) are described as only effecting the admin panel. If they do not impact data, their types should not include hooks in the fields config. ### Why? Developers can currently assign hooks to these fields, expecting them to work, when in reality they are not called. ### How? Omit `hooks` from `FieldBase` Fixes #11507 --------- Co-authored-by: German Jablonski <43938777+GermanJablo@users.noreply.github.com>
1 parent e40c821 commit 285fc8c

File tree

9 files changed

+39
-30
lines changed

9 files changed

+39
-30
lines changed

docs/fields/overview.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ Here are the available Presentational Fields:
9595

9696
- [Collapsible](../fields/collapsible) - nests fields within a collapsible component
9797
- [Row](../fields/row) - aligns fields horizontally
98-
- [Tabs (Unnamed)](../fields/tabs) - nests fields within a tabbed layout
98+
- [Tabs (Unnamed)](../fields/tabs) - nests fields within a tabbed layout. It is not presentational if the tab has a name.
99+
- [Group (Unnamed)](../fields/group) - nests fields within a keyed object It is not presentational if the group has a name.
99100
- [UI](../fields/ui) - blank field for custom UI components
100101

101102
### Virtual Fields

packages/payload/src/fields/config/types.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ export type NamedGroupField = {
753753
export type UnnamedGroupField = {
754754
interfaceName?: never
755755
localized?: never
756-
} & Omit<GroupBase, 'name' | 'virtual'>
756+
} & Omit<GroupBase, 'hooks' | 'name' | 'virtual'>
757757

758758
export type GroupField = NamedGroupField | UnnamedGroupField
759759

@@ -777,7 +777,7 @@ export type RowField = {
777777
admin?: Omit<Admin, 'description'>
778778
fields: Field[]
779779
type: 'row'
780-
} & Omit<FieldBase, 'admin' | 'label' | 'localized' | 'name' | 'validate' | 'virtual'>
780+
} & Omit<FieldBase, 'admin' | 'hooks' | 'label' | 'localized' | 'name' | 'validate' | 'virtual'>
781781

782782
export type RowFieldClient = {
783783
admin?: Omit<AdminClient, 'description'>
@@ -816,7 +816,7 @@ export type CollapsibleField = {
816816
label: Required<FieldBase['label']>
817817
}
818818
) &
819-
Omit<FieldBase, 'label' | 'localized' | 'name' | 'validate' | 'virtual'>
819+
Omit<FieldBase, 'hooks' | 'label' | 'localized' | 'name' | 'validate' | 'virtual'>
820820

821821
export type CollapsibleFieldClient = {
822822
admin?: {
@@ -863,7 +863,7 @@ export type UnnamedTab = {
863863
| LabelFunction
864864
| string
865865
localized?: never
866-
} & Omit<TabBase, 'name' | 'virtual'>
866+
} & Omit<TabBase, 'hooks' | 'name' | 'virtual'>
867867

868868
export type Tab = NamedTab | UnnamedTab
869869
export type TabsField = {

packages/payload/src/fields/hooks/afterChange/promise.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const promise = async ({
7373

7474
if (fieldAffectsData(field)) {
7575
// Execute hooks
76-
if (field.hooks?.afterChange) {
76+
if ('hooks' in field && field.hooks?.afterChange) {
7777
for (const hook of field.hooks.afterChange) {
7878
const hookedValue = await hook({
7979
blockData,
@@ -88,16 +88,16 @@ export const promise = async ({
8888
path: pathSegments,
8989
previousDoc,
9090
previousSiblingDoc,
91-
previousValue: previousDoc?.[field.name!],
91+
previousValue: previousDoc?.[field.name],
9292
req,
9393
schemaPath: schemaPathSegments,
9494
siblingData,
9595
siblingFields: siblingFields!,
96-
value: siblingDoc?.[field.name!],
96+
value: siblingDoc?.[field.name],
9797
})
9898

9999
if (hookedValue !== undefined) {
100-
siblingDoc[field.name!] = hookedValue
100+
siblingDoc[field.name] = hookedValue
101101
}
102102
}
103103
}

packages/payload/src/fields/hooks/afterRead/promise.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,15 @@ export const promise = async ({
238238

239239
if (fieldAffectsDataResult) {
240240
// Execute hooks
241-
if (triggerHooks && field.hooks?.afterRead) {
241+
if (triggerHooks && 'hooks' in field && field.hooks?.afterRead) {
242242
for (const hook of field.hooks.afterRead) {
243243
const shouldRunHookOnAllLocales =
244244
fieldShouldBeLocalized({ field, parentIsLocalized: parentIsLocalized! }) &&
245245
(locale === 'all' || !flattenLocales) &&
246-
typeof siblingDoc[field.name!] === 'object'
246+
typeof siblingDoc[field.name] === 'object'
247247

248248
if (shouldRunHookOnAllLocales) {
249-
const localesAndValues = Object.entries(siblingDoc[field.name!])
249+
const localesAndValues = Object.entries(siblingDoc[field.name])
250250
await Promise.all(
251251
localesAndValues.map(async ([localeKey, value]) => {
252252
const hookedValue = await hook({
@@ -274,7 +274,7 @@ export const promise = async ({
274274
})
275275

276276
if (hookedValue !== undefined) {
277-
siblingDoc[field.name!][localeKey] = hookedValue
277+
siblingDoc[field.name][localeKey] = hookedValue
278278
}
279279
}),
280280
)
@@ -300,11 +300,11 @@ export const promise = async ({
300300
showHiddenFields,
301301
siblingData: siblingDoc,
302302
siblingFields: siblingFields!,
303-
value: siblingDoc[field.name!],
303+
value: siblingDoc[field.name],
304304
})
305305

306306
if (hookedValue !== undefined) {
307-
siblingDoc[field.name!] = hookedValue
307+
siblingDoc[field.name] = hookedValue
308308
}
309309
}
310310
}

packages/payload/src/fields/hooks/beforeChange/promise.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export const promise = async ({
129129
}
130130

131131
// Execute hooks
132-
if (field.hooks?.beforeChange) {
132+
if ('hooks' in field && field.hooks?.beforeChange) {
133133
for (const hook of field.hooks.beforeChange) {
134134
const hookedValue = await hook({
135135
blockData,
@@ -143,17 +143,17 @@ export const promise = async ({
143143
originalDoc: doc,
144144
path: pathSegments,
145145
previousSiblingDoc: siblingDoc,
146-
previousValue: siblingDoc[field.name!],
146+
previousValue: siblingDoc[field.name],
147147
req,
148148
schemaPath: schemaPathSegments,
149149
siblingData,
150150
siblingDocWithLocales,
151151
siblingFields: siblingFields!,
152-
value: siblingData[field.name!],
152+
value: siblingData[field.name],
153153
})
154154

155155
if (hookedValue !== undefined) {
156-
siblingData[field.name!] = hookedValue
156+
siblingData[field.name] = hookedValue
157157
}
158158
}
159159
}

packages/payload/src/fields/hooks/beforeDuplicate/promise.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export const promise = async <T>({
6565

6666
// Run field beforeDuplicate hooks.
6767
// These hooks are responsible for resetting the `id` field values of array and block rows. See `baseIDField`.
68-
if (Array.isArray(field.hooks?.beforeDuplicate)) {
68+
if (Array.isArray('hooks' in field && field.hooks?.beforeDuplicate)) {
6969
if (fieldIsLocalized) {
7070
const localeData: JsonObject = {}
7171

@@ -90,8 +90,10 @@ export const promise = async <T>({
9090
}
9191

9292
let hookResult
93-
for (const hook of field.hooks.beforeDuplicate) {
94-
hookResult = await hook(beforeDuplicateArgs)
93+
if ('hooks' in field && field.hooks?.beforeDuplicate) {
94+
for (const hook of field.hooks.beforeDuplicate) {
95+
hookResult = await hook(beforeDuplicateArgs)
96+
}
9597
}
9698

9799
if (typeof hookResult !== 'undefined') {
@@ -121,8 +123,10 @@ export const promise = async <T>({
121123
}
122124

123125
let hookResult
124-
for (const hook of field.hooks.beforeDuplicate) {
125-
hookResult = await hook(beforeDuplicateArgs)
126+
if ('hooks' in field && field.hooks?.beforeDuplicate) {
127+
for (const hook of field.hooks.beforeDuplicate) {
128+
hookResult = await hook(beforeDuplicateArgs)
129+
}
126130
}
127131

128132
if (typeof hookResult !== 'undefined') {

packages/payload/src/fields/hooks/beforeValidate/promise.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export const promise = async <T>({
284284
}
285285

286286
// Execute hooks
287-
if (field.hooks?.beforeValidate) {
287+
if ('hooks' in field && field.hooks?.beforeValidate) {
288288
for (const hook of field.hooks.beforeValidate) {
289289
const hookedValue = await hook({
290290
blockData,
@@ -299,19 +299,19 @@ export const promise = async <T>({
299299
overrideAccess,
300300
path: pathSegments,
301301
previousSiblingDoc: siblingDoc,
302-
previousValue: siblingDoc[field.name!],
302+
previousValue: siblingDoc[field.name],
303303
req,
304304
schemaPath: schemaPathSegments,
305305
siblingData,
306306
siblingFields: siblingFields!,
307307
value:
308-
typeof siblingData[field.name!] === 'undefined'
308+
typeof siblingData[field.name] === 'undefined'
309309
? fallbackResult.value
310-
: siblingData[field.name!],
310+
: siblingData[field.name],
311311
})
312312

313313
if (hookedValue !== undefined) {
314-
siblingData[field.name!] = hookedValue
314+
siblingData[field.name] = hookedValue
315315
}
316316
}
317317
}

packages/payload/src/fields/setDefaultBeforeDuplicate.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const setDefaultBeforeDuplicate = (
1313
) => {
1414
if (
1515
(('required' in field && field.required) || field.unique) &&
16+
'hooks' in field &&
1617
(!field.hooks?.beforeDuplicate ||
1718
(Array.isArray(field.hooks.beforeDuplicate) && field.hooks.beforeDuplicate.length === 0))
1819
) {

packages/plugin-cloud-storage/src/fields/getFields.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ export const getFields = ({
119119
generateFileURL,
120120
size,
121121
}),
122-
...(existingSizeURLField?.hooks?.afterRead || []),
122+
...((typeof existingSizeURLField === 'object' &&
123+
'hooks' in existingSizeURLField &&
124+
existingSizeURLField?.hooks?.afterRead) ||
125+
[]),
123126
],
124127
},
125128
},

0 commit comments

Comments
 (0)