tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix(ui): form state race conditions (#12026)

Browse Source 
Fixes form state race conditions. Modifying state while a request is in
flight or while the response is being processed could result in those
changes being overridden.

This was happening for a few reasons:

1. Our merge logic was incorrect. We were disregarding local changes to
state that may have occurred while form state requests are pending. This
was because we were iterating over local state, then while building up
new state, we were ignoring any fields that did not exist in the server
response, like this:
    
    ```ts
    for (const [path, newFieldState] of Object.entries(existingState)) {
    
      if (!incomingState[path]) {
        continue
      }
      
      // ...
    }
    ```

To fix this, we need to use local state as the source of truth. Then
when the server state arrives, we need to iterate over _that_. If a
field matches in local state, merge in any new properties. This will
ensure all changes to the underlying state are preserved, including any
potential addition or deletions.
    
However, this logic breaks down if the server might have created _new_
fields, like when populating array rows. This means they, too, would be
ignored. To get around this, there is a new `addedByServer` property
that flags new fields to ensure they are kept.
    
This new merge strategy also saves an additional loop over form state.
    
1. We were merging form state based on a mutable ref. This meant that
changes made within one action cause concurrent actions to have dirty
reads. The fix for this is to merge in an isolated manner by copying
state. This will remove any object references. It is generally not good
practice to mutate state without setting it, anyways, as this causes
mismatches between what is rendered and what is in memory.
    
1. We were merging server form state directly within an effect, then
replacing state entirely. This meant that if another action took place
at the exact moment in time _after_ merge but _before_ dispatch, the
results of that other action would be completely overridden. The fix for
this is to perform the merge within the reducer itself. This will ensure
that we are working with a trustworthy snapshot of state at the exact
moment in time that the action was invoked, and that React can properly
queue the event within its lifecycle.
This commit is contained in:
Jacob Fletcher
2025-04-10 12:11:54 -04:00
committed by GitHub
parent 37bfc63da2
commit 4d7c1d45fa
19 changed files with 617 additions and 481 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
packages/payload/src/admin/forms/Form.ts
View File

@@ -26,6 +26,13 @@ export type FilterOptionsResult = {
}
export type FieldState = {
/**
* This is used to determine if the field was added by the server.
* This ensures the field is not ignored by the client when merging form state.
* This can happen because the current local state is treated as the source of truth.
* See `mergeServerFormState` for more details.
*/
addedByServer?: boolean
customComponents?: {
/**
* This is used by UI fields, as they can have arbitrary components defined if used
@@ -60,18 +67,6 @@ export type FieldState = {
lastRenderedPath?: string
passesCondition?: boolean
rows?: Row[]
/**
* The `serverPropsToIgnore` obj is used to prevent the various properties from being overridden across form state requests.
* This can happen when queueing a form state request with `requiresRender: true` while the another is already processing.
* For example:
* 1. One "add row" action will set `requiresRender: true` and dispatch a form state request
* 2. Another "add row" action will set `requiresRender: true` and queue a form state request
* 3. The first request will return with `requiresRender: false`
* 4. The second request will be dispatched with `requiresRender: false` but should be `true`
* To fix this, only merge the `requiresRender` property if the previous state has not set it to `true`.
* See the `mergeServerFormState` function for implementation details.
*/
serverPropsToIgnore?: Array<keyof FieldState>
valid?: boolean
validate?: Validate
value?: unknown

17
packages/ui/src/forms/Form/fieldReducer.ts
View File

@@ -3,10 +3,11 @@ import type { FormField, FormState, Row } from 'payload'
import ObjectIdImport from 'bson-objectid'
import { dequal } from 'dequal/lite' // lite: no need for Map and Set support
import { deepCopyObjectSimple, deepCopyObjectSimpleWithoutReactComponents } from 'payload/shared'
import { deepCopyObjectSimpleWithoutReactComponents } from 'payload/shared'
import type { FieldAction } from './types.js'
import { mergeServerFormState } from './mergeServerFormState.js'
import { flattenRows, separateRows } from './rows.js'
const ObjectId = (ObjectIdImport.default ||
@@ -179,6 +180,20 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
return newState
}
case 'MERGE_SERVER_STATE': {
const { acceptValues, prevStateRef, serverState } = action
const newState = mergeServerFormState({
acceptValues,
currentState: state || {},
incomingState: serverState,
})
prevStateRef.current = newState
return newState
}
case 'MOVE_ROW': {
const { moveFromIndex, moveToIndex, path } = action

40
packages/ui/src/forms/Form/index.tsx
View File

@@ -52,7 +52,6 @@ import {
import { errorMessages } from './errorMessages.js'
import { fieldReducer } from './fieldReducer.js'
import { initContextState } from './initContextState.js'
import { mergeServerFormState } from './mergeServerFormState.js'
const baseClass = 'form'
@@ -368,17 +367,13 @@ export const Form: React.FC<FormProps> = (props) => {
if (res.status < 400) {
if (typeof onSuccess === 'function') {
const newFormState = await onSuccess(json)
if (newFormState) {
const { newState: mergedFormState } = mergeServerFormState({
acceptValues: true,
existingState: contextRef.current.fields || {},
incomingState: newFormState,
})
if (newFormState) {
dispatchFields({
type: 'REPLACE_STATE',
optimize: false,
state: mergedFormState,
type: 'MERGE_SERVER_STATE',
acceptValues: true,
prevStateRef: prevFormState,
serverState: newFormState,
})
}
}
@@ -740,35 +735,22 @@ export const Form: React.FC<FormProps> = (props) => {
const executeOnChange = useEffectEvent((submitted: boolean) => {
queueTask(async () => {
if (Array.isArray(onChange)) {
let revalidatedFormState: FormState = contextRef.current.fields
let serverState: FormState
for (const onChangeFn of onChange) {
// Edit view default onChange is in packages/ui/src/views/Edit/index.tsx. This onChange usually sends a form state request
revalidatedFormState = await onChangeFn({
formState: deepCopyObjectSimpleWithoutReactComponents(contextRef.current.fields),
serverState = await onChangeFn({
formState: deepCopyObjectSimpleWithoutReactComponents(formState),
submitted,
})
}
if (!revalidatedFormState) {
return
}
const { changed, newState } = mergeServerFormState({
existingState: contextRef.current.fields || {},
incomingState: revalidatedFormState,
})
if (changed) {
prevFormState.current = newState
dispatchFields({
type: 'REPLACE_STATE',
optimize: false,
state: newState,
type: 'MERGE_SERVER_STATE',
prevStateRef: prevFormState,
serverState,
})
}
}
})
})

39
packages/ui/src/forms/Form/mergeErrorPaths.ts
View File

@@ -1,39 +0,0 @@
'use client'
import { arraysHaveSameStrings } from '../../utilities/arraysHaveSameStrings.js'
export const mergeErrorPaths = (
existing?: string[],
incoming?: string[],
): {
changed: boolean
result?: string[]
} => {
if (!existing) {
return {
changed: false,
result: undefined,
}
}
const existingErrorPaths: string[] = []
const incomingErrorPaths: string[] = []
if (Array.isArray(incoming) && incoming?.length) {
incoming.forEach((path) => incomingErrorPaths.push(path))
}
if (Array.isArray(existing) && existing?.length) {
existing.forEach((path) => existingErrorPaths.push(path))
}
if (!arraysHaveSameStrings(existingErrorPaths, incomingErrorPaths)) {
return {
changed: true,
result: incomingErrorPaths,
}
}
return {
changed: false,
result: existing,
}
}

155
packages/ui/src/forms/Form/mergeServerFormState.ts
View File

@@ -1,155 +1,84 @@
'use client'
import type { FieldState, FormState } from 'payload'
import type { FormState } from 'payload'
import { dequal } from 'dequal/lite' // lite: no need for Map and Set support
import { mergeErrorPaths } from './mergeErrorPaths.js'
type Args = {
acceptValues?: boolean
existingState: FormState
currentState?: FormState
incomingState: FormState
}
/**
* Merges certain properties from the server state into the client state. These do not include values,
* as we do not want to update them on the client like that, which would cause flickering.
* This function receives form state from the server and intelligently merges it into the client state.
* The server contains extra properties that the client may not have, e.g. custom components and error states.
* We typically do not want to merge properties that rely on user input, however, such as values, unless explicitly requested.
* Doing this would cause the client to lose any local changes to those fields.
*
* We want to use this to update the error state, and other properties that are not user input, as the error state
* is the thing we want to keep in sync with the server (where it's calculated) on the client.
* This function will also a few defaults, as well as clean up the server response in preparation for the client.
* e.g. it will set `valid` and `passesCondition` to true if undefined, and remove `addedByServer` from the response.
*/
export const mergeServerFormState = ({
acceptValues,
existingState,
currentState = {},
incomingState,
}: Args): { changed: boolean; newState: FormState } => {
let changed = false
}: Args): FormState => {
const newState = { ...currentState }
const newState = {}
if (existingState) {
const serverPropsToAccept: Array<keyof FieldState> = [
'passesCondition',
'valid',
'errorMessage',
'errorPaths',
'customComponents',
]
if (acceptValues) {
serverPropsToAccept.push('value')
serverPropsToAccept.push('initialValue')
}
for (const [path, newFieldState] of Object.entries(existingState)) {
if (!incomingState[path]) {
for (const [path, incomingField] of Object.entries(incomingState || {})) {
if (!(path in currentState) && !incomingField.addedByServer) {
continue
}
let fieldChanged = false
/**
* Handle error paths
*/
const errorPathsResult = mergeErrorPaths(
newFieldState.errorPaths,
incomingState[path].errorPaths as unknown as string[],
)
if (errorPathsResult.result) {
if (errorPathsResult.changed) {
changed = errorPathsResult.changed
if (!acceptValues && !incomingField.addedByServer) {
delete incomingField.value
delete incomingField.initialValue
}
newFieldState.errorPaths = errorPathsResult.result
newState[path] = {
...currentState[path],
...incomingField,
}
/**
* Handle filterOptions
*/
if (incomingState[path]?.filterOptions || newFieldState.filterOptions) {
if (!dequal(incomingState[path]?.filterOptions, newFieldState.filterOptions)) {
changed = true
fieldChanged = true
newFieldState.filterOptions = incomingState[path].filterOptions
}
}
/**
* Need to intelligently merge the rows array to ensure changes to local state are not lost while the request was pending
* Intelligently merge the rows array to ensure changes to local state are not lost while the request was pending
* For example, the server response could come back with a row which has been deleted on the client
* Loop over the incoming rows, if it exists in client side form state, merge in any new properties from the server
* Note: read `currentState` and not `newState` here, as the `rows` property have already been merged above
*/
if (Array.isArray(incomingState[path].rows)) {
incomingState[path].rows.forEach((row) => {
const matchedExistingRowIndex = newFieldState.rows.findIndex(
if (Array.isArray(incomingField.rows) && path in currentState) {
newState[path].rows = [...(currentState[path]?.rows || [])] // shallow copy to avoid mutating the original array
incomingField.rows.forEach((row) => {
const indexInCurrentState = currentState[path].rows.findIndex(
(existingRow) => existingRow.id === row.id,
)
if (matchedExistingRowIndex > -1) {
newFieldState.rows[matchedExistingRowIndex] = {
...newFieldState.rows[matchedExistingRowIndex],
if (indexInCurrentState > -1) {
newState[path].rows[indexInCurrentState] = {
...currentState[path].rows[indexInCurrentState],
...row,
}
}
})
}
/**
* Handle adding all the remaining props that should be updated in the local form state from the server form state
*/
serverPropsToAccept.forEach((propFromServer) => {
if (!dequal(incomingState[path]?.[propFromServer], newFieldState[propFromServer])) {
changed = true
fieldChanged = true
if (newFieldState?.serverPropsToIgnore?.includes(propFromServer)) {
// Remove the ignored prop for the next request
newFieldState.serverPropsToIgnore = newFieldState.serverPropsToIgnore.filter(
(prop) => prop !== propFromServer,
)
// if no keys left, remove the entire object
if (!newFieldState.serverPropsToIgnore.length) {
delete newFieldState.serverPropsToIgnore
// If `valid` is `undefined`, mark it as `true`
if (incomingField.valid !== false) {
newState[path].valid = true
}
return
// If `passesCondition` is `undefined`, mark it as `true`
if (incomingField.passesCondition !== false) {
newState[path].passesCondition = true
}
if (!(propFromServer in incomingState[path])) {
// Regarding excluding the customComponents prop from being deleted: the incoming state might not have been rendered, as rendering components for every form onchange is expensive.
// Thus, we simply re-use the initial render state
if (propFromServer !== 'customComponents') {
delete newFieldState[propFromServer]
}
} else {
newFieldState[propFromServer as any] = incomingState[path][propFromServer]
}
}
})
if (newFieldState.valid !== false) {
newFieldState.valid = true
// Strip away the `addedByServer` property from the client
// This will prevent it from being passed back to the server
delete newState[path].addedByServer
}
if (newFieldState.passesCondition !== false) {
newFieldState.passesCondition = true
}
// Conditions don't work if we don't memcopy the new state, as the object references would otherwise be the same
newState[path] = fieldChanged ? { ...newFieldState } : newFieldState
}
// Now loop over values that are part of incoming state but not part of existing state, and add them to the new state.
// This can happen if a new array row was added. In our local state, we simply add out stubbed `array` and `array.[index].id` entries to the local form state.
// However, all other array sub-fields are not added to the local state - those will be added by the server and may be incoming here.
for (const [path, field] of Object.entries(incomingState)) {
if (!existingState[path]) {
changed = true
newState[path] = field
}
}
}
return { changed, newState }
// Return the original object reference if the state is unchanged
// This will avoid unnecessary re-renders and dependency updates
return dequal(newState, currentState) ? currentState : newState
}

8
packages/ui/src/forms/Form/types.ts
View File

@@ -150,6 +150,13 @@ export type ADD_ROW = {
type: 'ADD_ROW'
}
export type MERGE_SERVER_STATE = {
acceptValues?: boolean
prevStateRef: React.RefObject<FormState>
serverState: FormState
type: 'MERGE_SERVER_STATE'
}
export type REPLACE_ROW = {
blockType?: string
path: string
@@ -192,6 +199,7 @@ export type FieldAction =
| ADD_ROW
| ADD_SERVER_ERRORS
| DUPLICATE_ROW
| MERGE_SERVER_STATE
| MODIFY_CONDITION
| MOVE_ROW
| REMOVE

27
packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts
View File

@@ -152,20 +152,31 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
)
}
const lastRenderedPath = previousFormState?.[path]?.lastRenderedPath
let fieldPermissions: SanitizedFieldPermissions = true
const fieldState: FieldState = {}
const lastRenderedPath = previousFormState?.[path]?.lastRenderedPath
// Append only if true to avoid sending '$undefined' through the network
if (lastRenderedPath) {
fieldState.lastRenderedPath = lastRenderedPath
}
// If we're rendering all fields, no need to flag this as added by server
const addedByServer = !renderAllFields && !previousFormState?.[path]
// Append only if true to avoid sending '$undefined' through the network
if (addedByServer) {
fieldState.addedByServer = true
}
// Append only if true to avoid sending '$undefined' through the network
if (passesCondition === false) {
fieldState.passesCondition = false
}
// Append only if true to avoid sending '$undefined' through the network
if (includeSchema) {
fieldState.fieldSchema = field
}
@@ -719,7 +730,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
}
}
} else if (fieldHasSubFields(field) && !fieldAffectsData(field)) {
// Handle field types that do not use names (row, etc)
// Handle field types that do not use names (row, collapsible, etc)
if (!filter || filter(args)) {
state[path] = {
@@ -871,8 +882,6 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
}
if (renderFieldFn && !fieldIsHiddenOrDisabled(field)) {
const fieldState = state[path]
const fieldConfig = fieldSchemaMap.get(schemaPath)
if (!fieldConfig && !mockRSCs) {
@@ -883,12 +892,16 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
}
}
if (!fieldState) {
if (!state[path]) {
// Some fields (ie `Tab`) do not live in form state
// therefore we cannot attach customComponents to them
return
}
if (addedByServer) {
state[path].addedByServer = addedByServer
}
renderFieldFn({
id,
clientFieldSchemaMap,
@@ -896,7 +909,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
data: fullData,
fieldConfig: fieldConfig as Field,
fieldSchemaMap,
fieldState,
fieldState: state[path],
formState: state,
indexPath,
lastRenderedPath,

1
packages/ui/src/forms/fieldSchemasToFormState/index.tsx
View File

@@ -5,6 +5,7 @@ import type {
DocumentPreferences,
Field,
FieldSchemaMap,
FieldState,
FormState,
FormStateWithoutComponents,
PayloadRequest,

1
packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts
View File

@@ -5,6 +5,7 @@ import type {
DocumentPreferences,
Field as FieldSchema,
FieldSchemaMap,
FieldState,
FormState,
FormStateWithoutComponents,
PayloadRequest,

2
packages/ui/src/forms/fieldSchemasToFormState/renderField.tsx
View File

@@ -200,7 +200,7 @@ export const renderField: RenderFieldMethod = ({
}
/**
* Set the lastRenderedPath equal to the new path of the field
* Set the `lastRenderedPath` equal to the new path of the field, this will prevent it from being rendered again
*/
fieldState.lastRenderedPath = path

19
packages/ui/src/utilities/arraysHaveSameStrings.ts
View File

@@ -1,19 +0,0 @@
export function arraysHaveSameStrings(arr1: string[], arr2: string[]): boolean {
// Step 1: Check if arrays have the same length
if (arr1.length !== arr2.length) {
return false
}
// Step 2: Sort both arrays
const sortedArr1 = arr1.slice().sort()
const sortedArr2 = arr2.slice().sort()
// Step 3: Compare each element
for (let i = 0; i < sortedArr1.length; i++) {
if (sortedArr1[i] !== sortedArr2[i]) {
return false
}
}
return true
}

6
test/fields/collections/Array/e2e.spec.ts
View File

@@ -75,12 +75,6 @@ describe('Array', () => {
await expect(page.locator('#field-readOnly .array-field__add-row')).toBeHidden()
})
test('should have defaultValue', async () => {
await page.goto(url.create)
const field = page.locator('#field-readOnly__0__text')
await expect(field).toHaveValue('defaultValue')
})
test('should render RowLabel using a component', async () => {
const label = 'custom row label as component'
await page.goto(url.create)

2
test/fields/collections/Blocks/e2e.spec.ts
View File

@@ -214,7 +214,7 @@ describe('Block fields', () => {
).toBeVisible()
})
test.skip('should add different blocks with similar field configs', async () => {
test('should add different blocks with similar field configs', async () => {
await page.goto(url.create)
await addBlock({

8
test/form-state/collections/Posts/TextField.tsx Normal file
View File

@@ -0,0 +1,8 @@
'use client'
import type { TextFieldClientComponent } from 'payload'
import { TextField } from '@payloadcms/ui'
export const CustomTextField: TextFieldClientComponent = (props) => {
return <TextField {...props} />
}

13
test/form-state/collections/Posts/index.ts
View File

@@ -1,7 +1,5 @@
import type { CollectionConfig } from 'payload'
import { lexicalEditor } from '@payloadcms/richtext-lexical'
export const postsSlug = 'posts'
export const PostsCollection: CollectionConfig = {
@@ -76,9 +74,14 @@ export const PostsCollection: CollectionConfig = {
},
fields: [
{
name: 'richText',
type: 'richText',
editor: lexicalEditor(),
name: 'customTextField',
type: 'text',
defaultValue: 'This is a default value',
admin: {
components: {
Field: './collections/Posts/TextField.js#CustomTextField',
},
},
},
],
},

147
test/form-state/e2e.spec.ts
View File

@@ -1,4 +1,4 @@
import type { BrowserContext, Page } from '@playwright/test'
import type { BrowserContext, CDPSession, Page } from '@playwright/test'
import type { PayloadTestSDK } from 'helpers/sdk/index.js'
import type { FormState } from 'payload'
@@ -23,6 +23,8 @@ import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
import { TEST_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
import { postsSlug } from './collections/Posts/index.js'
const { describe, beforeEach, afterEach } = test
const filename = fileURLToPath(import.meta.url)
const dirname = path.dirname(filename)
@@ -45,6 +47,7 @@ test.describe('Form State', () => {
initPageConsoleErrorCatch(page)
await ensureCompilationIsDone({ page, serverURL })
})
test.beforeEach(async () => {
// await throttleTest({ page, context, delay: 'Fast 3G' })
})
@@ -159,7 +162,8 @@ test.describe('Form State', () => {
url: postsUrl.create,
expect: (body) =>
Boolean(
body?.[0]?.args?.formState?.['array'] && body[0].args.formState['array'].lastRenderedPath,
body?.[0]?.args?.formState?.['array'] &&
body[0].args.formState['array'].lastRenderedPath === 'array',
),
})
@@ -171,15 +175,16 @@ test.describe('Form State', () => {
)
// The `array` itself SHOULD still have a `lastRenderedPath`
// The rich text field in the first row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the first request
// The custom text field in the first row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the first request
await assertRequestBody<{ args: { formState: FormState } }[]>(page, {
action: async () => await page.locator('#field-array .array-field__add-row').click(),
url: postsUrl.create,
expect: (body) =>
Boolean(
body?.[0]?.args?.formState?.['array'] &&
body[0].args.formState['array'].lastRenderedPath &&
body[0].args.formState['array.0.richText']?.lastRenderedPath,
body[0].args.formState['array'].lastRenderedPath === 'array' &&
body[0].args.formState['array.0.customTextField']?.lastRenderedPath ===
'array.0.customTextField',
),
})
@@ -191,8 +196,8 @@ test.describe('Form State', () => {
)
// The `array` itself SHOULD still have a `lastRenderedPath`
// The rich text field in the first row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the first request
// The rich text field in the second row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the second request
// The custom text field in the first row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the first request
// The custom text field in the second row SHOULD ALSO have a `lastRenderedPath` bc it was rendered in the second request
await assertRequestBody<{ args: { formState: FormState } }[]>(page, {
action: async () => await page.locator('#field-array .array-field__add-row').click(),
url: postsUrl.create,
@@ -200,38 +205,38 @@ test.describe('Form State', () => {
Boolean(
body?.[0]?.args?.formState?.['array'] &&
body[0].args.formState['array'].lastRenderedPath &&
body[0].args.formState['array.0.richText']?.lastRenderedPath &&
body[0].args.formState['array.1.richText']?.lastRenderedPath,
body[0].args.formState['array.0.customTextField']?.lastRenderedPath ===
'array.0.customTextField' &&
body[0].args.formState['array.1.customTextField']?.lastRenderedPath ===
'array.1.customTextField',
),
})
})
test('should queue onChange functions', async () => {
test('new rows should contain default values', async () => {
await page.goto(postsUrl.create)
await page.locator('#field-array .array-field__add-row').click()
await expect(
page.locator('#field-array #array-row-0 #field-array__0__customTextField'),
).toHaveValue('This is a default value')
})
describe('Throttled tests', () => {
let cdpSession: CDPSession
beforeEach(async () => {
await page.goto(postsUrl.create)
const field = page.locator('#field-title')
await field.fill('Test')
// only throttle test after initial load to avoid timeouts
const cdpSession = await throttleTest({
cdpSession = await throttleTest({
page,
context,
delay: 'Slow 3G',
})
})
await assertNetworkRequests(
page,
postsUrl.create,
async () => {
await field.fill('')
// Need to type into a _slower_ than the debounce rate (250ms), but _faster_ than the network request
await field.pressSequentially('Some text to type', { delay: 275 })
},
{
allowedNumberOfRequests: 2,
timeout: 10000, // watch network for 10 seconds to allow requests to build up
},
)
afterEach(async () => {
await cdpSession.send('Network.emulateNetworkConditions', {
offline: false,
latency: 0,
@@ -243,17 +248,6 @@ test.describe('Form State', () => {
})
test('optimistic rows should not disappear between pending network requests', async () => {
await page.goto(postsUrl.create)
const field = page.locator('#field-title')
await field.fill('Test')
// only throttle test after initial load to avoid timeouts
const cdpSession = await throttleTest({
page,
context,
delay: 'Slow 3G',
})
let requestCount = 0
// increment the response count for form state requests
@@ -266,6 +260,7 @@ test.describe('Form State', () => {
// Add the first row and expect an optimistic loading state
await page.locator('#field-array .array-field__add-row').click()
await expect(page.locator('#field-array #array-row-0')).toBeVisible()
// use waitForSelector because the shimmer effect is not always visible
// eslint-disable-next-line playwright/no-wait-for-selector
await page.waitForSelector('#field-array #array-row-0 .shimmer-effect')
@@ -276,6 +271,7 @@ test.describe('Form State', () => {
// Before the first request comes back, add the second row and expect an optimistic loading state
await page.locator('#field-array .array-field__add-row').click()
await expect(page.locator('#field-array #array-row-1')).toBeVisible()
// use waitForSelector because the shimmer effect is not always visible
// eslint-disable-next-line playwright/no-wait-for-selector
await page.waitForSelector('#field-array #array-row-0 .shimmer-effect')
@@ -297,33 +293,31 @@ test.describe('Form State', () => {
await route.abort()
return
}
await route.continue()
})
await assertElementStaysVisible(page, '#field-array #array-row-1')
await cdpSession.send('Network.emulateNetworkConditions', {
offline: false,
latency: 0,
downloadThroughput: -1,
uploadThroughput: -1,
})
await cdpSession.detach()
test('should queue onChange functions', async () => {
const field = page.locator('#field-title')
await assertNetworkRequests(
page,
postsUrl.create,
async () => {
await field.fill('')
// Need to type into a _slower_ than the debounce rate (250ms), but _faster_ than the network request
await field.pressSequentially('Some text to type', { delay: 275 })
},
{
allowedNumberOfRequests: 2,
timeout: 10000, // watch network for 10 seconds to allow requests to build up
},
)
})
test('should not cause nested custom components to disappear when adding a row then editing a field', async () => {
await page.goto(postsUrl.create)
const field = page.locator('#field-title')
await field.fill('Test')
const cdpSession = await throttleTest({
page,
context,
delay: 'Slow 3G',
})
await assertNetworkRequests(
page,
postsUrl.create,
@@ -333,43 +327,26 @@ test.describe('Form State', () => {
// use `waitForSelector` to ensure the element doesn't appear and then disappear
// eslint-disable-next-line playwright/no-wait-for-selector
await page.waitForSelector('#field-array #array-row-0 .field-type.rich-text-lexical', {
await page.waitForSelector('#field-array #array-row-0 #field-array__0__customTextField', {
timeout: TEST_TIMEOUT,
})
await expect(
page.locator('#field-array #array-row-0 .field-type.rich-text-lexical'),
page.locator('#field-array #array-row-0 #field-array__0__customTextField'),
).toBeVisible()
await expect(page.locator('#field-title')).toHaveValue('Test 2')
},
{
allowedNumberOfRequests: 2,
timeout: 10000,
},
)
await cdpSession.send('Network.emulateNetworkConditions', {
offline: false,
latency: 0,
downloadThroughput: -1,
uploadThroughput: -1,
})
await cdpSession.detach()
})
test('should not cause nested custom components to disappear when adding rows back-to-back', async () => {
await page.goto(postsUrl.create)
const field = page.locator('#field-title')
await field.fill('Test')
const cdpSession = await throttleTest({
page,
context,
delay: 'Slow 3G',
})
// Add two rows quickly
// Test that the rich text fields within the rows do not disappear
// Test that the custom text field within the rows do not disappear
await assertNetworkRequests(
page,
postsUrl.create,
@@ -379,22 +356,22 @@ test.describe('Form State', () => {
// use `waitForSelector` to ensure the element doesn't appear and then disappear
// eslint-disable-next-line playwright/no-wait-for-selector
await page.waitForSelector('#field-array #array-row-0 .field-type.rich-text-lexical', {
await page.waitForSelector('#field-array #array-row-0 #field-array__0__customTextField', {
timeout: TEST_TIMEOUT,
})
// use `waitForSelector` to ensure the element doesn't appear and then disappear
// eslint-disable-next-line playwright/no-wait-for-selector
await page.waitForSelector('#field-array #array-row-1 .field-type.rich-text-lexical', {
await page.waitForSelector('#field-array #array-row-1 #field-array__1__customTextField', {
timeout: TEST_TIMEOUT,
})
await expect(
page.locator('#field-array #array-row-0 .field-type.rich-text-lexical'),
page.locator('#field-array #array-row-0 #field-array__0__customTextField'),
).toBeVisible()
await expect(
page.locator('#field-array #array-row-1 .field-type.rich-text-lexical'),
page.locator('#field-array #array-row-1 #field-array__1__customTextField'),
).toBeVisible()
},
{
@@ -402,15 +379,7 @@ test.describe('Form State', () => {
timeout: 10000,
},
)
await cdpSession.send('Network.emulateNetworkConditions', {
offline: false,
latency: 0,
downloadThroughput: -1,
uploadThroughput: -1,
})
await cdpSession.detach()
})
})

334
test/form-state/int.spec.ts
View File

@@ -14,7 +14,6 @@ import { postsSlug } from './collections/Posts/index.js'
import { mergeServerFormState } from '../../packages/ui/src/forms/Form/mergeServerFormState.js'
let payload: Payload
let token: string
let restClient: NextRESTClient
let user: User
@@ -35,7 +34,6 @@ describe('Form State', () => {
})
.then((res) => res.json())
token = data.token
user = data.user
})
@@ -135,6 +133,7 @@ describe('Form State', () => {
value: postData.title,
initialValue: postData.title,
lastRenderedPath: 'title',
addedByServer: true,
},
})
})
@@ -170,8 +169,10 @@ describe('Form State', () => {
})
// Ensure that row 1 _DOES_ return with rendered components
expect(stateWithRow?.['array.0.richText']?.lastRenderedPath).toStrictEqual('array.0.richText')
expect(stateWithRow?.['array.0.richText']?.customComponents?.Field).toBeDefined()
expect(stateWithRow?.['array.0.customTextField']?.lastRenderedPath).toStrictEqual(
'array.0.customTextField',
)
expect(stateWithRow?.['array.0.customTextField']?.customComponents?.Field).toBeDefined()
const { state: stateWithTitle } = await buildFormState({
mockRSCs: true,
@@ -191,8 +192,8 @@ describe('Form State', () => {
value: '123',
initialValue: '123',
},
'array.0.richText': {
lastRenderedPath: 'array.0.richText',
'array.0.customTextField': {
lastRenderedPath: 'array.0.customTextField',
},
'array.1.id': {
value: '456',
@@ -211,44 +212,299 @@ describe('Form State', () => {
})
// Ensure that row 1 _DOES NOT_ return with rendered components
expect(stateWithTitle?.['array.0.richText']).toHaveProperty('lastRenderedPath')
expect(stateWithTitle?.['array.0.richText']).not.toHaveProperty('customComponents')
expect(stateWithTitle?.['array.0.customTextField']).toHaveProperty('lastRenderedPath')
expect(stateWithTitle?.['array.0.customTextField']).not.toHaveProperty('customComponents')
// Ensure that row 2 _DOES_ return with rendered components
expect(stateWithTitle?.['array.1.richText']).toHaveProperty('lastRenderedPath')
expect(stateWithTitle?.['array.1.richText']).toHaveProperty('customComponents')
expect(stateWithTitle?.['array.1.richText']?.customComponents?.Field).toBeDefined()
expect(stateWithTitle?.['array.1.customTextField']).toHaveProperty('lastRenderedPath')
expect(stateWithTitle?.['array.1.customTextField']).toHaveProperty('customComponents')
expect(stateWithTitle?.['array.1.customTextField']?.customComponents?.Field).toBeDefined()
})
it('should merge array rows without losing current state', () => {
it('should add `addedByServer` flag to fields that originate on the server', async () => {
const req = await createLocalReq({ user }, payload)
const postData = await payload.create({
collection: postsSlug,
data: {
title: 'Test Post',
},
})
const { state } = await buildFormState({
mockRSCs: true,
id: postData.id,
collectionSlug: postsSlug,
data: postData,
docPermissions: undefined,
docPreferences: {
fields: {},
},
documentFormState: undefined,
operation: 'update',
renderAllFields: false,
req,
schemaPath: postsSlug,
})
expect(state.title?.addedByServer).toBe(true)
// Ensure that `addedByServer` is removed after being received by the client
const newState = mergeServerFormState({
currentState: state,
incomingState: state,
})
expect(newState.title?.addedByServer).toBeUndefined()
})
it('should not omit value and initialValue from fields added by the server', () => {
const currentState: FormState = {
array: {
rows: [
{
id: '1',
},
],
},
}
const serverState: FormState = {
array: {
rows: [
{
id: '1',
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
addedByServer: true,
},
}
const newState = mergeServerFormState({
currentState,
incomingState: serverState,
})
expect(newState['array.0.customTextField']).toStrictEqual({
passesCondition: true,
valid: true,
value: 'Test',
initialValue: 'Test',
})
})
it('should merge array rows without losing rows added to local state', () => {
const currentState: FormState = {
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
},
{
id: '2',
isLoading: true,
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
'array.1.id': {
value: '2',
initialValue: '2',
},
}
const incomingState: FormState = {
const serverState: FormState = {
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.text',
lastRenderedPath: 'array.0.customTextField',
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
addedByServer: true,
},
}
const { newState } = mergeServerFormState({
existingState: currentState,
incomingState,
const newState = mergeServerFormState({
currentState,
incomingState: serverState,
})
// Row 2 should still exist
expect(newState).toStrictEqual({
array: {
passesCondition: true,
valid: true,
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
},
{
id: '2',
isLoading: true,
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
passesCondition: true,
valid: true,
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
passesCondition: true,
valid: true,
},
'array.1.id': {
value: '2',
initialValue: '2',
},
})
})
it('should merge array rows without bringing back rows deleted from local state', () => {
const currentState: FormState = {
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
}
const serverState: FormState = {
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
},
{
id: '2',
lastRenderedPath: 'array.1.customTextField',
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
addedByServer: true,
},
'array.1.id': {
value: '2',
initialValue: '2',
},
'array.1.customTextField': {
value: 'Test',
initialValue: 'Test',
},
}
const newState = mergeServerFormState({
currentState,
incomingState: serverState,
})
// Row 2 should not exist
expect(newState).toStrictEqual({
array: {
passesCondition: true,
valid: true,
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
passesCondition: true,
valid: true,
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
passesCondition: true,
valid: true,
},
})
})
it('should merge new fields returned from the server that do not yet exist in local state', () => {
const currentState: FormState = {
array: {
rows: [
{
id: '1',
isLoading: true,
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
}
const serverState: FormState = {
array: {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.customTextField',
isLoading: false,
},
],
},
'array.0.id': {
value: '1',
initialValue: '1',
},
'array.0.customTextField': {
value: 'Test',
initialValue: 'Test',
addedByServer: true,
},
}
const newState = mergeServerFormState({
currentState,
incomingState: serverState,
})
expect(newState).toStrictEqual({
@@ -258,14 +514,48 @@ describe('Form State', () => {
rows: [
{
id: '1',
lastRenderedPath: 'array.0.text',
},
{
id: '2',
isLoading: true,
lastRenderedPath: 'array.0.customTextField',
isLoading: false,
},
],
},
'array.0.id': {
passesCondition: true,
valid: true,
value: '1',
initialValue: '1',
},
'array.0.customTextField': {
passesCondition: true,
valid: true,
value: 'Test',
initialValue: 'Test',
},
})
})
it('should return the same object reference when only modifying a value', () => {
const currentState = {
title: {
value: 'Test Post',
initialValue: 'Test Post',
valid: true,
passesCondition: true,
},
}
const newState = mergeServerFormState({
currentState,
incomingState: {
title: {
value: 'Test Post (modified)',
initialValue: 'Test Post',
valid: true,
passesCondition: true,
},
},
})
expect(newState === currentState).toBe(true)
})
})

18
test/form-state/payload-types.ts
View File

@@ -143,21 +143,7 @@ export interface Post {
| null;
array?:
| {
richText?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
customTextField?: string | null;
id?: string | null;
}[]
| null;
@@ -267,7 +253,7 @@ export interface PostsSelect<T extends boolean = true> {
array?:
| T
| {
richText?: T;
customTextField?: T;
id?: T;
};
updatedAt?: T;

2
tsconfig.base.json
View File

@@ -31,7 +31,7 @@
}
],
"paths": {
"@payload-config": ["./test/form-state/config.ts"],
"@payload-config": ["./test/hooks/config.ts"],
"@payloadcms/admin-bar": ["./packages/admin-bar/src"],
"@payloadcms/live-preview": ["./packages/live-preview/src"],
"@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"],