Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 4 additions & 22 deletions packages/ui/src/elements/Autosave/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,6 @@ export const Autosave: React.FC<Props> = ({ 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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These refs are relics of the past, prior to the advent of useEffectEvent.


// 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<NodeJS.Timeout | null>(null)
Expand Down Expand Up @@ -130,21 +112,21 @@ export const Autosave: React.FC<Props> = ({ 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<any, OnSaveContext>({
acceptValues: {
overrideLocalChanges: false,
Expand Down
75 changes: 65 additions & 10 deletions packages/ui/src/forms/Form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ export const Form: React.FC<FormProps> = (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.
Expand All @@ -116,9 +117,50 @@ export const Form: React.FC<FormProps> = (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<HTMLFormElement>(null)
const contextRef = useRef({} as FormContextType)
const abortResetFormRef = useRef<AbortController>(null)
Expand Down Expand Up @@ -360,7 +402,12 @@ export const Form: React.FC<FormProps> = (props) => {
res = await action(formData)
}

setModified(false)
if (!modifiedWhileProcessingRef.current) {
setModified(false)
} else {
modifiedWhileProcessingRef.current = false
}

setDisabled(false)

if (typeof handleResponse === 'function') {
Expand Down Expand Up @@ -412,6 +459,7 @@ export const Form: React.FC<FormProps> = (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)
}
Expand Down Expand Up @@ -498,6 +546,8 @@ export const Form: React.FC<FormProps> = (props) => {
i18n,
validateDrafts,
waitForAutocomplete,
setModified,
setSubmitted,
],
)

Expand Down Expand Up @@ -612,6 +662,7 @@ export const Form: React.FC<FormProps> = (props) => {
docPermissions,
getDocPreferences,
locale,
setModified,
],
)

Expand All @@ -621,7 +672,7 @@ export const Form: React.FC<FormProps> = (props) => {
setModified(false)
dispatchFields({ type: 'REPLACE_STATE', state })
},
[dispatchFields],
[dispatchFields, setModified],
)

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

setModified(true)
},
[dispatchFields, getDataByPath],
[dispatchFields, getDataByPath, setModified],
)

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

setModified(true)
},
[dispatchFields],
[dispatchFields, setModified],
)

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

setModified(true)
},
[dispatchFields],
[dispatchFields, setModified],
)

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

setModified(true)
},
[dispatchFields, getDataByPath],
[dispatchFields, getDataByPath, setModified],
)

useEffect(() => {
Expand Down Expand Up @@ -763,9 +814,13 @@ export const Form: React.FC<FormProps> = (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(' ')
Expand Down
53 changes: 52 additions & 1 deletion test/form-state/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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,
Expand Down Expand Up @@ -487,13 +488,62 @@ 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

beforeEach(async () => {
await page.goto(postsUrl.create)
const field = page.locator('#field-title')
await field.fill('Test')
await expect(field).toBeEnabled()

cdpSession = await throttleTest({
page,
Expand Down Expand Up @@ -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
},
)
Expand Down
Loading