Skip to content

Commit f382c39

Browse files
jacobsfletchjacobsfletch
authored
fix: accept computed array and block rows from server form state (#13501)
If you have a beforeChange hook that manipulates arrays or blocks by _adding rows_, the result of that hook will not be reflected in the UI after save or autosave as you might expect. For example, this hook that ensures at least one array row is populated: ```ts { type: 'array', hooks: { beforeChange: [ ({ value }) => !value?.length ? [ // this is an added/computed row if attempt to save with no rows ] : value, ], }, // ... } ``` When you save without any rows, this hook will have automatically computed a row for you and saved it to the database. Form state will not reflect this fact, however, until you refresh or navigate back. This is for two reasons: 1. When merging server form state, we receive the new fields, but do not receive the new rows. This is because the `acceptValues` flag only applies to the `value` property of fields, but should also apply to the `rows` property on `array` and `blocks` fields too. 2. When creating new form state on the server, the newly added rows are not being flagged with `addedByServer`, and so never make it into form state when it is merged in on the client. To do this we need to send the previous form state to the server and set `renderAllFields` to false in order receive this property as expected. Fixed by #13524. Before: https://github.com/user-attachments/assets/3ab07ef5-3afd-456f-a9a8-737909b75016 After: https://github.com/user-attachments/assets/27ad1d83-9313-45a9-b44a-db1e64452a99 --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211094073049042
1 parent fea6742 commit f382c39

File tree

5 files changed

+142
-41
lines changed

5 files changed

+142
-41
lines changed

‎packages/ui/src/forms/Form/mergeServerFormState.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,25 @@ export const mergeServerFormState = ({
9494
* Loop over the incoming rows, if it exists in client side form state, merge in any new properties from the server
9595
* Note: read `currentState` and not `newState` here, as the `rows` property have already been merged above
9696
*/
97-
if (Array.isArray(incomingField.rows) && path in currentState) {
98-
newState[path].rows = [...(currentState[path]?.rows || [])] // shallow copy to avoid mutating the original array
99-
100-
incomingField.rows.forEach((row) => {
101-
const indexInCurrentState = currentState[path].rows?.findIndex(
102-
(existingRow) => existingRow.id === row.id,
103-
)
104-
105-
if (indexInCurrentState > -1) {
106-
newState[path].rows[indexInCurrentState] = {
107-
...currentState[path].rows[indexInCurrentState],
108-
...row,
97+
if (Array.isArray(incomingField.rows)) {
98+
if (acceptValues === true) {
99+
newState[path].rows = incomingField.rows
100+
} else if (path in currentState) {
101+
newState[path].rows = [...(currentState[path]?.rows || [])] // shallow copy to avoid mutating the original array
102+
103+
incomingField.rows.forEach((row) => {
104+
const indexInCurrentState = currentState[path].rows?.findIndex(
105+
(existingRow) => existingRow.id === row.id,
106+
)
107+
108+
if (indexInCurrentState > -1) {
109+
newState[path].rows[indexInCurrentState] = {
110+
...currentState[path].rows[indexInCurrentState],
111+
...row,
112+
}
109113
}
110-
}
111-
})
114+
})
115+
}
112116
}
113117

114118
// If `valid` is `undefined`, mark it as `true`

‎test/form-state/collections/Posts/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,24 @@ export const PostsCollection: CollectionConfig = {
7676
name: 'array',
7777
type: 'array',
7878
admin: {
79+
description: 'If there is no value, a default row will be added by a beforeChange hook',
7980
components: {
8081
RowLabel: './collections/Posts/ArrayRowLabel.js#ArrayRowLabel',
8182
},
8283
},
84+
hooks: {
85+
beforeChange: [
86+
({ value }) =>
87+
!value?.length
88+
? [
89+
{
90+
defaultTextField: 'This is a computed value.',
91+
customTextField: 'This is a computed value.',
92+
},
93+
]
94+
: value,
95+
],
96+
},
8397
fields: [
8498
{
8599
name: 'customTextField',

‎test/form-state/e2e.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,26 @@ test.describe('Form State', () => {
308308
await saveDocAndAssert(page)
309309

310310
await expect(computedTitleField).toHaveValue('Test Title')
311+
312+
// Now test array rows, as their merge logic is different
313+
314+
await page.locator('#field-array #array-row-0').isVisible()
315+
316+
await removeArrayRow(page, { fieldName: 'array' })
317+
318+
await page.locator('#field-array .array-row-0').isHidden()
319+
320+
await saveDocAndAssert(page)
321+
322+
await expect(page.locator('#field-array #array-row-0')).toBeVisible()
323+
324+
await expect(
325+
page.locator('#field-array #array-row-0 #field-array__0__customTextField'),
326+
).toHaveValue('This is a computed value.')
327+
328+
await expect(
329+
page.locator('#field-array #array-row-0 #field-array__0__defaultTextField'),
330+
).toHaveValue('This is a computed value.')
311331
})
312332

313333
test('autosave - should render computed values after autosave', async () => {

‎test/form-state/payload-types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ export interface Post {
144144
}
145145
)[]
146146
| null;
147+
/**
148+
* If there is no value, a default row will be added by a beforeChange hook
149+
*/
147150
array?:
148151
| {
149152
customTextField?: string | null;

‎tsconfig.base.json

Lines changed: 87 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,15 @@
2121
"skipLibCheck": true,
2222
"emitDeclarationOnly": true,
2323
"sourceMap": true,
24-
"lib": ["DOM", "DOM.Iterable", "ES2022"],
25-
"types": ["node", "jest"],
24+
"lib": [
25+
"DOM",
26+
"DOM.Iterable",
27+
"ES2022"
28+
],
29+
"types": [
30+
"node",
31+
"jest"
32+
],
2633
"incremental": true,
2734
"isolatedModules": true,
2835
"plugins": [
@@ -31,36 +38,72 @@
3138
}
3239
],
3340
"paths": {
34-
"@payload-config": ["./test/_community/config.ts"],
35-
"@payloadcms/admin-bar": ["./packages/admin-bar/src"],
36-
"@payloadcms/live-preview": ["./packages/live-preview/src"],
37-
"@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"],
38-
"@payloadcms/live-preview-vue": ["./packages/live-preview-vue/src/index.ts"],
39-
"@payloadcms/ui": ["./packages/ui/src/exports/client/index.ts"],
40-
"@payloadcms/ui/shared": ["./packages/ui/src/exports/shared/index.ts"],
41-
"@payloadcms/ui/rsc": ["./packages/ui/src/exports/rsc/index.ts"],
42-
"@payloadcms/ui/scss": ["./packages/ui/src/scss.scss"],
43-
"@payloadcms/ui/scss/app.scss": ["./packages/ui/src/scss/app.scss"],
44-
"@payloadcms/next/*": ["./packages/next/src/exports/*.ts"],
41+
"@payload-config": [
42+
"./test/form-state/config.ts"
43+
],
44+
"@payloadcms/admin-bar": [
45+
"./packages/admin-bar/src"
46+
],
47+
"@payloadcms/live-preview": [
48+
"./packages/live-preview/src"
49+
],
50+
"@payloadcms/live-preview-react": [
51+
"./packages/live-preview-react/src/index.ts"
52+
],
53+
"@payloadcms/live-preview-vue": [
54+
"./packages/live-preview-vue/src/index.ts"
55+
],
56+
"@payloadcms/ui": [
57+
"./packages/ui/src/exports/client/index.ts"
58+
],
59+
"@payloadcms/ui/shared": [
60+
"./packages/ui/src/exports/shared/index.ts"
61+
],
62+
"@payloadcms/ui/rsc": [
63+
"./packages/ui/src/exports/rsc/index.ts"
64+
],
65+
"@payloadcms/ui/scss": [
66+
"./packages/ui/src/scss.scss"
67+
],
68+
"@payloadcms/ui/scss/app.scss": [
69+
"./packages/ui/src/scss/app.scss"
70+
],
71+
"@payloadcms/next/*": [
72+
"./packages/next/src/exports/*.ts"
73+
],
4574
"@payloadcms/richtext-lexical/client": [
4675
"./packages/richtext-lexical/src/exports/client/index.ts"
4776
],
48-
"@payloadcms/richtext-lexical/rsc": ["./packages/richtext-lexical/src/exports/server/rsc.ts"],
49-
"@payloadcms/richtext-slate/rsc": ["./packages/richtext-slate/src/exports/server/rsc.ts"],
77+
"@payloadcms/richtext-lexical/rsc": [
78+
"./packages/richtext-lexical/src/exports/server/rsc.ts"
79+
],
80+
"@payloadcms/richtext-slate/rsc": [
81+
"./packages/richtext-slate/src/exports/server/rsc.ts"
82+
],
5083
"@payloadcms/richtext-slate/client": [
5184
"./packages/richtext-slate/src/exports/client/index.ts"
5285
],
53-
"@payloadcms/plugin-seo/client": ["./packages/plugin-seo/src/exports/client.ts"],
54-
"@payloadcms/plugin-sentry/client": ["./packages/plugin-sentry/src/exports/client.ts"],
55-
"@payloadcms/plugin-stripe/client": ["./packages/plugin-stripe/src/exports/client.ts"],
56-
"@payloadcms/plugin-search/client": ["./packages/plugin-search/src/exports/client.ts"],
86+
"@payloadcms/plugin-seo/client": [
87+
"./packages/plugin-seo/src/exports/client.ts"
88+
],
89+
"@payloadcms/plugin-sentry/client": [
90+
"./packages/plugin-sentry/src/exports/client.ts"
91+
],
92+
"@payloadcms/plugin-stripe/client": [
93+
"./packages/plugin-stripe/src/exports/client.ts"
94+
],
95+
"@payloadcms/plugin-search/client": [
96+
"./packages/plugin-search/src/exports/client.ts"
97+
],
5798
"@payloadcms/plugin-form-builder/client": [
5899
"./packages/plugin-form-builder/src/exports/client.ts"
59100
],
60101
"@payloadcms/plugin-import-export/rsc": [
61102
"./packages/plugin-import-export/src/exports/rsc.ts"
62103
],
63-
"@payloadcms/plugin-multi-tenant/rsc": ["./packages/plugin-multi-tenant/src/exports/rsc.ts"],
104+
"@payloadcms/plugin-multi-tenant/rsc": [
105+
"./packages/plugin-multi-tenant/src/exports/rsc.ts"
106+
],
64107
"@payloadcms/plugin-multi-tenant/utilities": [
65108
"./packages/plugin-multi-tenant/src/exports/utilities.ts"
66109
],
@@ -70,25 +113,42 @@
70113
"@payloadcms/plugin-multi-tenant/client": [
71114
"./packages/plugin-multi-tenant/src/exports/client.ts"
72115
],
73-
"@payloadcms/plugin-multi-tenant": ["./packages/plugin-multi-tenant/src/index.ts"],
116+
"@payloadcms/plugin-multi-tenant": [
117+
"./packages/plugin-multi-tenant/src/index.ts"
118+
],
74119
"@payloadcms/plugin-multi-tenant/translations/languages/all": [
75120
"./packages/plugin-multi-tenant/src/translations/index.ts"
76121
],
77122
"@payloadcms/plugin-multi-tenant/translations/languages/*": [
78123
"./packages/plugin-multi-tenant/src/translations/languages/*.ts"
79124
],
80-
"@payloadcms/next": ["./packages/next/src/exports/*"],
81-
"@payloadcms/storage-azure/client": ["./packages/storage-azure/src/exports/client.ts"],
82-
"@payloadcms/storage-s3/client": ["./packages/storage-s3/src/exports/client.ts"],
125+
"@payloadcms/next": [
126+
"./packages/next/src/exports/*"
127+
],
128+
"@payloadcms/storage-azure/client": [
129+
"./packages/storage-azure/src/exports/client.ts"
130+
],
131+
"@payloadcms/storage-s3/client": [
132+
"./packages/storage-s3/src/exports/client.ts"
133+
],
83134
"@payloadcms/storage-vercel-blob/client": [
84135
"./packages/storage-vercel-blob/src/exports/client.ts"
85136
],
86-
"@payloadcms/storage-gcs/client": ["./packages/storage-gcs/src/exports/client.ts"],
137+
"@payloadcms/storage-gcs/client": [
138+
"./packages/storage-gcs/src/exports/client.ts"
139+
],
87140
"@payloadcms/storage-uploadthing/client": [
88141
"./packages/storage-uploadthing/src/exports/client.ts"
89142
]
90143
}
91144
},
92-
"include": ["${configDir}/src"],
93-
"exclude": ["${configDir}/dist", "${configDir}/build", "${configDir}/temp", "**/*.spec.ts"]
145+
"include": [
146+
"${configDir}/src"
147+
],
148+
"exclude": [
149+
"${configDir}/dist",
150+
"${configDir}/build",
151+
"${configDir}/temp",
152+
"**/*.spec.ts"
153+
]
94154
}

0 commit comments

Comments
 (0)