From cb59b09a1027db19cf66bdc0f92978ee23d60077 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 2 Dec 2025 17:29:54 -0500 Subject: [PATCH 1/9] add failing test --- test/form-state/e2e.spec.ts | 60 +++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index cc9bfafbbaf..05d0a8d084f 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,13 +488,65 @@ 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', + }) + + await assertNetworkRequests( + page, + `/api/${autosavePostsSlug}/${autosavePost.id}`, + async () => { + // Type "Hell" then pause for longer than debounce rate to trigger first onChange + await field.fill('Hell') + await wait(250) // wait for debounce to trigger + // Type "o" to trigger second onChange + await field.press('o') + }, + { + allowedNumberOfRequests: 2, + minimumNumberOfRequests: 2, + timeout: 10000, + }, + ) + + await cdpSession.send('Network.emulateNetworkConditions', { + offline: false, + latency: 0, + downloadThroughput: -1, + uploadThroughput: -1, + }) + + await cdpSession.detach() + }) + describe('Throttled tests', () => { let cdpSession: CDPSession beforeEach(async () => { - await page.goto(postsUrl.create) + const post = await payload.create({ + collection: postSlug, + data: { + title: 'Test', + }, + }) + + await page.goto(postsUrl.edit(post.id)) const field = page.locator('#field-title') - await field.fill('Test') + await expect(field).toBeEnabled() cdpSession = await throttleTest({ page, @@ -580,6 +633,7 @@ test.describe('Form State', () => { }, { allowedNumberOfRequests: 2, + minimumNumberOfRequests: 2, timeout: 10000, // watch network for 10 seconds to allow requests to build up }, ) From 77e312bb61394dc394094a9d226c305784bcbb85 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 2 Dec 2025 17:38:14 -0500 Subject: [PATCH 2/9] cleanup --- test/form-state/e2e.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index 05d0a8d084f..c4b86604a87 100644 --- a/test/form-state/e2e.spec.ts +++ b/test/form-state/e2e.spec.ts @@ -510,11 +510,11 @@ test.describe('Form State', () => { page, `/api/${autosavePostsSlug}/${autosavePost.id}`, async () => { - // Type "Hell" then pause for longer than debounce rate to trigger first onChange - await field.fill('Hell') - await wait(250) // wait for debounce to trigger - // Type "o" to trigger second onChange - await field.press('o') + // 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, From 2ba3cda0b7d1f99f60d23302e7e4f292f8aa7256 Mon Sep 17 00:00:00 2001 From: Jake Date: Thu, 4 Dec 2025 13:34:31 -0500 Subject: [PATCH 3/9] cleanup throttling for ci --- test/form-state/e2e.spec.ts | 51 ++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index c4b86604a87..fbc76f6fef0 100644 --- a/test/form-state/e2e.spec.ts +++ b/test/form-state/e2e.spec.ts @@ -506,31 +506,34 @@ test.describe('Form State', () => { delay: 'Slow 3G', }) - 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, - }, - ) - - await cdpSession.send('Network.emulateNetworkConditions', { - offline: false, - latency: 0, - downloadThroughput: -1, - uploadThroughput: -1, - }) + 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() + await cdpSession.detach() + } }) describe('Throttled tests', () => { From 1478a42a7b47feec973e7338a326a344cd08705a Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 09:28:27 -0500 Subject: [PATCH 4/9] more trial to isolate failing test --- test/form-state/e2e.spec.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/form-state/e2e.spec.ts b/test/form-state/e2e.spec.ts index fbc76f6fef0..73904cd7738 100644 --- a/test/form-state/e2e.spec.ts +++ b/test/form-state/e2e.spec.ts @@ -540,15 +540,9 @@ test.describe('Form State', () => { let cdpSession: CDPSession beforeEach(async () => { - const post = await payload.create({ - collection: postSlug, - data: { - title: 'Test', - }, - }) - - await page.goto(postsUrl.edit(post.id)) + await page.goto(postsUrl.create) const field = page.locator('#field-title') + await field.fill('Test') await expect(field).toBeEnabled() cdpSession = await throttleTest({ From 541f59b64b56f7df76869a5cb0d92e2a64e84134 Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 11:20:12 -0500 Subject: [PATCH 5/9] add modifiedWhileProcessing flag to control modified state post-autosave --- packages/ui/src/elements/Autosave/index.tsx | 26 ++----- packages/ui/src/forms/Form/index.tsx | 76 ++++++++++++++++++--- packages/ui/src/forms/Form/types.ts | 2 +- 3 files changed, 70 insertions(+), 34 deletions(-) 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..bbacbc2c1a5 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,45 @@ 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) + /** + * State that determines whether the form is processing asynchronously in the background, e.g. autosave is running. + * Useful to determine whether to disable the form while background processing, e.g. let user continue editing while autosaving. + */ + const [backgroundProcessing, _setBackgroundProcessing] = useState(false) + + /** + * A ref that can be read immediately within `setBackgroundProcessing` without waiting for another render cycle + */ + const backgroundProcessingRef = useRef(backgroundProcessing) + + /** + * Flag to track if the form was modified during a submit. + * Useful in order to avoid setting the modified state to false after a submit. + * For example, if the user modifies while the previous submit is still processing. + * Conditions that watch the modified state rely on its accuracy, e.g. autosave. + */ + const modifiedWhileProcessing = useRef(false) + + const setBackgroundProcessing = useCallback((backgroundProcessing: boolean) => { + backgroundProcessingRef.current = backgroundProcessing + _setBackgroundProcessing(backgroundProcessing) + }, []) + + const [modified, _setModified] = useState(false) + + /** + * Intercept the `setModified` method to track if the form was modified during a submit. + * See the `modifiedDuringSubmit` ref for more details. + */ + const setModified = useCallback((modified: boolean) => { + if (backgroundProcessingRef.current) { + modifiedWhileProcessing.current = true + } + + _setModified(modified) + }, []) + const formRef = useRef(null) const contextRef = useRef({} as FormContextType) const abortResetFormRef = useRef(null) @@ -360,7 +397,12 @@ export const Form: React.FC = (props) => { res = await action(formData) } - setModified(false) + if (!modifiedWhileProcessing.current) { + setModified(false) + } else { + modifiedWhileProcessing.current = false + } + setDisabled(false) if (typeof handleResponse === 'function') { @@ -412,6 +454,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 +541,8 @@ export const Form: React.FC = (props) => { i18n, validateDrafts, waitForAutocomplete, + setModified, + setSubmitted, ], ) @@ -612,6 +657,7 @@ export const Form: React.FC = (props) => { docPermissions, getDocPreferences, locale, + setModified, ], ) @@ -621,7 +667,7 @@ export const Form: React.FC = (props) => { setModified(false) dispatchFields({ type: 'REPLACE_STATE', state }) }, - [dispatchFields], + [dispatchFields, setModified], ) const addFieldRow: FormContextType['addFieldRow'] = useCallback( @@ -641,7 +687,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields, getDataByPath], + [dispatchFields, getDataByPath, setModified], ) const moveFieldRow: FormContextType['moveFieldRow'] = useCallback( @@ -655,7 +701,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields], + [dispatchFields, setModified], ) const removeFieldRow: FormContextType['removeFieldRow'] = useCallback( @@ -664,7 +710,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields], + [dispatchFields, setModified], ) const replaceFieldRow: FormContextType['replaceFieldRow'] = useCallback( @@ -682,7 +728,7 @@ export const Form: React.FC = (props) => { setModified(true) }, - [dispatchFields, getDataByPath], + [dispatchFields, getDataByPath, setModified], ) useEffect(() => { @@ -737,10 +783,14 @@ export const Form: React.FC = (props) => { } }, [disabledFromProps]) - useEffect(() => { + const handleExternalSubmittedChange = useEffectEvent((submittedFromProps: boolean) => { if (typeof submittedFromProps === 'boolean') { setSubmitted(submittedFromProps) } + }) + + useEffect(() => { + handleExternalSubmittedChange(submittedFromProps) }, [submittedFromProps]) useEffect(() => { @@ -763,9 +813,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/packages/ui/src/forms/Form/types.ts b/packages/ui/src/forms/Form/types.ts index 7e24e418081..9e9d06c8626 100644 --- a/packages/ui/src/forms/Form/types.ts +++ b/packages/ui/src/forms/Form/types.ts @@ -8,7 +8,7 @@ import type { ValidationFieldError, } from 'payload' import type React from 'react' -import type { Dispatch } from 'react' +import type { Dispatch, JSX } from 'react' import type { AcceptValues } from './mergeServerFormState.js' From 30f3257ad70ca9f4f60ca06741f963704baf653e Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 11:35:01 -0500 Subject: [PATCH 6/9] cleanup --- packages/ui/src/forms/Form/index.tsx | 6 +----- packages/ui/src/forms/Form/types.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index bbacbc2c1a5..28c3f99f1fc 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -783,14 +783,10 @@ export const Form: React.FC = (props) => { } }, [disabledFromProps]) - const handleExternalSubmittedChange = useEffectEvent((submittedFromProps: boolean) => { + useEffect(() => { if (typeof submittedFromProps === 'boolean') { setSubmitted(submittedFromProps) } - }) - - useEffect(() => { - handleExternalSubmittedChange(submittedFromProps) }, [submittedFromProps]) useEffect(() => { diff --git a/packages/ui/src/forms/Form/types.ts b/packages/ui/src/forms/Form/types.ts index 9e9d06c8626..7e24e418081 100644 --- a/packages/ui/src/forms/Form/types.ts +++ b/packages/ui/src/forms/Form/types.ts @@ -8,7 +8,7 @@ import type { ValidationFieldError, } from 'payload' import type React from 'react' -import type { Dispatch, JSX } from 'react' +import type { Dispatch } from 'react' import type { AcceptValues } from './mergeServerFormState.js' From d27f1da16410c2ae5b9ee252ade52e213946e871 Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 13:02:42 -0500 Subject: [PATCH 7/9] cleanup jsdocs --- packages/ui/src/forms/Form/index.tsx | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 28c3f99f1fc..14a5bd05dc9 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -125,18 +125,23 @@ export const Form: React.FC = (props) => { const [backgroundProcessing, _setBackgroundProcessing] = useState(false) /** - * A ref that can be read immediately within `setBackgroundProcessing` without waiting for another render cycle + * A ref that can be read immediately within `setModified` without waiting for another render cycle. + * Dependents on 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 submit. - * Useful in order to avoid setting the modified state to false after a submit. - * For example, if the user modifies while the previous submit is still processing. - * Conditions that watch the modified state rely on its accuracy, e.g. autosave. + * 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 modifiedWhileProcessing = 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) @@ -145,8 +150,8 @@ export const Form: React.FC = (props) => { const [modified, _setModified] = useState(false) /** - * Intercept the `setModified` method to track if the form was modified during a submit. - * See the `modifiedDuringSubmit` ref for more details. + * Intercept the `setModified` method to track whether the event happened during background processing. + * See the `modifiedWhileProcessing` ref for more details. */ const setModified = useCallback((modified: boolean) => { if (backgroundProcessingRef.current) { From c6fc1885b325c41fc5e521cae06495cf16f7d689 Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 13:06:40 -0500 Subject: [PATCH 8/9] more cleanup to jsdocs --- packages/ui/src/forms/Form/index.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 14a5bd05dc9..50b67a171ee 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -119,14 +119,14 @@ export const Form: React.FC = (props) => { const [processing, setProcessing] = useState(false) /** - * State that determines whether the form is processing asynchronously in the background, e.g. autosave is running. - * Useful to determine whether to disable the form while background processing, e.g. let user continue editing while autosaving. + * 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 immediately within `setModified` without waiting for another render cycle. - * Dependents on this state can read it immediately without needing to wait for a render cycle. + * 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) From bc70dfd3a96282f6030b51f963ba13c1cb9bcf90 Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 5 Dec 2025 13:15:45 -0500 Subject: [PATCH 9/9] rename ref for jarrod --- packages/ui/src/forms/Form/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 50b67a171ee..1661ba44f05 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -136,7 +136,7 @@ export const Form: React.FC = (props) => { * 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 modifiedWhileProcessing = useRef(false) + const modifiedWhileProcessingRef = useRef(false) /** * Intercept the `setBackgroundProcessing` method to keep the ref in sync. @@ -151,11 +151,11 @@ export const Form: React.FC = (props) => { /** * Intercept the `setModified` method to track whether the event happened during background processing. - * See the `modifiedWhileProcessing` ref for more details. + * See the `modifiedWhileProcessingRef` ref for more details. */ const setModified = useCallback((modified: boolean) => { if (backgroundProcessingRef.current) { - modifiedWhileProcessing.current = true + modifiedWhileProcessingRef.current = true } _setModified(modified) @@ -402,10 +402,10 @@ export const Form: React.FC = (props) => { res = await action(formData) } - if (!modifiedWhileProcessing.current) { + if (!modifiedWhileProcessingRef.current) { setModified(false) } else { - modifiedWhileProcessing.current = false + modifiedWhileProcessingRef.current = false } setDisabled(false)