Skip to content

Commit b40c581

Browse files
jacobsfletchjacobsfletch
authored
fix(ui): autosave infinite loop within document drawer (#13007)
Required for #13005. Opening an autosave-enabled document within a drawer triggers an infinite loop when the root document is also autosave-enabled. This was for two reasons: 1. Autosave would run and change the `updatedAt` timestamp. This would trigger another run of autosave, and so on. The timestamp is now removed before comparison to ensure that sequential autosave runs are skipped. 2. The `dequal()` call was not being given the `.current` property off the ref object. This meant that is was never evaluate to `true` and therefore never skip unnecessary autosaves to begin with. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1210697235723932
1 parent 335af1b commit b40c581

File tree

8 files changed

+59
-55
lines changed

8 files changed

+59
-55
lines changed

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

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
6060
const { onSave: onSaveFromDocumentDrawer } = useDocumentDrawerContext()
6161

6262
const { reportUpdate } = useDocumentEvents()
63-
const { dispatchFields, isValid, setBackgroundProcessing, setIsValid, setSubmitted } = useForm()
63+
const { dispatchFields, isValid, setBackgroundProcessing, setIsValid } = useForm()
6464

65-
const [fields] = useAllFormFields()
65+
const [formState] = useAllFormFields()
6666
const modified = useFormModified()
6767
const submitted = useFormSubmitted()
6868

@@ -81,32 +81,27 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
8181
)
8282

8383
const [_saving, setSaving] = useState(false)
84+
8485
const saving = useDeferredValue(_saving)
85-
const debouncedFields = useDebounce(fields, interval)
86-
const fieldRef = useRef(fields)
86+
87+
const debouncedFormState = useDebounce(formState, interval)
88+
89+
const formStateRef = useRef(formState)
8790
const modifiedRef = useRef(modified)
8891
const localeRef = useRef(locale)
89-
/**
90-
* Track the validation internally so Autosave can determine when to run queue processing again
91-
* Helps us prevent infinite loops when the queue is processing and the form is invalid
92-
*/
93-
const isValidRef = useRef(isValid)
9492

9593
// Store fields in ref so the autosave func
9694
// can always retrieve the most to date copies
9795
// after the timeout has executed
98-
// eslint-disable-next-line react-compiler/react-compiler -- TODO: fix
99-
fieldRef.current = fields
96+
formStateRef.current = formState
10097

10198
// Store modified in ref so the autosave func
10299
// can bail out if modified becomes false while
103100
// timing out during autosave
104-
// eslint-disable-next-line react-compiler/react-compiler -- TODO: fix
105101
modifiedRef.current = modified
106102

107103
// Store locale in ref so the autosave func
108104
// can always retrieve the most to date locale
109-
// eslint-disable-next-line react-compiler/react-compiler -- TODO: fix
110105
localeRef.current = locale
111106

112107
const { queueTask } = useQueues()
@@ -158,14 +153,14 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
158153

159154
if (url) {
160155
if (modifiedRef.current) {
161-
const { data, valid } = reduceFieldsToValuesWithValidation(fieldRef.current, true)
156+
const { data, valid } = reduceFieldsToValuesWithValidation(formStateRef.current, true)
162157

163158
data._status = 'draft'
164159

165160
const skipSubmission =
166161
submitted && !valid && versionsConfig?.drafts && versionsConfig?.drafts?.validate
167162

168-
if (!skipSubmission && isValidRef.current) {
163+
if (!skipSubmission) {
169164
let res
170165

171166
try {
@@ -250,10 +245,7 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
250245
toast.error(err.message || i18n.t('error:unknown'))
251246
})
252247

253-
// Set valid to false internally so the queue doesn't process
254-
isValidRef.current = false
255248
setIsValid(false)
256-
setSubmitted(true)
257249
hideIndicator()
258250
return
259251
}
@@ -264,9 +256,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
264256
// Manually update the data since this function doesn't fire the `submit` function from useForm
265257
if (document) {
266258
setIsValid(true)
267-
268-
// Reset internal state allowing the queue to process
269-
isValidRef.current = true
270259
updateSavedDocumentData(document)
271260
}
272261
}
@@ -282,19 +271,15 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
282271
setBackgroundProcessing(false)
283272
},
284273
beforeProcess: () => {
285-
if (!isValidRef.current) {
286-
isValidRef.current = true
287-
return false
288-
}
289-
290274
setBackgroundProcessing(true)
291275
},
292276
},
293277
)
294278
})
295279

296280
const didMount = useRef(false)
297-
const previousDebouncedFieldValues = useRef(reduceFieldsToValues(debouncedFields))
281+
const previousDebouncedData = useRef(reduceFieldsToValues(debouncedFormState))
282+
298283
// When debounced fields change, autosave
299284
useEffect(() => {
300285
/**
@@ -307,16 +292,19 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
307292

308293
/**
309294
* Ensure autosave only runs if the form data changes, not every time the entire form state changes
295+
* Remove `updatedAt` from comparison as it changes on every autosave interval.
310296
*/
311-
const debouncedFieldValues = reduceFieldsToValues(debouncedFields)
312-
if (dequal(debouncedFieldValues, previousDebouncedFieldValues)) {
297+
const { updatedAt: _, ...formData } = reduceFieldsToValues(debouncedFormState)
298+
const { updatedAt: __, ...prevFormData } = previousDebouncedData.current
299+
300+
if (dequal(formData, prevFormData)) {
313301
return
314302
}
315303

316-
previousDebouncedFieldValues.current = debouncedFieldValues
304+
previousDebouncedData.current = formData
317305

318306
handleAutosave()
319-
}, [debouncedFields])
307+
}, [debouncedFormState])
320308

321309
/**
322310
* If component unmounts, clear the autosave timeout

‎packages/ui/src/hooks/useQueues.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type QueuedTaskOptions = {
1414
* Can also be used to perform side effects before processing the queue
1515
* @returns {boolean} If `false`, the queue will not process
1616
*/
17-
beforeProcess?: () => boolean
17+
beforeProcess?: () => boolean | void
1818
}
1919

2020
type QueueTask = (fn: QueuedFunction, options?: QueuedTaskOptions) => void

‎test/versions/collections/AutosaveWithValidate.ts renamed to ‎test/versions/collections/AutosaveWithDraftValidate.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import type { CollectionConfig } from 'payload'
22

3-
import { autosaveWithValidateCollectionSlug } from '../slugs.js'
3+
import { autosaveWithDraftValidateSlug } from '../slugs.js'
44

5-
const AutosaveWithValidatePosts: CollectionConfig = {
6-
slug: autosaveWithValidateCollectionSlug,
5+
const AutosaveWithDraftValidate: CollectionConfig = {
6+
slug: autosaveWithDraftValidateSlug,
77
labels: {
8-
singular: 'Autosave with Validate Post',
9-
plural: 'Autosave with Validate Posts',
8+
singular: 'Autosave with Draft Validate',
9+
plural: 'Autosave with Draft Validate',
1010
},
1111
admin: {
1212
useAsTitle: 'title',
@@ -30,4 +30,4 @@ const AutosaveWithValidatePosts: CollectionConfig = {
3030
],
3131
}
3232

33-
export default AutosaveWithValidatePosts
33+
export default AutosaveWithDraftValidate

‎test/versions/config.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const dirname = path.dirname(filename)
55
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
66
import AutosavePosts from './collections/Autosave.js'
77
import AutosaveWithDraftButtonPosts from './collections/AutosaveWithDraftButton.js'
8-
import AutosaveWithValidate from './collections/AutosaveWithValidate.js'
8+
import AutosaveWithDraftValidate from './collections/AutosaveWithDraftValidate.js'
99
import CustomIDs from './collections/CustomIDs.js'
1010
import { Diff } from './collections/Diff/index.js'
1111
import DisablePublish from './collections/DisablePublish.js'
@@ -41,7 +41,7 @@ export default buildConfigWithDefaults({
4141
Posts,
4242
AutosavePosts,
4343
AutosaveWithDraftButtonPosts,
44-
AutosaveWithValidate,
44+
AutosaveWithDraftValidate,
4545
DraftPosts,
4646
DraftWithMax,
4747
DraftsWithValidate,

‎test/versions/e2e.spec.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
exactText,
3939
initPageConsoleErrorCatch,
4040
saveDocAndAssert,
41+
// throttleTest,
4142
} from '../helpers.js'
4243
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
4344
import { assertNetworkRequests } from '../helpers/e2e/assertNetworkRequests.js'
@@ -50,7 +51,7 @@ import {
5051
autoSaveGlobalSlug,
5152
autosaveWithDraftButtonGlobal,
5253
autosaveWithDraftButtonSlug,
53-
autosaveWithValidateCollectionSlug,
54+
autosaveWithDraftValidateSlug,
5455
customIDSlug,
5556
diffCollectionSlug,
5657
disablePublishGlobalSlug,
@@ -82,7 +83,7 @@ describe('Versions', () => {
8283
let serverURL: string
8384
let autosaveURL: AdminUrlUtil
8485
let autosaveWithDraftButtonURL: AdminUrlUtil
85-
let autosaveWithValidateURL: AdminUrlUtil
86+
let autosaveWithDraftValidateURL: AdminUrlUtil
8687
let draftWithValidateURL: AdminUrlUtil
8788
let disablePublishURL: AdminUrlUtil
8889
let customIDURL: AdminUrlUtil
@@ -103,11 +104,12 @@ describe('Versions', () => {
103104
})
104105

105106
beforeEach(async () => {
106-
/* await throttleTest({
107-
page,
108-
context,
109-
delay: 'Slow 4G',
110-
}) */
107+
// await throttleTest({
108+
// page,
109+
// context,
110+
// delay: 'Fast 4G',
111+
// })
112+
111113
await reInitializeDB({
112114
serverURL,
113115
snapshotKey: 'versionsTest',
@@ -121,7 +123,7 @@ describe('Versions', () => {
121123
url = new AdminUrlUtil(serverURL, draftCollectionSlug)
122124
autosaveURL = new AdminUrlUtil(serverURL, autosaveCollectionSlug)
123125
autosaveWithDraftButtonURL = new AdminUrlUtil(serverURL, autosaveWithDraftButtonSlug)
124-
autosaveWithValidateURL = new AdminUrlUtil(serverURL, autosaveWithValidateCollectionSlug)
126+
autosaveWithDraftValidateURL = new AdminUrlUtil(serverURL, autosaveWithDraftValidateSlug)
125127
disablePublishURL = new AdminUrlUtil(serverURL, disablePublishSlug)
126128
customIDURL = new AdminUrlUtil(serverURL, customIDSlug)
127129
postURL = new AdminUrlUtil(serverURL, postCollectionSlug)
@@ -1059,7 +1061,7 @@ describe('Versions', () => {
10591061

10601062
describe('Collections with draft validation', () => {
10611063
beforeAll(() => {
1062-
autosaveWithValidateURL = new AdminUrlUtil(serverURL, autosaveWithValidateCollectionSlug)
1064+
autosaveWithDraftValidateURL = new AdminUrlUtil(serverURL, autosaveWithDraftValidateSlug)
10631065
draftWithValidateURL = new AdminUrlUtil(serverURL, draftWithValidateCollectionSlug)
10641066
})
10651067

@@ -1173,7 +1175,7 @@ describe('Versions', () => {
11731175
})
11741176

11751177
test('- with autosave - can save', async () => {
1176-
await page.goto(autosaveWithValidateURL.create)
1178+
await page.goto(autosaveWithDraftValidateURL.create)
11771179

11781180
const titleField = page.locator('#field-title')
11791181
await titleField.fill('Initial')
@@ -1191,7 +1193,7 @@ describe('Versions', () => {
11911193

11921194
test('- with autosave - can safely trigger validation errors and then continue editing', async () => {
11931195
// This test has to make sure we don't enter an infinite loop when draft.validate is on and we have autosave enabled
1194-
await page.goto(autosaveWithValidateURL.create)
1196+
await page.goto(autosaveWithDraftValidateURL.create)
11951197

11961198
const titleField = page.locator('#field-title')
11971199
await titleField.fill('Initial')
@@ -1213,7 +1215,7 @@ describe('Versions', () => {
12131215
})
12141216

12151217
test('- with autosave - shows a prevent leave alert when form is submitted but invalid', async () => {
1216-
await page.goto(autosaveWithValidateURL.create)
1218+
await page.goto(autosaveWithDraftValidateURL.create)
12171219

12181220
// Flag to check against if window alert has been displayed and dismissed since we can only check via events
12191221
let alertDisplayed = false

‎test/versions/payload-types.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,13 @@ export interface User {
536536
hash?: string | null;
537537
loginAttempts?: number | null;
538538
lockUntil?: string | null;
539+
sessions?:
540+
| {
541+
id: string;
542+
createdAt?: string | null;
543+
expiresAt: string;
544+
}[]
545+
| null;
539546
password?: string | null;
540547
}
541548
/**
@@ -1049,6 +1056,13 @@ export interface UsersSelect<T extends boolean = true> {
10491056
hash?: T;
10501057
loginAttempts?: T;
10511058
lockUntil?: T;
1059+
sessions?:
1060+
| T
1061+
| {
1062+
id?: T;
1063+
createdAt?: T;
1064+
expiresAt?: T;
1065+
};
10521066
}
10531067
/**
10541068
* This interface was referenced by `Config`'s JSON-Schema

‎test/versions/seed.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { devUser } from '../credentials.js'
88
import { executePromises } from '../helpers/executePromises.js'
99
import { generateLexicalData } from './collections/Diff/generateLexicalData.js'
1010
import {
11-
autosaveWithValidateCollectionSlug,
11+
autosaveWithDraftValidateSlug,
1212
diffCollectionSlug,
1313
draftCollectionSlug,
1414
media2CollectionSlug,
@@ -141,7 +141,7 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
141141
})
142142

143143
await _payload.create({
144-
collection: autosaveWithValidateCollectionSlug,
144+
collection: autosaveWithDraftValidateSlug,
145145
data: {
146146
title: 'Initial seeded title',
147147
},

‎test/versions/slugs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export const autosaveCollectionSlug = 'autosave-posts'
22

33
export const autosaveWithDraftButtonSlug = 'autosave-with-draft-button-posts'
44

5-
export const autosaveWithValidateCollectionSlug = 'autosave-with-validate-posts'
5+
export const autosaveWithDraftValidateSlug = 'autosave-with-validate-posts'
66

77
export const customIDSlug = 'custom-ids'
88

0 commit comments

Comments
 (0)