Skip to content

Commit 9f2bca1

Browse files
authored
fix(richtext-lexical): afterRead hooks were not awaited (#10747)
This was a tricky one. Fixes #10700 May potentially fix 9163 This could have also been causing glitchyness related to things like lexical upload / relationship / link node population. ## Issue and solution explained The lexical field afterRead hook is responsible for executing afterRead hooks (this includes relationship population) for all sub-nodes, e.g. upload or block nodes. Any field and population promises that were created in the process of traversing the lexical editor state were added to the parent `fieldPromises` and `populationPromises` array. Now this lexical `afterRead` hook, which is responsible for creating and adding all field and population promises of its sub-nodes to the parent `fieldPromises` and `populationPromises` arrays, was itself part of the **same** `fieldPromises` array. The execution of this lexical `afterRead` hook itself is happening while the `fieldPromises` array is being awaited. This means that new field and population promises were being added to this same array DURING the process of awaiting all promises of this array. As a result, any promises dynamically added while awaiting the initial set of fieldPromises were not included in the initial `Promise.all()` awaiting process, leading to unhandled promises. This PR resolves the issue by ensuring that promises are repeatedly awaited until no new promises remain in the arrays. By continuously awaiting the `fieldPromises` and `populationPromises` until all dynamically added promises are fully resolved, this PR ensures that any promises added during the processing of a parent promise are also properly awaited. This guarantees that no promises are skipped, preserving the intended behavior of the afterRead hook
1 parent ec1a441 commit 9f2bca1

File tree

4 files changed

+104
-4
lines changed

4 files changed

+104
-4
lines changed

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { SanitizedGlobalConfig } from '../../../globals/config/types.js'
33
import type { RequestContext } from '../../../index.js'
44
import type { JsonObject, PayloadRequest, PopulateType, SelectType } from '../../../types/index.js'
55

6-
import { deepCopyObjectSimple } from '../../../utilities/deepCopyObject.js'
76
import { getSelectMode } from '../../../utilities/getSelectMode.js'
87
import { traverseFields } from './traverseFields.js'
98

@@ -95,8 +94,26 @@ export async function afterRead<T extends JsonObject>(args: Args<T>): Promise<T>
9594
siblingDoc: incomingDoc,
9695
})
9796

98-
await Promise.all(fieldPromises)
99-
await Promise.all(populationPromises)
97+
/**
98+
* Await all field and population promises in parallel.
99+
* A field promise is able to add more field promises to the fieldPromises array, which will not be
100+
* awaited in the first run.
101+
* This is why we need to loop again to process the new field promises, until there are no more field promises left.
102+
*/
103+
let iterations = 0
104+
while (fieldPromises.length > 0 || populationPromises.length > 0) {
105+
const currentFieldPromises = fieldPromises.splice(0, fieldPromises.length)
106+
const currentPopulationPromises = populationPromises.splice(0, populationPromises.length)
100107

108+
await Promise.all(currentFieldPromises)
109+
await Promise.all(currentPopulationPromises)
110+
111+
iterations++
112+
if (iterations >= 100) {
113+
throw new Error(
114+
'Infinite afterRead promise loop detected. A hook is likely adding field promises in an infinitely recursive way.',
115+
)
116+
}
117+
}
101118
return incomingDoc
102119
}

test/fields/collections/Lexical/blocks.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,44 @@ import { BlocksFeature, FixedToolbarFeature, lexicalEditor } from '@payloadcms/r
44

55
import { textFieldsSlug } from '../Text/shared.js'
66

7+
async function asyncFunction(param: string) {
8+
return new Promise((resolve) => {
9+
setTimeout(() => {
10+
resolve(param?.toUpperCase())
11+
}, 1000)
12+
})
13+
}
14+
export const AsyncHooksBlock: Block = {
15+
slug: 'asyncHooksBlock',
16+
fields: [
17+
{
18+
name: 'test1',
19+
label: 'Text',
20+
type: 'text',
21+
hooks: {
22+
afterRead: [
23+
({ value }) => {
24+
return value?.toUpperCase()
25+
},
26+
],
27+
},
28+
},
29+
{
30+
name: 'test2',
31+
label: 'Text',
32+
type: 'text',
33+
hooks: {
34+
afterRead: [
35+
async ({ value }) => {
36+
const valuenew = await asyncFunction(value)
37+
return valuenew
38+
},
39+
],
40+
},
41+
},
42+
],
43+
}
44+
745
export const BlockColumns = ({ name }: { name: string }): ArrayField => ({
846
type: 'array',
947
name,

test/fields/collections/Lexical/e2e/blocks/e2e.spec.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,49 @@ describe('lexicalBlocks', () => {
312312
})
313313
})
314314

315+
test('ensure async hooks are awaited properly', async () => {
316+
const { richTextField } = await navigateToLexicalFields()
317+
318+
const lastParagraph = richTextField.locator('p').last()
319+
await lastParagraph.scrollIntoViewIfNeeded()
320+
await expect(lastParagraph).toBeVisible()
321+
322+
await lastParagraph.click()
323+
324+
await page.keyboard.press('Enter')
325+
await page.keyboard.press('/')
326+
await page.keyboard.type('Async')
327+
328+
// CreateBlock
329+
const slashMenuPopover = page.locator('#slash-menu .slash-menu-popup')
330+
await expect(slashMenuPopover).toBeVisible()
331+
332+
const asyncHooksBlockSelectButton = slashMenuPopover.locator('button').first()
333+
await expect(asyncHooksBlockSelectButton).toBeVisible()
334+
await expect(asyncHooksBlockSelectButton).toContainText('Async Hooks Block')
335+
await asyncHooksBlockSelectButton.click()
336+
await expect(slashMenuPopover).toBeHidden()
337+
338+
const newBlock = richTextField
339+
.locator('.lexical-block:not(.lexical-block .lexical-block)')
340+
.nth(8) // The :not(.lexical-block .lexical-block) makes sure this does not select sub-blocks
341+
await newBlock.scrollIntoViewIfNeeded()
342+
await newBlock.locator('#field-test1').fill('text1')
343+
await newBlock.locator('#field-test2').fill('text2')
344+
345+
await wait(300)
346+
347+
// save document and assert
348+
await saveDocAndAssert(page)
349+
await wait(300)
350+
351+
// Reload page
352+
await page.reload()
353+
354+
await expect(newBlock.locator('#field-test1')).toHaveValue('TEXT1')
355+
await expect(newBlock.locator('#field-test2')).toHaveValue('TEXT2')
356+
})
357+
315358
describe('nested lexical editor in block', () => {
316359
test('should type and save typed text', async () => {
317360
const { richTextField } = await navigateToLexicalFields()

test/fields/collections/Lexical/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { $convertToMarkdownString } from '@payloadcms/richtext-lexical/lexical/m
2020

2121
import { lexicalFieldsSlug } from '../../slugs.js'
2222
import {
23+
AsyncHooksBlock,
2324
CodeBlock,
2425
ConditionalLayoutBlock,
2526
RadioButtonsBlock,
@@ -73,6 +74,7 @@ const editorConfig: ServerEditorConfig = {
7374
ModifyInlineBlockFeature(),
7475
BlocksFeature({
7576
blocks: [
77+
AsyncHooksBlock,
7678
RichTextBlock,
7779
TextBlock,
7880
UploadAndRichTextBlock,
@@ -365,7 +367,7 @@ export const LexicalFields: CollectionConfig = {
365367
}
366368

367369
// Export to markdown
368-
let markdown: string
370+
let markdown: string = ''
369371
headlessEditor.getEditorState().read(() => {
370372
markdown = $convertToMarkdownString(
371373
yourSanitizedEditorConfig?.features?.markdownTransformers,

0 commit comments

Comments
 (0)