Skip to content

Commit 1328522

Browse files
authored
fix(ui): database errors when running autosave and ensure autosave doesn't run unnecessarily (#11270)
## Change 1 - database errors when running autosave The previous autosave implementation allowed multiple autosave fetch calls (=> save document draft) to run in parallel. While the AbortController aborted previous autosave calls if a new one comes in in order to only process the latest one, this had one flaw: Using the AbortController to abort the autosave call only aborted the `fetch` call - it did not however abort the database operation that may have started as part of this fetch call. If you then started a new autosave call, this will start yet another database operation on the backend, resulting in two database operations that would be running at the same time. This has caused a lot of transaction errors that were only noticeable when connected to a slower, remote database. This PR removes the AbortController and ensures that the previous autosave operation is properly awaited before starting a new one, while still discarding outdated autosave requests from the queue **that have not started yet**. Additionally, it cleans up the AutoSave component to make it more readable. ## Change 2 - ensure autosave doesn't run unnecessarily If connected to a slower backend or database, one change in a document may trigger two autosave operations instead of just one. This is how it could happen: 1. Type something => formstate changes => autosave is triggered 2. 200ms later: form state request is triggered. Autosave is still processing 3. 100ms later: form state comes back from server => local form state is updated => another autosave is triggered 4. First autosave is aborted - this lead to a browser error. This PR ensures that that error is no longer surfaced to the user 5. Another autosave is started This PR adds additional checks to ONLY trigger an autosave if the form DATA (not the entire form state itself) changes. Previously, it ran every time the object reference of the form state changes. This includes changes that do not affect the form data, like `field.valid`. => Basically every time form state comes back from the server, we were triggering another, unnecessary autosave
1 parent 7f5aaad commit 1328522

File tree

2 files changed

+156
-130
lines changed

2 files changed

+156
-130
lines changed

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

Lines changed: 155 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// TODO: abstract the `next/navigation` dependency out from this component
33
import type { ClientCollectionConfig, ClientGlobalConfig } from 'payload'
44

5-
import { versionDefaults } from 'payload/shared'
6-
import React, { useEffect, useRef, useState } from 'react'
5+
import { dequal } from 'dequal/lite'
6+
import { reduceFieldsToValues, versionDefaults } from 'payload/shared'
7+
import React, { useDeferredValue, useEffect, useRef, useState } from 'react'
78
import { toast } from 'sonner'
89

910
import {
@@ -18,8 +19,8 @@ import { useConfig } from '../../providers/Config/index.js'
1819
import { useDocumentEvents } from '../../providers/DocumentEvents/index.js'
1920
import { useDocumentInfo } from '../../providers/DocumentInfo/index.js'
2021
import { useLocale } from '../../providers/Locale/index.js'
21-
import { useTranslation } from '../../providers/Translation/index.js'
2222
import './index.scss'
23+
import { useTranslation } from '../../providers/Translation/index.js'
2324
import { formatTimeToNow } from '../../utilities/formatDate.js'
2425
import { reduceFieldsToValuesWithValidation } from '../../utilities/reduceFieldsToValuesWithValidation.js'
2526
import { LeaveWithoutSaving } from '../LeaveWithoutSaving/index.js'
@@ -76,7 +77,8 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
7677
docConfig?.versions?.drafts && docConfig?.versions?.drafts.validate,
7778
)
7879

79-
const [saving, setSaving] = useState(false)
80+
const [_saving, setSaving] = useState(false)
81+
const saving = useDeferredValue(_saving)
8082
const debouncedFields = useDebounce(fields, interval)
8183
const fieldRef = useRef(fields)
8284
const modifiedRef = useRef(modified)
@@ -86,9 +88,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
8688
* Helps us prevent infinite loops when the queue is processing and the form is invalid
8789
*/
8890
const isValidRef = useRef(isValid)
89-
const debouncedRef = useRef(debouncedFields)
90-
91-
debouncedRef.current = debouncedFields
9291

9392
// Store fields in ref so the autosave func
9493
// can always retrieve the most to date copies
@@ -116,8 +115,8 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
116115
isValidRef.current = true
117116
return
118117
}
119-
120118
isProcessingRef.current = true
119+
121120
const latestAction = queueRef.current[queueRef.current.length - 1]
122121
queueRef.current = []
123122

@@ -131,13 +130,28 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
131130
}
132131
}, [])
133132

133+
const autosaveTimeoutRef = useRef<NodeJS.Timeout | null>(null)
134+
134135
const handleAutosave = useEffectEvent(() => {
135-
const abortController = new AbortController()
136-
let autosaveTimeout = undefined
136+
autosaveTimeoutRef.current = undefined
137137
// We need to log the time in order to figure out if we need to trigger the state off later
138138
let startTimestamp = undefined
139139
let endTimestamp = undefined
140140

141+
const hideIndicator = () => {
142+
// If request was faster than minimum animation time, animate the difference
143+
if (endTimestamp - startTimestamp < minimumAnimationTime) {
144+
autosaveTimeoutRef.current = setTimeout(
145+
() => {
146+
setSaving(false)
147+
},
148+
minimumAnimationTime - (endTimestamp - startTimestamp),
149+
)
150+
} else {
151+
stopAutoSaveIndicator()
152+
}
153+
}
154+
141155
const autosave = async () => {
142156
if (modified) {
143157
startTimestamp = new Date().getTime()
@@ -162,120 +176,110 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
162176

163177
if (url) {
164178
if (modifiedRef.current) {
165-
const { data, valid } = {
166-
...reduceFieldsToValuesWithValidation(fieldRef.current, true),
167-
}
179+
const { data, valid } = reduceFieldsToValuesWithValidation(fieldRef.current, true)
180+
168181
data._status = 'draft'
182+
169183
const skipSubmission =
170184
submitted && !valid && versionsConfig?.drafts && versionsConfig?.drafts?.validate
171185

172186
if (!skipSubmission && isValidRef.current) {
173-
await fetch(url, {
174-
body: JSON.stringify(data),
175-
credentials: 'include',
176-
headers: {
177-
'Accept-Language': i18n.language,
178-
'Content-Type': 'application/json',
179-
},
180-
method,
181-
signal: abortController.signal,
182-
})
183-
.then((res) => {
184-
const newDate = new Date()
185-
// We need to log the time in order to figure out if we need to trigger the state off later
186-
endTimestamp = newDate.getTime()
187-
188-
if (res.status === 200) {
189-
setLastUpdateTime(newDate.getTime())
190-
191-
reportUpdate({
192-
id,
193-
entitySlug,
194-
updatedAt: newDate.toISOString(),
195-
})
196-
197-
if (!mostRecentVersionIsAutosaved) {
198-
incrementVersionCount()
199-
setMostRecentVersionIsAutosaved(true)
200-
setUnpublishedVersionCount((prev) => prev + 1)
201-
}
202-
}
203-
204-
return res.json()
187+
let res
188+
try {
189+
res = await fetch(url, {
190+
body: JSON.stringify(data),
191+
credentials: 'include',
192+
headers: {
193+
'Accept-Language': i18n.language,
194+
'Content-Type': 'application/json',
195+
},
196+
method,
205197
})
206-
.then((json) => {
207-
if (versionsConfig?.drafts && versionsConfig?.drafts?.validate && json?.errors) {
208-
if (Array.isArray(json.errors)) {
209-
const [fieldErrors, nonFieldErrors] = json.errors.reduce(
210-
([fieldErrs, nonFieldErrs], err) => {
211-
const newFieldErrs = []
212-
const newNonFieldErrs = []
213-
214-
if (err?.message) {
215-
newNonFieldErrs.push(err)
216-
}
198+
} catch (error) {
199+
// Swallow Error
200+
}
217201

218-
if (Array.isArray(err?.data)) {
219-
err.data.forEach((dataError) => {
220-
if (dataError?.field) {
221-
newFieldErrs.push(dataError)
222-
} else {
223-
newNonFieldErrs.push(dataError)
224-
}
225-
})
226-
}
202+
const newDate = new Date()
203+
// We need to log the time in order to figure out if we need to trigger the state off later
204+
endTimestamp = newDate.getTime()
227205

228-
return [
229-
[...fieldErrs, ...newFieldErrs],
230-
[...nonFieldErrs, ...newNonFieldErrs],
231-
]
232-
},
233-
[[], []],
234-
)
235-
236-
dispatchFields({
237-
type: 'ADD_SERVER_ERRORS',
238-
errors: fieldErrors,
239-
})
240-
241-
nonFieldErrors.forEach((err) => {
242-
toast.error(err.message || i18n.t('error:unknown'))
243-
})
244-
245-
// Set valid to false internally so the queue doesn't process
246-
isValidRef.current = false
247-
setIsValid(false)
248-
setSubmitted(true)
249-
250-
return
251-
}
252-
} else {
253-
// If it's not an error then we can update the document data inside the context
254-
const document = json?.doc || json?.result
255-
256-
// Manually update the data since this function doesn't fire the `submit` function from useForm
257-
if (document) {
258-
setIsValid(true)
259-
260-
// Reset internal state allowing the queue to process
261-
isValidRef.current = true
262-
updateSavedDocumentData(document)
263-
}
264-
}
265-
})
266-
.finally(() => {
267-
// If request was faster than minimum animation time, animate the difference
268-
if (endTimestamp - startTimestamp < minimumAnimationTime) {
269-
autosaveTimeout = setTimeout(
270-
() => {
271-
setSaving(false)
272-
},
273-
minimumAnimationTime - (endTimestamp - startTimestamp),
274-
)
275-
} else {
276-
setSaving(false)
277-
}
206+
if (res.status === 200) {
207+
setLastUpdateTime(newDate.getTime())
208+
209+
reportUpdate({
210+
id,
211+
entitySlug,
212+
updatedAt: newDate.toISOString(),
278213
})
214+
215+
if (!mostRecentVersionIsAutosaved) {
216+
incrementVersionCount()
217+
setMostRecentVersionIsAutosaved(true)
218+
setUnpublishedVersionCount((prev) => prev + 1)
219+
}
220+
}
221+
const json = await res.json()
222+
223+
if (versionsConfig?.drafts && versionsConfig?.drafts?.validate && json?.errors) {
224+
if (Array.isArray(json.errors)) {
225+
const [fieldErrors, nonFieldErrors] = json.errors.reduce(
226+
([fieldErrs, nonFieldErrs], err) => {
227+
const newFieldErrs = []
228+
const newNonFieldErrs = []
229+
230+
if (err?.message) {
231+
newNonFieldErrs.push(err)
232+
}
233+
234+
if (Array.isArray(err?.data)) {
235+
err.data.forEach((dataError) => {
236+
if (dataError?.field) {
237+
newFieldErrs.push(dataError)
238+
} else {
239+
newNonFieldErrs.push(dataError)
240+
}
241+
})
242+
}
243+
244+
return [
245+
[...fieldErrs, ...newFieldErrs],
246+
[...nonFieldErrs, ...newNonFieldErrs],
247+
]
248+
},
249+
[[], []],
250+
)
251+
252+
dispatchFields({
253+
type: 'ADD_SERVER_ERRORS',
254+
errors: fieldErrors,
255+
})
256+
257+
nonFieldErrors.forEach((err) => {
258+
toast.error(err.message || i18n.t('error:unknown'))
259+
})
260+
261+
// Set valid to false internally so the queue doesn't process
262+
isValidRef.current = false
263+
setIsValid(false)
264+
setSubmitted(true)
265+
hideIndicator()
266+
return
267+
}
268+
} else {
269+
// If it's not an error then we can update the document data inside the context
270+
const document = json?.doc || json?.result
271+
272+
// Manually update the data since this function doesn't fire the `submit` function from useForm
273+
if (document) {
274+
setIsValid(true)
275+
276+
// Reset internal state allowing the queue to process
277+
isValidRef.current = true
278+
updateSavedDocumentData(document)
279+
}
280+
}
281+
282+
hideIndicator()
279283
}
280284
}
281285
}
@@ -284,29 +288,50 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
284288

285289
queueRef.current.push(autosave)
286290
void processQueue()
287-
288-
return { abortController, autosaveTimeout }
289291
})
290292

293+
const didMount = useRef(false)
294+
const previousDebouncedFieldValues = useRef(reduceFieldsToValues(debouncedFields))
291295
// When debounced fields change, autosave
292296
useEffect(() => {
293-
const { abortController, autosaveTimeout } = handleAutosave()
297+
/**
298+
* Ensure autosave doesn't run on mount
299+
*/
300+
if (!didMount.current) {
301+
didMount.current = true
302+
return
303+
}
294304

295-
return () => {
296-
if (autosaveTimeout) {
297-
clearTimeout(autosaveTimeout)
298-
}
299-
if (abortController.signal) {
300-
try {
301-
abortController.abort('Autosave closed early.')
302-
} catch (error) {
303-
// swallow error
304-
}
305-
}
306-
setSaving(false)
305+
/**
306+
* Ensure autosave only runs if the form data changes, not every time the entire form state changes
307+
*/
308+
const debouncedFieldValues = reduceFieldsToValues(debouncedFields)
309+
if (dequal(debouncedFieldValues, previousDebouncedFieldValues)) {
310+
return
307311
}
312+
313+
previousDebouncedFieldValues.current = debouncedFieldValues
314+
315+
handleAutosave()
308316
}, [debouncedFields])
309317

318+
/**
319+
* If component unmounts, clear the autosave timeout
320+
*/
321+
useEffect(() => {
322+
return () => {
323+
stopAutoSaveIndicator()
324+
}
325+
}, [])
326+
327+
const stopAutoSaveIndicator = useEffectEvent(() => {
328+
if (autosaveTimeoutRef.current) {
329+
clearTimeout(autosaveTimeoutRef.current)
330+
}
331+
332+
setSaving(false)
333+
})
334+
310335
return (
311336
<div className={baseClass}>
312337
{validateOnDraft && !isValid && <LeaveWithoutSaving />}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ export const Form: React.FC<FormProps> = (props) => {
446446
},
447447
[
448448
beforeSubmit,
449+
startRouteTransition,
449450
action,
450451
disableSuccessStatus,
451452
disableValidationOnSubmit,

0 commit comments

Comments
 (0)