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

Merge pull request #5491 from payloadcms/temp24

Browse Source 
fix: form-state issues
This commit is contained in:
Alessio Gravili
2024-03-27 16:28:26 -04:00
committed by GitHub
parent 0f0da809da 8deb19e3ac
commit c16b869fea
19 changed files with 139 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.github/workflows/main.yml vendored
View File

@@ -254,12 +254,7 @@ jobs:
run: pnpm exec playwright install
- name: E2E Tests
uses: nick-fields/retry@v3
with:
retry_on: error
max_attempts: 2
timeout_minutes: 15
command: pnpm test:e2e ${{ matrix.suite }}
run: pnpm test:e2e ${{ matrix.suite }}
- uses: actions/upload-artifact@v4
if: always()

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

@@ -8,7 +8,7 @@ export type Data = {
export type Row = {
blockType?: string
collapsed?: boolean
errorPaths?: Set<string>
errorPaths?: string[]
id: string
}
@@ -19,7 +19,7 @@ export type FilterOptionsResult = {
export type FormField = {
disableFormData?: boolean
errorMessage?: string
errorPaths?: Set<string>
errorPaths?: string[]
fieldSchema?: Field
filterOptions?: FilterOptionsResult
initialValue: unknown

6
packages/payload/src/fields/baseFields/baseIDField.ts
View File

@@ -12,10 +12,8 @@ export const baseIDField: Field = {
name: 'id',
type: 'text',
admin: {
disabled: true,
},
hooks: {
beforeChange: [generateID],
hidden: true,
},
defaultValue: generateID,
label: 'ID',
}

2
packages/ui/src/fields/Array/ArrayRow.tsx
View File

@@ -70,7 +70,7 @@ export const ArrayRow: React.FC<ArrayRowProps> = ({
'0',
)}`
const errorCount = row.errorPaths?.size
const errorCount = row.errorPaths?.length
const fieldHasErrors = errorCount > 0 && hasSubmitted
const classNames = [

2
packages/ui/src/fields/Array/index.tsx
View File

@@ -181,7 +181,7 @@ export const _ArrayField: React.FC<ArrayFieldProps> = (props) => {
const hasMaxRows = maxRows && rows.length >= maxRows
const fieldErrorCount =
rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0) + (valid ? 0 : 1)
rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0) + (valid ? 0 : 1)
const fieldHasErrors = submitted && fieldErrorCount > 0

2
packages/ui/src/fields/Blocks/BlockRow.tsx
View File

@@ -67,7 +67,7 @@ export const BlockRow: React.FC<BlockFieldProps> = ({
const { i18n } = useTranslation()
const hasSubmitted = useFormSubmitted()
const errorCount = row.errorPaths?.size
const errorCount = row.errorPaths?.length
const fieldHasErrors = errorCount > 0 && hasSubmitted
const classNames = [

2
packages/ui/src/fields/Blocks/index.tsx
View File

@@ -192,7 +192,7 @@ const _BlocksField: React.FC<BlocksFieldProps> = (props) => {
const hasMaxRows = maxRows && rows.length >= maxRows
const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.size || 0), 0)
const fieldErrorCount = rows.reduce((total, row) => total + (row?.errorPaths?.length || 0), 0)
const fieldHasErrors = submitted && fieldErrorCount + (valid ? 0 : 1) > 0
const showMinRows = rows.length < minRows || (required && rows.length === 0)

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

@@ -17,14 +17,13 @@ const ObjectId = (ObjectIdImport.default ||
export function fieldReducer(state: FormState, action: FieldAction): FormState {
switch (action.type) {
case 'REPLACE_STATE': {
const newState = {}
if (action.optimize !== false) {
// Only update fields that have changed
// by comparing old value / initialValue to new
// ..
// This is a performance enhancement for saving
// large documents with hundreds of fields
const newState = {}
Object.entries(action.state).forEach(([path, field]) => {
const oldField = state[path]
@@ -36,12 +35,10 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
newState[path] = oldField
}
})
} else {
// If we're not optimizing, just set the state to the new state
return action.state
return newState
}
return newState
// If we're not optimizing, just set the state to the new state
return action.state
}
case 'REMOVE': {
@@ -114,7 +111,7 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
const withNewRow = [...(state[path]?.rows || [])]
const newRow: Row = {
id: new ObjectId().toHexString(),
id: (subFieldState?.id.value as string) || new ObjectId().toHexString(),
blockType: blockType || undefined,
collapsed: false,
}

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

@@ -5,7 +5,7 @@ import type { FormState } from 'payload/types'
import isDeepEqual from 'deep-equal'
import { useRouter } from 'next/navigation.js'
import { serialize } from 'object-to-formdata'
import { wait } from 'payload/utilities'
import { deepCopyObject, wait } from 'payload/utilities'
import QueryString from 'qs'
import React, { useCallback, useEffect, useReducer, useRef, useState } from 'react'
import { toast } from 'react-toastify'
@@ -530,7 +530,7 @@ export const Form: React.FC<FormProps> = (props) => {
() => {
const executeOnChange = async () => {
if (Array.isArray(onChange)) {
let revalidatedFormState: FormState = fields
let revalidatedFormState: FormState = contextRef.current.fields
for (const onChangeFn of onChange) {
revalidatedFormState = await onChangeFn({
@@ -538,10 +538,17 @@ export const Form: React.FC<FormProps> = (props) => {
})
}
const { changed, newState } = mergeServerFormState(fields || {}, revalidatedFormState)
const { changed, newState } = mergeServerFormState(
contextRef.current.fields || {},
revalidatedFormState,
)
if (changed) {
dispatchFields({ type: 'REPLACE_STATE', optimize: false, state: newState })
dispatchFields({
type: 'REPLACE_STATE',
optimize: false,
state: newState,
})
}
}
}
@@ -549,7 +556,9 @@ export const Form: React.FC<FormProps> = (props) => {
if (modified) void executeOnChange()
},
150,
[fields, dispatchFields, onChange],
// Make sure we trigger this whenever modified changes (not just when `fields` changes), otherwise we will miss merging server form state for the first form update/onChange. Here's why:
// `fields` updates before `modified`, because setModified is in a setTimeout. So on the first change, modified is false, so we don't trigger the effect even though we should.
[contextRef.current.fields, dispatchFields, onChange, modified],
)
const actionString =

38
packages/ui/src/forms/Form/mergeErrorPaths.ts Normal file
View File

@@ -0,0 +1,38 @@
import { arraysHaveSameStrings } from '@payloadcms/ui/utilities/arraysHaveSameStrings'
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,
}
}

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

@@ -1,8 +1,8 @@
import type { FormState } from 'payload/types'
import { arraysHaveSameStrings } from '../../utilities/arraysHaveSameStrings.js'
import { mergeErrorPaths } from './mergeErrorPaths.js'
const propsToCheck = ['passesCondition', 'valid', 'errorMessage']
const serverPropsToAccept = ['passesCondition', 'valid', 'errorMessage']
/**
* Merges certain properties from the server state into the client state. These do not include values,
@@ -12,44 +12,70 @@ const propsToCheck = ['passesCondition', 'valid', 'errorMessage']
* is the thing we want to keep in sync with the server (where it's calculated) on the client.
*/
export const mergeServerFormState = (
oldState: FormState,
newState: FormState,
existingState: FormState,
incomingState: FormState,
): { changed: boolean; newState: FormState } => {
let changed = false
if (oldState) {
Object.entries(newState).forEach(([path, newFieldState]) => {
if (!oldState[path]) return
if (existingState) {
Object.entries(existingState).forEach(([path, newFieldState]) => {
if (!incomingState[path]) return
newFieldState.initialValue = oldState[path].initialValue
newFieldState.value = oldState[path].value
/**
* Handle error paths
*/
const oldErrorPaths: string[] = []
const newErrorPaths: string[] = []
if (oldState[path].errorPaths instanceof Set) {
oldState[path].errorPaths.forEach((path) => oldErrorPaths.push(path))
const errorPathsResult = mergeErrorPaths(
newFieldState.errorPaths,
incomingState[path].errorPaths as unknown as string[],
)
if (errorPathsResult.result) {
if (errorPathsResult.changed) {
changed = errorPathsResult.changed
}
newFieldState.errorPaths = errorPathsResult.result
}
if (
newFieldState.errorPaths &&
!Array.isArray(newFieldState.errorPaths) &&
typeof newFieldState.errorPaths
) {
Object.values(newFieldState.errorPaths).forEach((path) => newErrorPaths.push(path))
}
if (!arraysHaveSameStrings(oldErrorPaths, newErrorPaths)) {
changed = true
}
propsToCheck.forEach((prop) => {
if (newFieldState[prop] != oldState[path][prop]) {
/**
* Handle the rest which is in serverPropsToAccept
*/
serverPropsToAccept.forEach((prop) => {
if (incomingState[path]?.[prop] !== newFieldState[prop]) {
changed = true
if (!(prop in incomingState[path])) {
delete newFieldState[prop]
} else {
newFieldState[prop] = incomingState[path][prop]
}
}
})
/**
* Handle rows
*/
if (Array.isArray(newFieldState.rows) && newFieldState?.rows.length) {
let i = 0
for (const row of newFieldState.rows) {
const incomingRow = incomingState[path]?.rows?.[i]
if (incomingRow) {
const errorPathsRowResult = mergeErrorPaths(
row.errorPaths,
incomingRow.errorPaths as unknown as string[],
)
if (errorPathsRowResult.result) {
if (errorPathsResult.changed) {
changed = true
}
incomingRow.errorPaths = errorPathsRowResult.result
}
}
i++
}
}
})
}
return { changed, newState }
// Conditions don't work if we don't memcopy the new state, as the object references would otherwise be the same
return { changed, newState: { ...existingState } }
}

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

@@ -174,7 +174,7 @@ export type Context = {
disabled: boolean
dispatchFields: Dispatch<FieldAction>
/**
* @deprecated Form context fields may be outdated and should not be relied on. Instead, prefer `useFormFields`.
* Form context fields may be outdated and should not be relied on. Instead, prefer `useFormFields`.
*/
fields: FormState
formRef: React.MutableRefObject<HTMLFormElement>

10
packages/ui/src/forms/buildStateFromSchema/addFieldStatePromise.ts
View File

@@ -23,7 +23,7 @@ export type AddFieldStatePromiseArgs = {
*/
anyParentLocalized?: boolean
data: Data
errorPaths: Set<string>
errorPaths: string[]
field: NonPresentationalField
/**
* You can use this to filter down to only `localized` fields that require translation (type: text, textarea, etc.). Another plugin might want to look for only `point` type fields to do some GIS function. With the filter function you can go in like a surgeon.
@@ -74,7 +74,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
id,
anyParentLocalized = false,
data,
errorPaths: parentErrorPaths,
errorPaths: parentErrorPaths = [],
field,
filter,
forceFullValue = false,
@@ -95,7 +95,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
const validate = operation === 'update' ? field.validate : undefined
const fieldState: FormField = {
errorPaths: new Set(),
errorPaths: [],
fieldSchema: includeSchema ? field : undefined,
initialValue: undefined,
passesCondition,
@@ -144,7 +144,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
fieldState.valid = false
// TODO: this is unpredictable, need to figure out why
// It will sometimes lead to inconsistencies across re-renders
parentErrorPaths.add(`${path}${field.name}`)
if (!parentErrorPaths.includes(`${path}${field.name}`)) {
parentErrorPaths.push(`${path}${field.name}`)
}
} else {
fieldState.valid = true
}

2
packages/ui/src/forms/buildStateFromSchema/index.tsx
View File

@@ -40,7 +40,7 @@ export const buildStateFromSchema = async (args: Args): Promise<FormState> => {
await iterateFields({
id,
data: fullData,
errorPaths: new Set(),
errorPaths: [],
fields: fieldSchema,
fullData,
operation,

2
packages/ui/src/forms/buildStateFromSchema/iterateFields.ts
View File

@@ -12,7 +12,7 @@ type Args = {
*/
anyParentLocalized?: boolean
data: Data
errorPaths: Set<string>
errorPaths: string[]
fields: FieldSchema[]
filter?: (args: AddFieldStatePromiseArgs) => boolean
/**

2
packages/ui/src/providers/ComponentMap/buildComponentMap/mapFields.tsx
View File

@@ -228,7 +228,6 @@ export const mapFields = (args: {
disabled: field.admin?.disabled,
fieldMap: mapFields({
config,
disableAddingID: true,
fieldSchema: field.fields,
filter,
parentPath: path,
@@ -251,7 +250,6 @@ export const mapFields = (args: {
const blocks = field.blocks.map((block) => {
const blockFieldMap = mapFields({
config,
disableAddingID: true,
fieldSchema: block.fields,
filter,
parentPath: `${path}.${block.slug}`,

5
test/access-control/e2e.spec.ts
View File

@@ -13,6 +13,7 @@ import {
initPageConsoleErrorCatch,
openDocControls,
openNav,
saveDocAndAssert,
} from '../helpers.js'
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
import { initPayloadE2E } from '../helpers/initPayloadE2E.js'
@@ -248,7 +249,7 @@ describe('access control', () => {
await expect(deleteAction).toBeHidden()
await page.locator('#field-approvedForRemoval').check()
await page.locator('#action-save').click()
await saveDocAndAssert(page)
await openDocControls(page)
const deleteAction2 = page.locator('#action-delete')
@@ -272,7 +273,7 @@ describe('access control', () => {
// Allow access to test global.
await page.locator('.checkbox-input:has(#field-test) input').check()
await page.locator('#action-save').click()
await saveDocAndAssert(page)
await openNav(page)

13
test/fields/e2e.spec.ts
View File

@@ -1386,7 +1386,8 @@ describe('fields', () => {
await expect(dateField).toBeVisible()
await dateField.fill('02/07/2023')
await expect(dateField).toHaveValue('02/07/2023')
await page.locator('#action-save').click()
await saveDocAndAssert(page)
const clearButton = page.locator('#field-default .date-time-picker__clear-button')
await expect(clearButton).toBeVisible()
await clearButton.click()
@@ -1409,10 +1410,7 @@ describe('fields', () => {
// enter date in default date field
await dateField.fill('02/07/2023')
await page.locator('#action-save').click()
// wait for navigation to update route
await expect.poll(() => page.url(), { timeout: 1000 }).not.toContain('create')
await saveDocAndAssert(page)
// get the ID of the doc
const routeSegments = page.url().split('/')
@@ -1473,10 +1471,7 @@ describe('fields', () => {
// enter date in default date field
await dateField.fill('02/07/2023')
await page.locator('#action-save').click()
// wait for navigation to update route
await expect.poll(() => page.url(), { timeout: 1000 }).not.toContain('create')
await saveDocAndAssert(page)
// get the ID of the doc
const routeSegments = page.url().split('/')

4
test/fields/lexical.e2e.spec.ts
View File

@@ -32,7 +32,7 @@ async function navigateToLexicalFields() {
const url: AdminUrlUtil = new AdminUrlUtil(serverURL, 'lexical-fields')
await page.goto(url.list)
const linkToDoc = page.locator('tbody tr:first-child .cell-title a').first()
await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: 45000 })
await expect(() => expect(linkToDoc).toBeTruthy()).toPass({ timeout: POLL_TOPASS_TIMEOUT })
const linkDocHref = await linkToDoc.getAttribute('href')
await linkToDoc.click()
@@ -718,7 +718,7 @@ describe('lexical', () => {
await expect(nestedEditorParagraph).toHaveText('Some text below relationship node 12345')
})
test.skip('should respect row removal in nested array field', async () => {
test('should respect row removal in nested array field', async () => {
await navigateToLexicalFields()
const richTextField = page.locator('.rich-text-lexical').nth(1) // second
await richTextField.scrollIntoViewIfNeeded()