diff --git a/packages/ui/src/elements/Autosave/index.tsx b/packages/ui/src/elements/Autosave/index.tsx index e4f16cca3de..f54244055a6 100644 --- a/packages/ui/src/elements/Autosave/index.tsx +++ b/packages/ui/src/elements/Autosave/index.tsx @@ -75,24 +75,6 @@ export const Autosave: React.FC = ({ id, collection, global: globalDoc }) const debouncedFormState = useDebounce(formState, interval) - const formStateRef = useRef(formState) - const modifiedRef = useRef(modified) - const localeRef = useRef(locale) - - // Store fields in ref so the autosave func - // can always retrieve the most to date copies - // after the timeout has executed - formStateRef.current = formState - - // Store modified in ref so the autosave func - // can bail out if modified becomes false while - // timing out during autosave - modifiedRef.current = modified - - // Store locale in ref so the autosave func - // can always retrieve the most to date locale - localeRef.current = locale - const { queueTask } = useQueue() const autosaveTimeoutRef = useRef(null) @@ -130,21 +112,21 @@ export const Autosave: React.FC = ({ id, collection, global: globalDoc }) if (collection && id) { entitySlug = collection.slug - url = `${serverURL}${api}/${entitySlug}/${id}?depth=0&draft=true&autosave=true&locale=${localeRef.current}&fallback-locale=null` + url = `${serverURL}${api}/${entitySlug}/${id}?depth=0&draft=true&autosave=true&locale=${locale}&fallback-locale=null` method = 'PATCH' } if (globalDoc) { entitySlug = globalDoc.slug - url = `${serverURL}${api}/globals/${entitySlug}?depth=0&draft=true&autosave=true&locale=${localeRef.current}&fallback-locale=null` + url = `${serverURL}${api}/globals/${entitySlug}?depth=0&draft=true&autosave=true&locale=${locale}&fallback-locale=null` method = 'POST' } - const { valid } = reduceFieldsToValuesWithValidation(formStateRef.current, true) + const { valid } = reduceFieldsToValuesWithValidation(formState, true) const skipSubmission = submitted && !valid && validateOnDraft - if (!skipSubmission && modifiedRef.current && url) { + if (!skipSubmission && modified && url) { const result = await submit({ acceptValues: { overrideLocalChanges: false, diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 4d832e1509f..1661ba44f05 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -106,7 +106,8 @@ export const Form: React.FC = (props) => { const [disabled, setDisabled] = useState(disabledFromProps || false) const [isMounted, setIsMounted] = useState(false) - const [modified, setModified] = useState(false) + + const [submitted, setSubmitted] = useState(false) /** * Tracks wether the form state passes validation. @@ -116,9 +117,50 @@ export const Form: React.FC = (props) => { const [initializing, setInitializing] = useState(initializingFromProps) const [processing, setProcessing] = useState(false) - const [backgroundProcessing, setBackgroundProcessing] = useState(false) - const [submitted, setSubmitted] = useState(false) + /** + * Determines whether the form is processing asynchronously in the background, e.g. autosave is running. + * 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. + */ + const [backgroundProcessing, _setBackgroundProcessing] = useState(false) + + /** + * A ref that can be read within the `setModified` interceptor. + * Dependents of this state can read it immediately without needing to wait for a render cycle. + */ + const backgroundProcessingRef = useRef(backgroundProcessing) + + /** + * Flag to track if the form was modified _during a submission_, e.g. while autosave is running. + * Useful in order to avoid resetting `modified` to false wrongfully after a submit. + * For example, if the user modifies a field while the a background process (autosave) is running, + * we need to ensure that after the submit completes, the `modified` state remains true. + */ + const modifiedWhileProcessingRef = useRef(false) + + /** + * Intercept the `setBackgroundProcessing` method to keep the ref in sync. + * See the `backgroundProcessingRef` for more details. + */ + const setBackgroundProcessing = useCallback((backgroundProcessing: boolean) => { + backgroundProcessingRef.current = backgroundProcessing + _setBackgroundProcessing(backgroundProcessing) + }, []) + + const [modified, _setModified] = useState(false) + + /** + * Intercept the `setModified` method to track whether the event happened during background processing. + * See the `modifiedWhileProcessingRef` ref for more details. + */ + const setModified = useCallback((modified: boolean) => { + if (backgroundProcessingRef.current) { + modifiedWhileProcessingRef.current = true + } + + _setModified(modified) + }, []) + const formRef = useRef(null) const contextRef = useRef({} as FormContextType) const abortResetFormRef = useRef(null) @@ -360,7 +402,12 @@ export const Form: React.FC = (props) => { res = await action(formData) } - setModified(false) + if (!modifiedWhileProcessingRef.current) { + setModified(false) + } else { + modifiedWhileProcessingRef.current = false + } + setDisabled(false) if (typeof handleResponse === 'function') { @@ -412,6 +459,7 @@ export const Form: React.FC = (props) => { // Also keep the form as modified so the save button remains enabled for retry. if (overridesFromArgs['_status'] === 'draft') { setModified(true) + if (!validateDrafts) { setSubmitted(false) } @@ -498,6 +546,8 @@ export const Form: React.FC = (props) => { i18n, validateDrafts, waitForAutocomplete, + setModified, + setSubmitted, ], ) @@ -612,6 +662,7 @@ export const Form: React.FC = (props) => { docPermissions, getDocPreferences, locale, + setModified, ], ) @@ -621,7 +672,7 @@ export const Form: React.FC = (props) => { setModified(false) dispatchFields({ type: 'REPLACE_STATE', state }) }, - [dispatchFields], + [dispatchFields, setModified], ) const addFieldRow: FormContextType['addFieldRow'] = useCallback( @@ -641,7 +692,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields, getDataByPath], + [dispatchFields, getDataByPath, setModified], ) const moveFieldRow: FormContextType['moveFieldRow'] = useCallback( @@ -655,7 +706,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields], + [dispatchFields, setModified], ) const removeFieldRow: FormContextType['removeFieldRow'] = useCallback( @@ -664,7 +715,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields], + [dispatchFields, setModified], ) const replaceFieldRow: FormContextType['replaceFieldRow'] = useCallback( @@ -682,7 +733,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields, getDataByPath], + [dispatchFields, getDataByPath, setModified], ) useEffect(() => { @@ -763,9 +814,13 @@ export const Form: React.FC = (props) => { [formState], ) - useEffect(() => { + const handleLocaleChange = useEffectEvent(() => { contextRef.current = { ...contextRef.current } // triggers rerender of all components that subscribe to form setModified(false) + }) + + useEffect(() => { + handleLocaleChange() }, [locale]) const classes = [className, baseClass].filter(Boolean).join(' ') diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index cc9bfafbbaf..73904cd7738 100644 --- a/test/form-state/e2e.spec.ts +++ b/test/form-state/e2e.spec.ts @@ -3,6 +3,7 @@ import type { PayloadTestSDK } from 'helpers/sdk/index.js' import type { FormState } from 'payload' import { expect, test } from '@playwright/test' +import { postSlug } from 'folders/shared.js' import { assertElementStaysVisible } from 'helpers/e2e/assertElementStaysVisible.js' import { assertNetworkRequests } from 'helpers/e2e/assertNetworkRequests.js' import { assertRequestBody } from 'helpers/e2e/assertRequestBody.js' @@ -18,7 +19,7 @@ import * as path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' -import type { Config, Post } from './payload-types.js' +import type { AutosavePost, Config, Post } from './payload-types.js' import { ensureCompilationIsDone, @@ -487,6 +488,54 @@ test.describe('Form State', () => { ) }) + test('onChange events are queued even while autosave is in-flight', async () => { + const autosavePost = await payload.create({ + collection: autosavePostsSlug, + data: { + title: 'Initial Title', + }, + }) + + await page.goto(autosavePostsUrl.edit(autosavePost.id)) + const field = page.locator('#field-title') + await expect(field).toBeEnabled() + + const cdpSession = await throttleTest({ + page, + context, + delay: 'Slow 3G', + }) + + try { + await assertNetworkRequests( + page, + `/api/${autosavePostsSlug}/${autosavePost.id}`, + async () => { + // Type a partial word, then pause for longer than debounce rate to trigger first onChange + await field.fill('Tes') + await wait(250) // wait for debounce to elapse, but not long enough for the autosave network request to complete + // Finish the word, which importantly, should trigger a second onChange while the autosave is still in-flight + await field.press('t') + }, + { + allowedNumberOfRequests: 2, + minimumNumberOfRequests: 2, + timeout: 10000, + }, + ) + } finally { + // Ensure throttling is always cleaned up, even if the test fails + await cdpSession.send('Network.emulateNetworkConditions', { + offline: false, + latency: 0, + downloadThroughput: -1, + uploadThroughput: -1, + }) + + await cdpSession.detach() + } + }) + describe('Throttled tests', () => { let cdpSession: CDPSession @@ -494,6 +543,7 @@ test.describe('Form State', () => { await page.goto(postsUrl.create) const field = page.locator('#field-title') await field.fill('Test') + await expect(field).toBeEnabled() cdpSession = await throttleTest({ page, @@ -580,6 +630,7 @@ test.describe('Form State', () => { }, { allowedNumberOfRequests: 2, + minimumNumberOfRequests: 2, timeout: 10000, // watch network for 10 seconds to allow requests to build up }, )