Skip to content

Commit ac1e3cf

Browse files
authored
feat(ui): form state queues (#11579)
Implements a form state task queue. This will prevent onChange handlers within the form component from processing unnecessarily often, sometimes long after the user has stopped making changes. This leads to a potentially huge number of network requests if those changes were made slower than the debounce rate. This is especially noticeable on slow networks. Does so through a new `useQueue` hook. This hook maintains a stack of events that need processing but only processes the final event to arrive. Every time a new event is pushed to the stack, the currently running process is aborted (if any), and that event becomes the next in the queue. This results in a shocking reduction in the time it takes between final change to form state and the final network response, from ~1.5 minutes to ~3 seconds (depending on the scenario, see below). This likely fixes a number of existing open issues. I will link those issues here once they are identified and verifiably fixed. Before: I'm typing slowly here to ensure my changes aren't debounce by the form. There are a total of 60 characters typed, triggering 58 network requests and taking around 1.5 minutes to complete after the final change was made. https://github.com/user-attachments/assets/49ba0790-a8f8-4390-8421-87453ff8b650 After: Here there are a total of 69 characters typed, triggering 11 network requests and taking only about 3 seconds to complete after the final change was made. https://github.com/user-attachments/assets/447f8303-0957-41bd-bb2d-9e1151ed9ec3
1 parent 397c1f1 commit ac1e3cf

File tree

24 files changed

+2641
-96
lines changed

24 files changed

+2641
-96
lines changed

.github/workflows/main.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ jobs:
308308
- fields__collections__Text
309309
- fields__collections__UI
310310
- fields__collections__Upload
311+
- form-state
311312
- live-preview
312313
- localization
313314
- locked-documents

packages/richtext-lexical/src/features/converters/jsx/converter/converters/upload.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export const UploadJSXConverter: JSXConverters<SerializedUploadNode> = {
1212
return null
1313
}
1414

15-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
15+
1616
const uploadDoc = uploadNode.value as FileData & TypeWithID
1717

1818
const url = uploadDoc.url

packages/ui/src/forms/Form/index.tsx

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
reduceFieldsToValues,
1111
wait,
1212
} from 'payload/shared'
13-
import React, { useCallback, useEffect, useReducer, useRef, useState } from 'react'
13+
import React, { useCallback, useEffect, useMemo, useReducer, useRef, useState } from 'react'
1414
import { toast } from 'sonner'
1515

1616
import type {
@@ -24,6 +24,7 @@ import type {
2424
import { FieldErrorsToast } from '../../elements/Toasts/fieldErrors.js'
2525
import { useDebouncedEffect } from '../../hooks/useDebouncedEffect.js'
2626
import { useEffectEvent } from '../../hooks/useEffectEvent.js'
27+
import { useQueues } from '../../hooks/useQueues.js'
2728
import { useThrottledEffect } from '../../hooks/useThrottledEffect.js'
2829
import { useAuth } from '../../providers/Auth/index.js'
2930
import { useConfig } from '../../providers/Config/index.js'
@@ -91,6 +92,7 @@ export const Form: React.FC<FormProps> = (props) => {
9192
const { i18n, t } = useTranslation()
9293
const { refreshCookie, user } = useAuth()
9394
const operation = useOperation()
95+
const { queueTask } = useQueues()
9496

9597
const { getFormState } = useServerFunctions()
9698
const { startRouteTransition } = useRouteTransition()
@@ -101,6 +103,7 @@ export const Form: React.FC<FormProps> = (props) => {
101103
const [disabled, setDisabled] = useState(disabledFromProps || false)
102104
const [isMounted, setIsMounted] = useState(false)
103105
const [modified, setModified] = useState(false)
106+
104107
/**
105108
* Tracks wether the form state passes validation.
106109
* For example the state could be submitted but invalid as field errors have been returned.
@@ -114,17 +117,16 @@ export const Form: React.FC<FormProps> = (props) => {
114117
const formRef = useRef<HTMLFormElement>(null)
115118
const contextRef = useRef({} as FormContextType)
116119
const abortResetFormRef = useRef<AbortController>(null)
120+
const isFirstRenderRef = useRef(true)
117121

118122
const fieldsReducer = useReducer(fieldReducer, {}, () => initialState)
119123

120-
/**
121-
* `fields` is the current, up-to-date state/data of all fields in the form. It can be modified by using dispatchFields,
122-
* which calls the fieldReducer, which then updates the state.
123-
*/
124124
const [fields, dispatchFields] = fieldsReducer
125125

126126
contextRef.current.fields = fields
127127

128+
const prevFields = useRef(fields)
129+
128130
const validateForm = useCallback(async () => {
129131
const validatedFieldState = {}
130132
let isValid = true
@@ -718,11 +720,15 @@ export const Form: React.FC<FormProps> = (props) => {
718720

719721
const classes = [className, baseClass].filter(Boolean).join(' ')
720722

721-
const executeOnChange = useEffectEvent(async (submitted: boolean) => {
723+
const executeOnChange = useEffectEvent(async (submitted: boolean, signal: AbortSignal) => {
722724
if (Array.isArray(onChange)) {
723725
let revalidatedFormState: FormState = contextRef.current.fields
724726

725727
for (const onChangeFn of onChange) {
728+
if (signal.aborted) {
729+
return
730+
}
731+
726732
// Edit view default onChange is in packages/ui/src/views/Edit/index.tsx. This onChange usually sends a form state request
727733
revalidatedFormState = await onChangeFn({
728734
formState: deepCopyObjectSimpleWithoutReactComponents(contextRef.current.fields),
@@ -739,7 +745,9 @@ export const Form: React.FC<FormProps> = (props) => {
739745
incomingState: revalidatedFormState,
740746
})
741747

742-
if (changed) {
748+
if (changed && !signal.aborted) {
749+
prevFields.current = newState
750+
743751
dispatchFields({
744752
type: 'REPLACE_STATE',
745753
optimize: false,
@@ -749,29 +757,16 @@ export const Form: React.FC<FormProps> = (props) => {
749757
}
750758
})
751759

752-
const prevFields = useRef(contextRef.current.fields)
753-
const isFirstRenderRef = useRef(true)
754-
755760
useDebouncedEffect(
756761
() => {
757-
if (isFirstRenderRef.current || !dequal(contextRef.current.fields, prevFields.current)) {
758-
if (modified) {
759-
void executeOnChange(submitted)
760-
}
762+
if ((isFirstRenderRef.current || !dequal(fields, prevFields.current)) && modified) {
763+
queueTask(async (signal) => executeOnChange(submitted, signal))
761764
}
762765

766+
prevFields.current = fields
763767
isFirstRenderRef.current = false
764-
prevFields.current = contextRef.current.fields
765768
},
766-
/*
767-
Make sure we trigger this whenever modified changes (not just when `fields` changes),
768-
otherwise we will miss merging server form state for the first form update/onChange.
769-
770-
Here's why:
771-
`fields` updates before `modified`, because setModified is in a setTimeout.
772-
So on the first change, modified is false, so we don't trigger the effect even though we should.
773-
**/
774-
[modified, submitted, contextRef.current.fields],
769+
[modified, submitted, fields, queueTask],
775770
250,
776771
)
777772

packages/ui/src/hooks/useQueues.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { useRef } from 'react'
2+
3+
export function useQueues(): {
4+
queueTask: (fn: (signal: AbortSignal) => Promise<void>) => void
5+
} {
6+
const runningTaskRef = useRef<null | Promise<void>>(null)
7+
const queuedTask = useRef<((signal: AbortSignal) => Promise<void>) | null>(null)
8+
const abortControllerRef = useRef<AbortController | null>(null)
9+
10+
const queueTask = (fn: (signal: AbortSignal) => Promise<void>) => {
11+
// Overwrite the queued task every time a new one arrives
12+
queuedTask.current = fn
13+
14+
// If a task is already running, abort it and return
15+
if (runningTaskRef.current !== null) {
16+
if (abortControllerRef.current) {
17+
abortControllerRef.current.abort()
18+
}
19+
20+
return
21+
}
22+
23+
const executeTask = async () => {
24+
while (queuedTask.current) {
25+
const taskToRun = queuedTask.current
26+
queuedTask.current = null // Reset latest task before running
27+
28+
const controller = new AbortController()
29+
abortControllerRef.current = controller
30+
31+
try {
32+
runningTaskRef.current = taskToRun(controller.signal)
33+
await runningTaskRef.current // Wait for the task to complete
34+
} catch (err) {
35+
if (err.name !== 'AbortError') {
36+
console.error('Error in queued function:', err) // eslint-disable-line no-console
37+
}
38+
} finally {
39+
runningTaskRef.current = null
40+
}
41+
}
42+
}
43+
44+
void executeTask()
45+
}
46+
47+
return { queueTask }
48+
}

test/admin/collections/Posts.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -263,25 +263,6 @@ export const Posts: CollectionConfig = {
263263
position: 'sidebar',
264264
},
265265
},
266-
{
267-
name: 'validateUsingEvent',
268-
type: 'text',
269-
admin: {
270-
description:
271-
'This field should only validate on submit. Try typing "Not allowed" and submitting the form.',
272-
},
273-
validate: (value, { event }) => {
274-
if (event === 'onChange') {
275-
return true
276-
}
277-
278-
if (value === 'Not allowed') {
279-
return 'This field has been validated only on submit'
280-
}
281-
282-
return true
283-
},
284-
},
285266
],
286267
labels: {
287268
plural: slugPluralLabel,

test/admin/e2e/document-view/e2e.spec.ts

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -174,36 +174,12 @@ describe('Document View', () => {
174174
})
175175
})
176176

177-
describe('form state', () => {
178-
test('collection — should re-enable fields after save', async () => {
179-
await page.goto(postsUrl.create)
180-
await page.locator('#field-title').fill(title)
181-
await saveDocAndAssert(page)
182-
await expect(page.locator('#field-title')).toBeEnabled()
183-
})
184-
185-
test('global — should re-enable fields after save', async () => {
186-
await page.goto(globalURL.global(globalSlug))
187-
await page.locator('#field-title').fill(title)
188-
await saveDocAndAssert(page)
189-
await expect(page.locator('#field-title')).toBeEnabled()
190-
})
191-
192-
test('should thread proper event argument to validation functions', async () => {
193-
await page.goto(postsUrl.create)
194-
await page.locator('#field-title').fill(title)
195-
await page.locator('#field-validateUsingEvent').fill('Not allowed')
196-
await saveDocAndAssert(page, '#action-save', 'error')
197-
})
198-
})
199-
200177
describe('document titles', () => {
201178
test('collection — should render fallback titles when creating new', async () => {
202179
await page.goto(postsUrl.create)
203180
await checkPageTitle(page, '[Untitled]')
204181
await checkBreadcrumb(page, 'Create New')
205182
await saveDocAndAssert(page)
206-
expect(true).toBe(true)
207183
})
208184

209185
test('collection — should render `useAsTitle` field', async () => {
@@ -213,7 +189,6 @@ describe('Document View', () => {
213189
await wait(500)
214190
await checkPageTitle(page, title)
215191
await checkBreadcrumb(page, title)
216-
expect(true).toBe(true)
217192
})
218193

219194
test('collection — should render `id` as `useAsTitle` fallback', async () => {

test/eslint.config.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,12 @@ export const testEslintConfig = [
6565
'playwright/expect-expect': [
6666
'error',
6767
{
68-
assertFunctionNames: ['assertToastErrors', 'saveDocAndAssert', 'runFilterOptionsTest'],
68+
assertFunctionNames: [
69+
'assertToastErrors',
70+
'saveDocAndAssert',
71+
'runFilterOptionsTest',
72+
'assertNetworkRequests',
73+
],
6974
},
7075
],
7176
},

test/fields-relationship/e2e.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type {
2323

2424
import { ensureCompilationIsDone, initPageConsoleErrorCatch, saveDocAndAssert } from '../helpers.js'
2525
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
26-
import { trackNetworkRequests } from '../helpers/e2e/trackNetworkRequests.js'
26+
import { assertNetworkRequests } from '../helpers/e2e/assertNetworkRequests.js'
2727
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
2828
import { POLL_TOPASS_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
2929
import {
@@ -176,7 +176,7 @@ describe('Relationship Field', () => {
176176
await expect(options).toHaveCount(2) // two docs
177177
await options.nth(0).click()
178178
await expect(field).toContainText(relationOneDoc.id)
179-
await trackNetworkRequests(page, `/api/${relationOneSlug}`, async () => {
179+
await assertNetworkRequests(page, `/api/${relationOneSlug}`, async () => {
180180
await saveDocAndAssert(page)
181181
await wait(200)
182182
})

test/fields/collections/Array/e2e.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe('Array', () => {
114114
await expect(page.locator('#field-customArrayField__0__text')).toBeVisible()
115115
})
116116

117-
// eslint-disable-next-line playwright/expect-expect
117+
118118
test('should bypass min rows validation when no rows present and field is not required', async () => {
119119
await page.goto(url.create)
120120
await saveDocAndAssert(page)

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
} from '../../../../../helpers.js'
2626
import { AdminUrlUtil } from '../../../../../helpers/adminUrlUtil.js'
2727
import { assertToastErrors } from '../../../../../helpers/assertToastErrors.js'
28-
import { trackNetworkRequests } from '../../../../../helpers/e2e/trackNetworkRequests.js'
28+
import { assertNetworkRequests } from '../../../../../helpers/e2e/assertNetworkRequests.js'
2929
import { initPayloadE2ENoConfig } from '../../../../../helpers/initPayloadE2ENoConfig.js'
3030
import { reInitializeDB } from '../../../../../helpers/reInitializeDB.js'
3131
import { RESTClient } from '../../../../../helpers/rest.js'
@@ -400,11 +400,12 @@ describe('lexicalBlocks', () => {
400400
await dependsOnBlockData.locator('.rs__control').click()
401401

402402
// Fill and wait for form state to come back
403-
await trackNetworkRequests(page, '/admin/collections/lexical-fields', async () => {
403+
await assertNetworkRequests(page, '/admin/collections/lexical-fields', async () => {
404404
await topLevelDocTextField.fill('invalid')
405405
})
406+
406407
// Ensure block form state is updated and comes back (=> filter options are updated)
407-
await trackNetworkRequests(
408+
await assertNetworkRequests(
408409
page,
409410
'/admin/collections/lexical-fields',
410411
async () => {
@@ -442,7 +443,7 @@ describe('lexicalBlocks', () => {
442443
topLevelDocTextField,
443444
} = await setupFilterOptionsTests()
444445

445-
await trackNetworkRequests(
446+
await assertNetworkRequests(
446447
page,
447448
'/admin/collections/lexical-fields',
448449
async () => {
@@ -478,7 +479,7 @@ describe('lexicalBlocks', () => {
478479
topLevelDocTextField,
479480
} = await setupFilterOptionsTests()
480481

481-
await trackNetworkRequests(
482+
await assertNetworkRequests(
482483
page,
483484
'/admin/collections/lexical-fields',
484485
async () => {
@@ -571,19 +572,21 @@ describe('lexicalBlocks', () => {
571572
await topLevelDocTextField.fill('invalid')
572573

573574
await saveDocAndAssert(page, '#action-save', 'error')
575+
574576
await assertToastErrors({
575577
page,
576578
errors: ['Lexical With Blocks', 'Lexical With Blocks → Group → Text Depends On Doc Data'],
577579
})
580+
578581
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
579582

580-
await trackNetworkRequests(
583+
await assertNetworkRequests(
581584
page,
582585
'/admin/collections/lexical-fields',
583586
async () => {
584587
await topLevelDocTextField.fill('Rich Text') // Default value
585588
},
586-
{ allowedNumberOfRequests: 2 },
589+
{ allowedNumberOfRequests: 1 },
587590
)
588591

589592
await saveDocAndAssert(page)
@@ -604,13 +607,13 @@ describe('lexicalBlocks', () => {
604607
})
605608
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
606609

607-
await trackNetworkRequests(
610+
await assertNetworkRequests(
608611
page,
609612
'/admin/collections/lexical-fields',
610613
async () => {
611614
await blockGroupTextField.fill('')
612615
},
613-
{ allowedNumberOfRequests: 3 },
616+
{ allowedNumberOfRequests: 2 },
614617
)
615618

616619
await saveDocAndAssert(page)
@@ -628,13 +631,13 @@ describe('lexicalBlocks', () => {
628631
})
629632
await expect(page.locator('.payload-toast-container .payload-toast-item')).toBeHidden()
630633

631-
await trackNetworkRequests(
634+
await assertNetworkRequests(
632635
page,
633636
'/admin/collections/lexical-fields',
634637
async () => {
635638
await blockTextField.fill('')
636639
},
637-
{ allowedNumberOfRequests: 3 },
640+
{ allowedNumberOfRequests: 2 },
638641
)
639642

640643
await saveDocAndAssert(page)

0 commit comments

Comments
 (0)