Skip to content

Commit c67ceca

Browse files
authored
perf(ui): do not fetch doc permissions on autosave (#13477)
No need to re-fetch doc permissions during autosave. This will save us from making two additional client-side requests on every autosave interval, on top of the two existing requests needed to autosave and refresh form state. This _does_ mean that the UI will not fully reflect permissions again until you fully save, or until you navigating back, but that has always been the behavior anyway (until #13416). Maybe we can find another solution for this in the future, or otherwise consider this to be expected behavior. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211094073049052
1 parent f382c39 commit c67ceca

File tree

7 files changed

+145
-29
lines changed

7 files changed

+145
-29
lines changed

packages/ui/src/elements/Autosave/index.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { dequal } from 'dequal/lite'
66
import { reduceFieldsToValues, versionDefaults } from 'payload/shared'
77
import React, { useDeferredValue, useEffect, useRef, useState } from 'react'
88

9+
import type { OnSaveContext } from '../../views/Edit/index.js'
10+
911
import {
1012
useAllFormFields,
1113
useForm,
@@ -45,7 +47,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
4547

4648
const {
4749
docConfig,
48-
incrementVersionCount,
4950
lastUpdateTime,
5051
mostRecentVersionIsAutosaved,
5152
setMostRecentVersionIsAutosaved,
@@ -149,15 +150,14 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
149150
submitted && !valid && versionsConfig?.drafts && versionsConfig?.drafts?.validate
150151

151152
if (!skipSubmission && modifiedRef.current && url) {
152-
const result = await submit<{
153-
incrementVersionCount: boolean
154-
}>({
153+
const result = await submit<any, OnSaveContext>({
155154
acceptValues: {
156155
overrideLocalChanges: false,
157156
},
158157
action: url,
159158
context: {
160-
incrementVersionCount: false,
159+
getDocPermissions: false,
160+
incrementVersionCount: !mostRecentVersionIsAutosaved,
161161
},
162162
disableFormWhileProcessing: false,
163163
disableSuccessStatus: true,
@@ -169,7 +169,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
169169
})
170170

171171
if (result && result?.res?.ok && !mostRecentVersionIsAutosaved) {
172-
incrementVersionCount()
173172
setMostRecentVersionIsAutosaved(true)
174173
setUnpublishedVersionCount((prev) => prev + 1)
175174
}

packages/ui/src/forms/Form/types.ts

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@ export type Preferences = {
1616
[key: string]: unknown
1717
}
1818

19+
export type FormOnSuccess<T = unknown, C = Record<string, unknown>> = (
20+
json: T,
21+
options?: {
22+
/**
23+
* Arbitrary context passed to the onSuccess callback.
24+
*/
25+
context?: C
26+
/**
27+
* Form state at the time of the request used to retrieve the JSON response.
28+
*/
29+
formState?: FormState
30+
},
31+
) => Promise<FormState | void> | void
32+
1933
export type FormProps = {
2034
beforeSubmit?: ((args: { formState: FormState }) => Promise<FormState>)[]
2135
children?: React.ReactNode
@@ -54,16 +68,7 @@ export type FormProps = {
5468
log?: boolean
5569
onChange?: ((args: { formState: FormState; submitted?: boolean }) => Promise<FormState>)[]
5670
onSubmit?: (fields: FormState, data: Data) => void
57-
onSuccess?: (
58-
json: unknown,
59-
options?: {
60-
/**
61-
* Arbitrary context passed to the onSuccess callback.
62-
*/
63-
context?: Record<string, unknown>
64-
formState?: FormState
65-
},
66-
) => Promise<FormState | void> | void
71+
onSuccess?: FormOnSuccess
6772
redirect?: string
6873
submitted?: boolean
6974
uuid?: string
@@ -79,14 +84,14 @@ export type FormProps = {
7984
}
8085
)
8186

82-
export type SubmitOptions<T = Record<string, unknown>> = {
87+
export type SubmitOptions<C = Record<string, unknown>> = {
8388
acceptValues?: AcceptValues
8489
action?: string
8590
/**
8691
* @experimental - Note: this property is experimental and may change in the future. Use at your own discretion.
8792
* If you want to pass additional data to the onSuccess callback, you can use this context object.
8893
*/
89-
context?: T
94+
context?: C
9095
/**
9196
* When true, will disable the form while it is processing.
9297
* @default true
@@ -108,14 +113,14 @@ export type SubmitOptions<T = Record<string, unknown>> = {
108113

109114
export type DispatchFields = React.Dispatch<any>
110115

111-
export type Submit = <T extends Record<string, unknown>>(
112-
options?: SubmitOptions<T>,
116+
export type Submit = <T extends Response, C extends Record<string, unknown>>(
117+
options?: SubmitOptions<C>,
113118
e?: React.FormEvent<HTMLFormElement>,
114119
) => Promise</**
115120
* @experimental - Note: the `{ res: ... }` return type is experimental and may change in the future. Use at your own discretion.
116121
* Returns the form state and the response from the server.
117122
*/
118-
{ formState?: FormState; res: Response } | void>
123+
{ formState?: FormState; res: T } | void>
119124

120125
export type ValidateForm = () => Promise<boolean>
121126

packages/ui/src/providers/DocumentInfo/types.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import type {
1414

1515
import React from 'react'
1616

17+
import type { GetDocPermissions } from './useGetDocPermissions.js'
18+
1719
export type DocumentInfoProps = {
1820
readonly action?: string
1921
readonly AfterDocument?: React.ReactNode
@@ -57,7 +59,7 @@ export type DocumentInfoContext = {
5759
isLocked: boolean
5860
user: ClientUser | number | string
5961
} | null>
60-
getDocPermissions: (data?: Data) => Promise<void>
62+
getDocPermissions: GetDocPermissions
6163
getDocPreferences: () => Promise<DocumentPreferences>
6264
incrementVersionCount: () => void
6365
isInitializing: boolean

packages/ui/src/providers/DocumentInfo/useGetDocPermissions.tsx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import React from 'react'
66
import { hasSavePermission as getHasSavePermission } from '../../utilities/hasSavePermission.js'
77
import { isEditing as getIsEditing } from '../../utilities/isEditing.js'
88

9+
export type GetDocPermissions = (data?: Data) => Promise<void>
10+
911
export const useGetDocPermissions = ({
1012
id,
1113
api,
@@ -30,7 +32,7 @@ export const useGetDocPermissions = ({
3032
setDocPermissions: React.Dispatch<React.SetStateAction<SanitizedDocumentPermissions>>
3133
setHasPublishPermission: React.Dispatch<React.SetStateAction<boolean>>
3234
setHasSavePermission: React.Dispatch<React.SetStateAction<boolean>>
33-
}) =>
35+
}): GetDocPermissions =>
3436
React.useCallback(
3537
async (data: Data) => {
3638
const params = {
@@ -111,5 +113,18 @@ export const useGetDocPermissions = ({
111113
)
112114
}
113115
},
114-
[serverURL, api, id, permissions, i18n.language, locale, collectionSlug, globalSlug],
116+
[
117+
locale,
118+
id,
119+
collectionSlug,
120+
globalSlug,
121+
serverURL,
122+
api,
123+
i18n.language,
124+
setDocPermissions,
125+
setHasSavePermission,
126+
setHasPublishPermission,
127+
permissions?.collections,
128+
permissions?.globals,
129+
],
115130
)

packages/ui/src/views/Edit/index.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
/* eslint-disable react-compiler/react-compiler -- TODO: fix */
22
'use client'
33

4-
import type { ClientUser, DocumentViewClientProps } from 'payload'
4+
import type { ClientUser, DocumentViewClientProps, FormState } from 'payload'
55

66
import { useRouter, useSearchParams } from 'next/navigation.js'
77
import { formatAdminURL } from 'payload/shared'
88
import React, { Fragment, useCallback, useEffect, useMemo, useRef, useState } from 'react'
99

1010
import type { FormProps } from '../../forms/Form/index.js'
11+
import type { FormOnSuccess } from '../../forms/Form/types.js'
1112
import type { LockedState } from '../../utilities/buildFormState.js'
1213

1314
import { DocumentControls } from '../../elements/DocumentControls/index.js'
@@ -42,6 +43,11 @@ import './index.scss'
4243

4344
const baseClass = 'collection-edit'
4445

46+
export type OnSaveContext = {
47+
getDocPermissions?: boolean
48+
incrementVersionCount?: boolean
49+
}
50+
4551
// This component receives props only on _pages_
4652
// When rendered within a drawer, props are empty
4753
// This is solely to support custom edit views which get server-rendered
@@ -256,13 +262,12 @@ export function DefaultEditView({
256262
user?.id,
257263
])
258264

259-
const onSave = useCallback<FormProps['onSuccess']>(
265+
const onSave: FormOnSuccess<any, OnSaveContext> = useCallback(
260266
async (json, options) => {
261267
const { context, formState } = options || {}
262268

263269
const controller = handleAbortRef(abortOnSaveRef)
264270

265-
// @ts-expect-error can ignore
266271
const document = json?.doc || json?.result
267272

268273
const updatedAt = document?.updatedAt || new Date().toISOString()
@@ -316,7 +321,9 @@ export function DefaultEditView({
316321
resetUploadEdits()
317322
}
318323

319-
await getDocPermissions(json)
324+
if (context?.getDocPermissions !== false) {
325+
await getDocPermissions(json)
326+
}
320327

321328
if (id || globalSlug) {
322329
const docPreferences = await getDocPreferences()

test/form-state/collections/Autosave/index.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ export const AutosavePostsCollection: CollectionConfig = {
1818
hooks: {
1919
beforeChange: [({ data }) => data?.title],
2020
},
21-
label: 'Computed Title',
2221
},
2322
],
2423
versions: {

test/form-state/e2e.spec.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { addArrayRowAsync, removeArrayRow } from 'helpers/e2e/fields/array/index
1010
import { addBlock } from 'helpers/e2e/fields/blocks/index.js'
1111
import { waitForAutoSaveToRunAndComplete } from 'helpers/e2e/waitForAutoSaveToRunAndComplete.js'
1212
import * as path from 'path'
13+
import { wait } from 'payload/shared'
1314
import { fileURLToPath } from 'url'
1415

1516
import type { Config, Post } from './payload-types.js'
@@ -330,6 +331,94 @@ test.describe('Form State', () => {
330331
).toHaveValue('This is a computed value.')
331332
})
332333

334+
test('should fetch new doc permissions after save', async () => {
335+
const doc = await createPost({ title: 'Initial Title' })
336+
await page.goto(postsUrl.edit(doc.id))
337+
const titleField = page.locator('#field-title')
338+
await expect(titleField).toBeEnabled()
339+
340+
await assertNetworkRequests(
341+
page,
342+
`${serverURL}/api/posts/access/${doc.id}`,
343+
async () => {
344+
await titleField.fill('Updated Title')
345+
await wait(500)
346+
await page.click('#action-save', { delay: 100 })
347+
},
348+
{
349+
allowedNumberOfRequests: 2,
350+
minimumNumberOfRequests: 2,
351+
timeout: 3000,
352+
},
353+
)
354+
355+
await assertNetworkRequests(
356+
page,
357+
`${serverURL}/api/posts/access/${doc.id}`,
358+
async () => {
359+
await titleField.fill('Updated Title 2')
360+
await wait(500)
361+
await page.click('#action-save', { delay: 100 })
362+
},
363+
{
364+
minimumNumberOfRequests: 2,
365+
allowedNumberOfRequests: 2,
366+
timeout: 3000,
367+
},
368+
)
369+
})
370+
371+
test('autosave - should not fetch new doc permissions on every autosave', async () => {
372+
const doc = await payload.create({
373+
collection: autosavePostsSlug,
374+
data: {
375+
title: 'Initial Title',
376+
},
377+
})
378+
379+
await page.goto(autosavePostsUrl.edit(doc.id))
380+
const titleField = page.locator('#field-title')
381+
await expect(titleField).toBeEnabled()
382+
383+
await assertNetworkRequests(
384+
page,
385+
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
386+
async () => {
387+
await titleField.fill('Updated Title')
388+
},
389+
{
390+
allowedNumberOfRequests: 0,
391+
timeout: 3000,
392+
},
393+
)
394+
395+
await assertNetworkRequests(
396+
page,
397+
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
398+
async () => {
399+
await titleField.fill('Updated Title Again')
400+
},
401+
{
402+
allowedNumberOfRequests: 0,
403+
timeout: 3000,
404+
},
405+
)
406+
407+
// save manually and ensure the permissions are fetched again
408+
await assertNetworkRequests(
409+
page,
410+
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
411+
async () => {
412+
await page.click('#action-save', { delay: 100 })
413+
},
414+
{
415+
allowedNumberOfRequests: 2,
416+
minimumNumberOfRequests: 2,
417+
timeout: 3000,
418+
},
419+
)
420+
})
421+
333422
test('autosave - should render computed values after autosave', async () => {
334423
await page.goto(autosavePostsUrl.create)
335424
const titleField = page.locator('#field-title')

0 commit comments

Comments
 (0)