refactor: simplify running field hooks (#11372)

Previously, we were quite frequently using `.reduce()` to sequentially run field hooks. This PR replaces them with simple `for` loops, which is less overhead, less code, less confusing and simpler to understand.

Additionally, it refactors `mergeLocaleActions` which previously was unnecessarily complex. They no longer entail async code, thus we no longer have to juggle with promises
This commit is contained in:
Alessio Gravili
2025-02-24 11:37:33 -07:00
committed by GitHub
parent 2477fc6c75
commit dc9e8fa655
10 changed files with 89 additions and 125 deletions

View File

@@ -88,7 +88,7 @@ export type BeforeChangeRichTextHookArgs<
errors?: ValidationFieldError[] errors?: ValidationFieldError[]
/** Only available in `beforeChange` field hooks */ /** Only available in `beforeChange` field hooks */
mergeLocaleActions?: (() => Promise<void>)[] mergeLocaleActions?: (() => Promise<void> | void)[]
/** A string relating to which operation the field type is currently executing within. */ /** A string relating to which operation the field type is currently executing within. */
operation?: 'create' | 'delete' | 'read' | 'update' operation?: 'create' | 'delete' | 'read' | 'update'
/** The sibling data of the document before changes being applied. */ /** The sibling data of the document before changes being applied. */

View File

@@ -75,10 +75,8 @@ export const promise = async ({
if (fieldAffectsData(field)) { if (fieldAffectsData(field)) {
// Execute hooks // Execute hooks
if (field.hooks?.afterChange) { if (field.hooks?.afterChange) {
await field.hooks.afterChange.reduce(async (priorHook, currentHook) => { for (const hook of field.hooks.afterChange) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
blockData, blockData,
collection, collection,
context, context,
@@ -102,7 +100,7 @@ export const promise = async ({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingDoc[field.name] = hookedValue siblingDoc[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
} }
@@ -242,17 +240,15 @@ export const promise = async ({
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 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') { if (typeof field.editor === 'function') {
throw new Error('Attempted to access unsanitized rich text editor.') throw new Error('Attempted to access unsanitized rich text editor.')
} }
const editor: RichTextAdapter = field?.editor const editor: RichTextAdapter = field.editor
if (editor?.hooks?.afterChange?.length) { if (editor?.hooks?.afterChange?.length) {
await editor.hooks.afterChange.reduce(async (priorHook, currentHook) => { for (const hook of editor.hooks.afterChange) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
collection, collection,
context, context,
data, data,
@@ -275,7 +271,7 @@ export const promise = async ({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingDoc[field.name] = hookedValue siblingDoc[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
break break
} }

View File

@@ -237,18 +237,17 @@ export const promise = async ({
if (fieldAffectsData(field)) { if (fieldAffectsData(field)) {
// Execute hooks // Execute hooks
if (triggerHooks && field.hooks?.afterRead) { if (triggerHooks && field.hooks?.afterRead) {
await field.hooks.afterRead.reduce(async (priorHook, currentHook) => { for (const hook of field.hooks.afterRead) {
await priorHook
const shouldRunHookOnAllLocales = const shouldRunHookOnAllLocales =
fieldShouldBeLocalized({ field, parentIsLocalized }) && fieldShouldBeLocalized({ field, parentIsLocalized }) &&
(locale === 'all' || !flattenLocales) && (locale === 'all' || !flattenLocales) &&
typeof siblingDoc[field.name] === 'object' typeof siblingDoc[field.name] === 'object'
if (shouldRunHookOnAllLocales) { if (shouldRunHookOnAllLocales) {
const hookPromises = Object.entries(siblingDoc[field.name]).map(([locale, value]) => const localesAndValues = Object.entries(siblingDoc[field.name])
(async () => { await Promise.all(
const hookedValue = await currentHook({ localesAndValues.map(async ([localeKey, value]) => {
const hookedValue = await hook({
blockData, blockData,
collection, collection,
context, context,
@@ -273,14 +272,12 @@ export const promise = async ({
}) })
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingDoc[field.name][locale] = hookedValue siblingDoc[field.name][localeKey] = hookedValue
} }
})(), }),
) )
await Promise.all(hookPromises)
} else { } else {
const hookedValue = await currentHook({ const hookedValue = await hook({
blockData, blockData,
collection, collection,
context, context,
@@ -308,7 +305,7 @@ export const promise = async ({
siblingDoc[field.name] = hookedValue siblingDoc[field.name] = hookedValue
} }
} }
}, Promise.resolve()) }
} }
// Execute access control // Execute access control
@@ -677,18 +674,18 @@ export const promise = async ({
const editor: RichTextAdapter = field?.editor const editor: RichTextAdapter = field?.editor
if (editor?.hooks?.afterRead?.length) { if (editor?.hooks?.afterRead?.length) {
await editor.hooks.afterRead.reduce(async (priorHook, currentHook) => { for (const hook of editor.hooks.afterRead) {
await priorHook
const shouldRunHookOnAllLocales = const shouldRunHookOnAllLocales =
fieldShouldBeLocalized({ field, parentIsLocalized }) && fieldShouldBeLocalized({ field, parentIsLocalized }) &&
(locale === 'all' || !flattenLocales) && (locale === 'all' || !flattenLocales) &&
typeof siblingDoc[field.name] === 'object' typeof siblingDoc[field.name] === 'object'
if (shouldRunHookOnAllLocales) { if (shouldRunHookOnAllLocales) {
const hookPromises = Object.entries(siblingDoc[field.name]).map(([locale, value]) => const localesAndValues = Object.entries(siblingDoc[field.name])
(async () => {
const hookedValue = await currentHook({ await Promise.all(
localesAndValues.map(async ([locale, value]) => {
const hookedValue = await hook({
collection, collection,
context, context,
currentDepth, currentDepth,
@@ -722,12 +719,10 @@ export const promise = async ({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingDoc[field.name][locale] = hookedValue siblingDoc[field.name][locale] = hookedValue
} }
})(), }),
) )
await Promise.all(hookPromises)
} else { } else {
const hookedValue = await currentHook({ const hookedValue = await hook({
collection, collection,
context, context,
currentDepth, currentDepth,
@@ -762,7 +757,7 @@ export const promise = async ({
siblingDoc[field.name] = hookedValue siblingDoc[field.name] = hookedValue
} }
} }
}, Promise.resolve()) }
} }
break break
} }

View File

@@ -81,10 +81,9 @@ export const beforeChange = async <T extends JsonObject>({
) )
} }
await mergeLocaleActions.reduce(async (priorAction, action) => { for (const action of mergeLocaleActions) {
await priorAction
await action() await action()
}, Promise.resolve()) }
return data return data
} }

View File

@@ -31,7 +31,7 @@ type Args = {
fieldIndex: number fieldIndex: number
global: null | SanitizedGlobalConfig global: null | SanitizedGlobalConfig
id?: number | string id?: number | string
mergeLocaleActions: (() => Promise<void>)[] mergeLocaleActions: (() => Promise<void> | void)[]
operation: Operation operation: Operation
parentIndexPath: string parentIndexPath: string
parentIsLocalized: boolean parentIsLocalized: boolean
@@ -108,10 +108,8 @@ export const promise = async ({
// Execute hooks // Execute hooks
if (field.hooks?.beforeChange) { if (field.hooks?.beforeChange) {
await field.hooks.beforeChange.reduce(async (priorHook, currentHook) => { for (const hook of field.hooks.beforeChange) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
blockData, blockData,
collection, collection,
context, context,
@@ -135,7 +133,7 @@ export const promise = async ({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingData[field.name] = hookedValue siblingData[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
// Validate // Validate
@@ -192,28 +190,20 @@ export const promise = async ({
// Push merge locale action if applicable // Push merge locale action if applicable
if (localization && fieldShouldBeLocalized({ field, parentIsLocalized })) { if (localization && fieldShouldBeLocalized({ field, parentIsLocalized })) {
mergeLocaleActions.push(async () => { mergeLocaleActions.push(() => {
const localeData = await localization.localeCodes.reduce( const localeData = {}
async (localizedValuesPromise: Promise<JsonObject>, locale) => {
const localizedValues = await localizedValuesPromise
const fieldValue =
locale === req.locale
? siblingData[field.name]
: siblingDocWithLocales?.[field.name]?.[locale]
// const result = await localizedValues for (const locale of localization.localeCodes) {
// update locale value if it's not undefined const fieldValue =
if (typeof fieldValue !== 'undefined') { locale === req.locale
return { ? siblingData[field.name]
...localizedValues, : siblingDocWithLocales?.[field.name]?.[locale]
[locale]: fieldValue,
}
}
return localizedValuesPromise // update locale value if it's not undefined
}, if (typeof fieldValue !== 'undefined') {
Promise.resolve({}), localeData[locale] = fieldValue
) }
}
// If there are locales with data, set the data // If there are locales with data, set the data
if (Object.keys(localeData).length > 0) { if (Object.keys(localeData).length > 0) {
@@ -423,10 +413,8 @@ export const promise = async ({
const editor: RichTextAdapter = field?.editor const editor: RichTextAdapter = field?.editor
if (editor?.hooks?.beforeChange?.length) { if (editor?.hooks?.beforeChange?.length) {
await editor.hooks.beforeChange.reduce(async (priorHook, currentHook) => { for (const hook of editor.hooks.beforeChange) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
collection, collection,
context, context,
data, data,
@@ -453,7 +441,7 @@ export const promise = async ({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingData[field.name] = hookedValue siblingData[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
break break

View File

@@ -28,7 +28,7 @@ type Args = {
fields: (Field | TabAsField)[] fields: (Field | TabAsField)[]
global: null | SanitizedGlobalConfig global: null | SanitizedGlobalConfig
id?: number | string id?: number | string
mergeLocaleActions: (() => Promise<void>)[] mergeLocaleActions: (() => Promise<void> | void)[]
operation: Operation operation: Operation
parentIndexPath: string parentIndexPath: string
/** /**

View File

@@ -6,7 +6,6 @@ import type { Block, Field, FieldHookArgs, TabAsField } from '../../config/types
import { fieldAffectsData, fieldShouldBeLocalized } from '../../config/types.js' import { fieldAffectsData, fieldShouldBeLocalized } from '../../config/types.js'
import { getFieldPathsModified as getFieldPaths } from '../../getFieldPaths.js' import { getFieldPathsModified as getFieldPaths } from '../../getFieldPaths.js'
import { runBeforeDuplicateHooks } from './runHook.js'
import { traverseFields } from './traverseFields.js' import { traverseFields } from './traverseFields.js'
type Args<T> = { type Args<T> = {
@@ -68,42 +67,37 @@ export const promise = async <T>({
// Run field beforeDuplicate hooks // Run field beforeDuplicate hooks
if (Array.isArray(field.hooks?.beforeDuplicate)) { if (Array.isArray(field.hooks?.beforeDuplicate)) {
if (fieldIsLocalized) { if (fieldIsLocalized) {
const localeData = await localization.localeCodes.reduce( const localeData: JsonObject = {}
async (localizedValuesPromise: Promise<JsonObject>, locale) => {
const localizedValues = await localizedValuesPromise
const beforeDuplicateArgs: FieldHookArgs = { for (const locale of localization.localeCodes) {
blockData, const beforeDuplicateArgs: FieldHookArgs = {
collection, blockData,
context, collection,
data: doc, context,
field, data: doc,
global: undefined, field,
indexPath: indexPathSegments, global: undefined,
path: pathSegments, indexPath: indexPathSegments,
previousSiblingDoc: siblingDoc, path: pathSegments,
previousValue: siblingDoc[field.name]?.[locale], previousSiblingDoc: siblingDoc,
req, previousValue: siblingDoc[field.name]?.[locale],
schemaPath: schemaPathSegments, req,
siblingData: siblingDoc, schemaPath: schemaPathSegments,
siblingDocWithLocales: siblingDoc, siblingData: siblingDoc,
siblingFields, siblingDocWithLocales: siblingDoc,
value: siblingDoc[field.name]?.[locale], siblingFields,
} value: siblingDoc[field.name]?.[locale],
}
const hookResult = await runBeforeDuplicateHooks(beforeDuplicateArgs) let hookResult
for (const hook of field.hooks.beforeDuplicate) {
hookResult = await hook(beforeDuplicateArgs)
}
if (typeof hookResult !== 'undefined') { if (typeof hookResult !== 'undefined') {
return { localeData[locale] = hookResult
...localizedValues, }
[locale]: hookResult, }
}
}
return localizedValuesPromise
},
Promise.resolve({}),
)
siblingDoc[field.name] = localeData siblingDoc[field.name] = localeData
} else { } else {
@@ -126,7 +120,11 @@ export const promise = async <T>({
value: siblingDoc[field.name], value: siblingDoc[field.name],
} }
const hookResult = await runBeforeDuplicateHooks(beforeDuplicateArgs) let hookResult
for (const hook of field.hooks.beforeDuplicate) {
hookResult = await hook(beforeDuplicateArgs)
}
if (typeof hookResult !== 'undefined') { if (typeof hookResult !== 'undefined') {
siblingDoc[field.name] = hookResult siblingDoc[field.name] = hookResult
} }

View File

@@ -1,8 +0,0 @@
// @ts-strict-ignore
import type { FieldHookArgs } from '../../config/types.js'
export const runBeforeDuplicateHooks = async (args: FieldHookArgs) =>
await args.field.hooks.beforeDuplicate.reduce(async (priorHook, currentHook) => {
await priorHook
return await currentHook(args)
}, Promise.resolve())

View File

@@ -276,10 +276,8 @@ export const promise = async <T>({
// Execute hooks // Execute hooks
if (field.hooks?.beforeValidate) { if (field.hooks?.beforeValidate) {
await field.hooks.beforeValidate.reduce(async (priorHook, currentHook) => { for (const hook of field.hooks.beforeValidate) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
blockData, blockData,
collection, collection,
context, context,
@@ -303,7 +301,7 @@ export const promise = async <T>({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingData[field.name] = hookedValue siblingData[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
// Execute access control // Execute access control
@@ -493,10 +491,8 @@ export const promise = async <T>({
const editor: RichTextAdapter = field?.editor const editor: RichTextAdapter = field?.editor
if (editor?.hooks?.beforeValidate?.length) { if (editor?.hooks?.beforeValidate?.length) {
await editor.hooks.beforeValidate.reduce(async (priorHook, currentHook) => { for (const hook of editor.hooks.beforeValidate) {
await priorHook const hookedValue = await hook({
const hookedValue = await currentHook({
collection, collection,
context, context,
data, data,
@@ -519,7 +515,7 @@ export const promise = async <T>({
if (hookedValue !== undefined) { if (hookedValue !== undefined) {
siblingData[field.name] = hookedValue siblingData[field.name] = hookedValue
} }
}, Promise.resolve()) }
} }
break break
} }

View File

@@ -176,7 +176,7 @@ export type BeforeChangeNodeHookArgs<T extends SerializedLexicalNode> = {
* Only available in `beforeChange` hooks. * Only available in `beforeChange` hooks.
*/ */
errors: ValidationFieldError[] errors: ValidationFieldError[]
mergeLocaleActions: (() => Promise<void>)[] mergeLocaleActions: (() => Promise<void> | void)[]
/** 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. */ /** 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' operation: 'create' | 'delete' | 'read' | 'update'
/** The value of the node before any changes. Not available in afterRead hooks */ /** The value of the node before any changes. Not available in afterRead hooks */