Skip to content

Commit 14f042f

Browse files
authored
fix(ui): autosave not queued while background process is in flight (#14805)
When editing fields while an autosave is in flight, subsequent autosave events are not queued as expected. This results in inconsistency between what appears on the screen and what is persisted to the database, due to race conditions in background processing. For example: https://github.com/user-attachments/assets/010f846c-ed07-4594-883e-3c60114e9f9e This is especially noticeable in virtual fields, `useAsTitle`, and Live Preview, where the displayed values are a direct representation of the database, not local to form state. The problem is related to how we manage `modified` form context. After successful submissions, we set this flag to false. This means if the user had made underlying changes to the form while a previous submission was processing, e.g. an autosave, the modified state gets wrongfully cleared and the next autosave bails out. Potentially related: #12691
1 parent 7520140 commit 14f042f

File tree

3 files changed

+121
-33
lines changed

3 files changed

+121
-33
lines changed

packages/ui/src/elements/Autosave/index.tsx

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
7575

7676
const debouncedFormState = useDebounce(formState, interval)
7777

78-
const formStateRef = useRef(formState)
79-
const modifiedRef = useRef(modified)
80-
const localeRef = useRef(locale)
81-
82-
// Store fields in ref so the autosave func
83-
// can always retrieve the most to date copies
84-
// after the timeout has executed
85-
formStateRef.current = formState
86-
87-
// Store modified in ref so the autosave func
88-
// can bail out if modified becomes false while
89-
// timing out during autosave
90-
modifiedRef.current = modified
91-
92-
// Store locale in ref so the autosave func
93-
// can always retrieve the most to date locale
94-
localeRef.current = locale
95-
9678
const { queueTask } = useQueue()
9779

9880
const autosaveTimeoutRef = useRef<NodeJS.Timeout | null>(null)
@@ -130,21 +112,21 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
130112

131113
if (collection && id) {
132114
entitySlug = collection.slug
133-
url = `${serverURL}${api}/${entitySlug}/${id}?depth=0&draft=true&autosave=true&locale=${localeRef.current}&fallback-locale=null`
115+
url = `${serverURL}${api}/${entitySlug}/${id}?depth=0&draft=true&autosave=true&locale=${locale}&fallback-locale=null`
134116
method = 'PATCH'
135117
}
136118

137119
if (globalDoc) {
138120
entitySlug = globalDoc.slug
139-
url = `${serverURL}${api}/globals/${entitySlug}?depth=0&draft=true&autosave=true&locale=${localeRef.current}&fallback-locale=null`
121+
url = `${serverURL}${api}/globals/${entitySlug}?depth=0&draft=true&autosave=true&locale=${locale}&fallback-locale=null`
140122
method = 'POST'
141123
}
142124

143-
const { valid } = reduceFieldsToValuesWithValidation(formStateRef.current, true)
125+
const { valid } = reduceFieldsToValuesWithValidation(formState, true)
144126

145127
const skipSubmission = submitted && !valid && validateOnDraft
146128

147-
if (!skipSubmission && modifiedRef.current && url) {
129+
if (!skipSubmission && modified && url) {
148130
const result = await submit<any, OnSaveContext>({
149131
acceptValues: {
150132
overrideLocalChanges: false,

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

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ export const Form: React.FC<FormProps> = (props) => {
106106

107107
const [disabled, setDisabled] = useState(disabledFromProps || false)
108108
const [isMounted, setIsMounted] = useState(false)
109-
const [modified, setModified] = useState(false)
109+
110+
const [submitted, setSubmitted] = useState(false)
110111

111112
/**
112113
* Tracks wether the form state passes validation.
@@ -116,9 +117,50 @@ export const Form: React.FC<FormProps> = (props) => {
116117
const [initializing, setInitializing] = useState(initializingFromProps)
117118

118119
const [processing, setProcessing] = useState(false)
119-
const [backgroundProcessing, setBackgroundProcessing] = useState(false)
120120

121-
const [submitted, setSubmitted] = useState(false)
121+
/**
122+
* Determines whether the form is processing asynchronously in the background, e.g. autosave is running.
123+
* Useful to determine whether to disable the form or queue other processes while in flight, e.g. disable manual submits while an autosave is running.
124+
*/
125+
const [backgroundProcessing, _setBackgroundProcessing] = useState(false)
126+
127+
/**
128+
* A ref that can be read within the `setModified` interceptor.
129+
* Dependents of this state can read it immediately without needing to wait for a render cycle.
130+
*/
131+
const backgroundProcessingRef = useRef(backgroundProcessing)
132+
133+
/**
134+
* Flag to track if the form was modified _during a submission_, e.g. while autosave is running.
135+
* Useful in order to avoid resetting `modified` to false wrongfully after a submit.
136+
* For example, if the user modifies a field while the a background process (autosave) is running,
137+
* we need to ensure that after the submit completes, the `modified` state remains true.
138+
*/
139+
const modifiedWhileProcessingRef = useRef(false)
140+
141+
/**
142+
* Intercept the `setBackgroundProcessing` method to keep the ref in sync.
143+
* See the `backgroundProcessingRef` for more details.
144+
*/
145+
const setBackgroundProcessing = useCallback((backgroundProcessing: boolean) => {
146+
backgroundProcessingRef.current = backgroundProcessing
147+
_setBackgroundProcessing(backgroundProcessing)
148+
}, [])
149+
150+
const [modified, _setModified] = useState(false)
151+
152+
/**
153+
* Intercept the `setModified` method to track whether the event happened during background processing.
154+
* See the `modifiedWhileProcessingRef` ref for more details.
155+
*/
156+
const setModified = useCallback((modified: boolean) => {
157+
if (backgroundProcessingRef.current) {
158+
modifiedWhileProcessingRef.current = true
159+
}
160+
161+
_setModified(modified)
162+
}, [])
163+
122164
const formRef = useRef<HTMLFormElement>(null)
123165
const contextRef = useRef({} as FormContextType)
124166
const abortResetFormRef = useRef<AbortController>(null)
@@ -360,7 +402,12 @@ export const Form: React.FC<FormProps> = (props) => {
360402
res = await action(formData)
361403
}
362404

363-
setModified(false)
405+
if (!modifiedWhileProcessingRef.current) {
406+
setModified(false)
407+
} else {
408+
modifiedWhileProcessingRef.current = false
409+
}
410+
364411
setDisabled(false)
365412

366413
if (typeof handleResponse === 'function') {
@@ -412,6 +459,7 @@ export const Form: React.FC<FormProps> = (props) => {
412459
// Also keep the form as modified so the save button remains enabled for retry.
413460
if (overridesFromArgs['_status'] === 'draft') {
414461
setModified(true)
462+
415463
if (!validateDrafts) {
416464
setSubmitted(false)
417465
}
@@ -498,6 +546,8 @@ export const Form: React.FC<FormProps> = (props) => {
498546
i18n,
499547
validateDrafts,
500548
waitForAutocomplete,
549+
setModified,
550+
setSubmitted,
501551
],
502552
)
503553

@@ -612,6 +662,7 @@ export const Form: React.FC<FormProps> = (props) => {
612662
docPermissions,
613663
getDocPreferences,
614664
locale,
665+
setModified,
615666
],
616667
)
617668

@@ -621,7 +672,7 @@ export const Form: React.FC<FormProps> = (props) => {
621672
setModified(false)
622673
dispatchFields({ type: 'REPLACE_STATE', state })
623674
},
624-
[dispatchFields],
675+
[dispatchFields, setModified],
625676
)
626677

627678
const addFieldRow: FormContextType['addFieldRow'] = useCallback(
@@ -641,7 +692,7 @@ export const Form: React.FC<FormProps> = (props) => {
641692

642693
setModified(true)
643694
},
644-
[dispatchFields, getDataByPath],
695+
[dispatchFields, getDataByPath, setModified],
645696
)
646697

647698
const moveFieldRow: FormContextType['moveFieldRow'] = useCallback(
@@ -655,7 +706,7 @@ export const Form: React.FC<FormProps> = (props) => {
655706

656707
setModified(true)
657708
},
658-
[dispatchFields],
709+
[dispatchFields, setModified],
659710
)
660711

661712
const removeFieldRow: FormContextType['removeFieldRow'] = useCallback(
@@ -664,7 +715,7 @@ export const Form: React.FC<FormProps> = (props) => {
664715

665716
setModified(true)
666717
},
667-
[dispatchFields],
718+
[dispatchFields, setModified],
668719
)
669720

670721
const replaceFieldRow: FormContextType['replaceFieldRow'] = useCallback(
@@ -682,7 +733,7 @@ export const Form: React.FC<FormProps> = (props) => {
682733

683734
setModified(true)
684735
},
685-
[dispatchFields, getDataByPath],
736+
[dispatchFields, getDataByPath, setModified],
686737
)
687738

688739
useEffect(() => {
@@ -763,9 +814,13 @@ export const Form: React.FC<FormProps> = (props) => {
763814
[formState],
764815
)
765816

766-
useEffect(() => {
817+
const handleLocaleChange = useEffectEvent(() => {
767818
contextRef.current = { ...contextRef.current } // triggers rerender of all components that subscribe to form
768819
setModified(false)
820+
})
821+
822+
useEffect(() => {
823+
handleLocaleChange()
769824
}, [locale])
770825

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

test/form-state/e2e.spec.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { PayloadTestSDK } from 'helpers/sdk/index.js'
33
import type { FormState } from 'payload'
44

55
import { expect, test } from '@playwright/test'
6+
import { postSlug } from 'folders/shared.js'
67
import { assertElementStaysVisible } from 'helpers/e2e/assertElementStaysVisible.js'
78
import { assertNetworkRequests } from 'helpers/e2e/assertNetworkRequests.js'
89
import { assertRequestBody } from 'helpers/e2e/assertRequestBody.js'
@@ -18,7 +19,7 @@ import * as path from 'path'
1819
import { wait } from 'payload/shared'
1920
import { fileURLToPath } from 'url'
2021

21-
import type { Config, Post } from './payload-types.js'
22+
import type { AutosavePost, Config, Post } from './payload-types.js'
2223

2324
import {
2425
ensureCompilationIsDone,
@@ -487,13 +488,62 @@ test.describe('Form State', () => {
487488
)
488489
})
489490

491+
test('onChange events are queued even while autosave is in-flight', async () => {
492+
const autosavePost = await payload.create({
493+
collection: autosavePostsSlug,
494+
data: {
495+
title: 'Initial Title',
496+
},
497+
})
498+
499+
await page.goto(autosavePostsUrl.edit(autosavePost.id))
500+
const field = page.locator('#field-title')
501+
await expect(field).toBeEnabled()
502+
503+
const cdpSession = await throttleTest({
504+
page,
505+
context,
506+
delay: 'Slow 3G',
507+
})
508+
509+
try {
510+
await assertNetworkRequests(
511+
page,
512+
`/api/${autosavePostsSlug}/${autosavePost.id}`,
513+
async () => {
514+
// Type a partial word, then pause for longer than debounce rate to trigger first onChange
515+
await field.fill('Tes')
516+
await wait(250) // wait for debounce to elapse, but not long enough for the autosave network request to complete
517+
// Finish the word, which importantly, should trigger a second onChange while the autosave is still in-flight
518+
await field.press('t')
519+
},
520+
{
521+
allowedNumberOfRequests: 2,
522+
minimumNumberOfRequests: 2,
523+
timeout: 10000,
524+
},
525+
)
526+
} finally {
527+
// Ensure throttling is always cleaned up, even if the test fails
528+
await cdpSession.send('Network.emulateNetworkConditions', {
529+
offline: false,
530+
latency: 0,
531+
downloadThroughput: -1,
532+
uploadThroughput: -1,
533+
})
534+
535+
await cdpSession.detach()
536+
}
537+
})
538+
490539
describe('Throttled tests', () => {
491540
let cdpSession: CDPSession
492541

493542
beforeEach(async () => {
494543
await page.goto(postsUrl.create)
495544
const field = page.locator('#field-title')
496545
await field.fill('Test')
546+
await expect(field).toBeEnabled()
497547

498548
cdpSession = await throttleTest({
499549
page,
@@ -580,6 +630,7 @@ test.describe('Form State', () => {
580630
},
581631
{
582632
allowedNumberOfRequests: 2,
633+
minimumNumberOfRequests: 2,
583634
timeout: 10000, // watch network for 10 seconds to allow requests to build up
584635
},
585636
)

0 commit comments

Comments
 (0)