From 97cffa51f804842ca259aa578deeb84ab9d454ed Mon Sep 17 00:00:00 2001 From: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:20:17 -0500 Subject: [PATCH] chore: improves abort controller logic for server functions (#9131) ### What? Removes abort controllers that were shared globally inside the server actions provider. ### Why? Constructing them in this way will cause different fetches using the same function to cancel one another accidentally. These are currently causing issues when two components call server functions, even different functions, because the global ref inside was being overwritten and aborting the previous one. ### How? Standardizes how we construct and destroy abort controllers. This PR is focused around creating them to pass into the exposed serverAction provider functions. There are other places where this pattern can be applied. --- .../views/CreateFirstUser/index.client.tsx | 17 +++- .../src/views/LivePreview/index.client.tsx | 37 ++++---- .../blocks/client/component/index.tsx | 40 +++------ .../utilities/fieldsDrawer/DrawerContent.tsx | 36 +++----- .../src/field/elements/link/Button/index.tsx | 1 - .../src/field/elements/link/Element/index.tsx | 1 - .../upload/Element/UploadDrawer/index.tsx | 1 - .../elements/BulkUpload/EditForm/index.tsx | 18 +++- .../elements/DocumentDrawer/DrawerContent.tsx | 82 ++++++++++------- packages/ui/src/elements/EditMany/index.tsx | 21 ++++- packages/ui/src/exports/shared/index.ts | 1 + packages/ui/src/forms/Form/index.tsx | 38 ++------ .../src/providers/ServerFunctions/index.tsx | 90 ++++--------------- packages/ui/src/utilities/abortAndIgnore.ts | 9 ++ packages/ui/src/views/Edit/index.tsx | 31 +++---- test/access-control/e2e.spec.ts | 17 +++- 16 files changed, 199 insertions(+), 241 deletions(-) create mode 100644 packages/ui/src/utilities/abortAndIgnore.ts diff --git a/packages/next/src/views/CreateFirstUser/index.client.tsx b/packages/next/src/views/CreateFirstUser/index.client.tsx index cac70c04e..4ba48599b 100644 --- a/packages/next/src/views/CreateFirstUser/index.client.tsx +++ b/packages/next/src/views/CreateFirstUser/index.client.tsx @@ -20,7 +20,8 @@ import { useServerFunctions, useTranslation, } from '@payloadcms/ui' -import React from 'react' +import { abortAndIgnore } from '@payloadcms/ui/shared' +import React, { useEffect } from 'react' export const CreateFirstUserClient: React.FC<{ docPermissions: DocumentPermissions @@ -42,10 +43,17 @@ export const CreateFirstUserClient: React.FC<{ const { t } = useTranslation() const { setUser } = useAuth() + const formStateAbortControllerRef = React.useRef(null) + const collectionConfig = getEntityConfig({ collectionSlug: userSlug }) as ClientCollectionConfig const onChange: FormProps['onChange'][0] = React.useCallback( async ({ formState: prevFormState }) => { + abortAndIgnore(formStateAbortControllerRef.current) + + const controller = new AbortController() + formStateAbortControllerRef.current = controller + const { state } = await getFormState({ collectionSlug: userSlug, docPermissions, @@ -53,6 +61,7 @@ export const CreateFirstUserClient: React.FC<{ formState: prevFormState, operation: 'create', schemaPath: `_${userSlug}.auth`, + signal: controller.signal, }) return state @@ -64,6 +73,12 @@ export const CreateFirstUserClient: React.FC<{ setUser(data) } + useEffect(() => { + return () => { + abortAndIgnore(formStateAbortControllerRef.current) + } + }, []) + return (
= ({ const [isReadOnlyForIncomingUser, setIsReadOnlyForIncomingUser] = useState(false) const [showTakeOverModal, setShowTakeOverModal] = useState(false) - const abortControllerRef = useRef(new AbortController()) + const formStateAbortControllerRef = useRef(new AbortController()) const [editSessionStartTime, setEditSessionStartTime] = useState(Date.now()) @@ -176,16 +181,10 @@ const PreviewView: React.FC = ({ const onChange: FormProps['onChange'][0] = useCallback( async ({ formState: prevFormState }) => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (_err) { - // swallow error - } - } + abortAndIgnore(formStateAbortControllerRef.current) - const abortController = new AbortController() - abortControllerRef.current = abortController + const controller = new AbortController() + formStateAbortControllerRef.current = controller const currentTime = Date.now() const timeSinceLastUpdate = currentTime - editSessionStartTime @@ -208,7 +207,7 @@ const PreviewView: React.FC = ({ operation, returnLockStatus: isLockingEnabled ? true : false, schemaPath, - signal: abortController.signal, + signal: controller.signal, updateLastEdited, }) @@ -265,14 +264,6 @@ const PreviewView: React.FC = ({ // Clean up when the component unmounts or when the document is unlocked useEffect(() => { return () => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (_err) { - // swallow error - } - } - if (!isLockingEnabled) { return } @@ -316,6 +307,12 @@ const PreviewView: React.FC = ({ setDocumentIsLocked, ]) + useEffect(() => { + return () => { + abortAndIgnore(formStateAbortControllerRef.current) + } + }) + const shouldShowDocumentLockedModal = documentIsLocked && currentEditor && diff --git a/packages/richtext-lexical/src/features/blocks/client/component/index.tsx b/packages/richtext-lexical/src/features/blocks/client/component/index.tsx index dfe4e422e..4a230a2d5 100644 --- a/packages/richtext-lexical/src/features/blocks/client/component/index.tsx +++ b/packages/richtext-lexical/src/features/blocks/client/component/index.tsx @@ -11,6 +11,7 @@ import { useServerFunctions, useTranslation, } from '@payloadcms/ui' +import { abortAndIgnore } from '@payloadcms/ui/shared' import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' const baseClass = 'lexical-block' @@ -37,7 +38,7 @@ export const BlockComponent: React.FC = (props) => { const { fieldProps: { featureClientSchemaMap, field: parentLexicalRichTextField, path, schemaPath }, } = useEditorConfigContext() - const abortControllerRef = useRef(new AbortController()) + const onChangeAbortControllerRef = useRef(new AbortController()) const { docPermissions, getDocPreferences } = useDocumentInfo() const { getFormState } = useServerFunctions() @@ -69,7 +70,6 @@ export const BlockComponent: React.FC = (props) => { data: formData, docPermissions, docPreferences: await getDocPreferences(), - doNotAbort: true, globalSlug, operation: 'update', renderAllFields: true, @@ -94,11 +94,7 @@ export const BlockComponent: React.FC = (props) => { } return () => { - try { - abortController.abort() - } catch (_err) { - // swallow error - } + abortAndIgnore(abortController) } }, [ getFormState, @@ -108,32 +104,26 @@ export const BlockComponent: React.FC = (props) => { globalSlug, getDocPreferences, docPermissions, - ]) // DO NOT ADD FORMDATA HERE! Adding formData will kick you out of sub block editors while writing. + // DO NOT ADD FORMDATA HERE! Adding formData will kick you out of sub block editors while writing. + ]) const onChange = useCallback( async ({ formState: prevFormState }) => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (_err) { - // swallow error - } - } + abortAndIgnore(onChangeAbortControllerRef.current) - const abortController = new AbortController() - abortControllerRef.current = abortController + const controller = new AbortController() + onChangeAbortControllerRef.current = controller const { state: newFormState } = await getFormState({ id, collectionSlug, docPermissions, docPreferences: await getDocPreferences(), - doNotAbort: true, formState: prevFormState, globalSlug, operation: 'update', schemaPath: schemaFieldsPath, - signal: abortController.signal, + signal: controller.signal, }) if (!newFormState) { @@ -162,16 +152,9 @@ export const BlockComponent: React.FC = (props) => { ], ) - // cleanup effect useEffect(() => { - const abortController = abortControllerRef.current - return () => { - try { - abortController.abort() - } catch (_err) { - // swallow error - } + abortAndIgnore(onChangeAbortControllerRef.current) } }, []) @@ -243,7 +226,8 @@ export const BlockComponent: React.FC = (props) => { schemaFieldsPath, classNames, i18n, - ]) // DO NOT ADD FORMDATA HERE! Adding formData will kick you out of sub block editors while writing. + // DO NOT ADD FORMDATA HERE! Adding formData will kick you out of sub block editors while writing. + ]) return
{formContent}
} diff --git a/packages/richtext-lexical/src/utilities/fieldsDrawer/DrawerContent.tsx b/packages/richtext-lexical/src/utilities/fieldsDrawer/DrawerContent.tsx index 8461c6db6..9714ce33d 100644 --- a/packages/richtext-lexical/src/utilities/fieldsDrawer/DrawerContent.tsx +++ b/packages/richtext-lexical/src/utilities/fieldsDrawer/DrawerContent.tsx @@ -9,6 +9,7 @@ import { useServerFunctions, useTranslation, } from '@payloadcms/ui' +import { abortAndIgnore } from '@payloadcms/ui/shared' import React, { useCallback, useEffect, useRef, useState } from 'react' import { v4 as uuid } from 'uuid' @@ -28,7 +29,7 @@ export const DrawerContent: React.FC(false) @@ -45,7 +46,7 @@ export const DrawerContent: React.FC { - const abortController = new AbortController() + const controller = new AbortController() const awaitInitialState = async () => { const { state } = await getFormState({ @@ -54,12 +55,11 @@ export const DrawerContent: React.FC { - try { - abortController.abort() - } catch (_err) { - // swallow error - } + abortAndIgnore(controller) } }, [ schemaFieldsPath, @@ -87,16 +83,10 @@ export const DrawerContent: React.FC { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (_err) { - // swallow error - } - } + abortAndIgnore(onChangeAbortControllerRef.current) - const abortController = new AbortController() - abortControllerRef.current = abortController + const controller = new AbortController() + onChangeAbortControllerRef.current = controller const { state } = await getFormState({ id, @@ -107,7 +97,7 @@ export const DrawerContent: React.FC { - const abortController = abortControllerRef.current - return () => { - try { - abortController.abort() - } catch (_err) { - // swallow error - } + abortAndIgnore(onChangeAbortControllerRef.current) } }, []) diff --git a/packages/richtext-slate/src/field/elements/link/Button/index.tsx b/packages/richtext-slate/src/field/elements/link/Button/index.tsx index 4b208e8e8..fa7b645c4 100644 --- a/packages/richtext-slate/src/field/elements/link/Button/index.tsx +++ b/packages/richtext-slate/src/field/elements/link/Button/index.tsx @@ -100,7 +100,6 @@ export const LinkButton: React.FC<{ data, docPermissions, docPreferences: await getDocPreferences(), - doNotAbort: true, globalSlug, operation: 'update', renderAllFields: true, diff --git a/packages/richtext-slate/src/field/elements/link/Element/index.tsx b/packages/richtext-slate/src/field/elements/link/Element/index.tsx index 578fb4306..42a8a1487 100644 --- a/packages/richtext-slate/src/field/elements/link/Element/index.tsx +++ b/packages/richtext-slate/src/field/elements/link/Element/index.tsx @@ -103,7 +103,6 @@ export const LinkElement = () => { data, docPermissions, docPreferences: await getDocPreferences(), - doNotAbort: true, globalSlug, operation: 'update', renderAllFields: true, diff --git a/packages/richtext-slate/src/field/elements/upload/Element/UploadDrawer/index.tsx b/packages/richtext-slate/src/field/elements/upload/Element/UploadDrawer/index.tsx index 58ccc41e3..21aff2e2e 100644 --- a/packages/richtext-slate/src/field/elements/upload/Element/UploadDrawer/index.tsx +++ b/packages/richtext-slate/src/field/elements/upload/Element/UploadDrawer/index.tsx @@ -77,7 +77,6 @@ export const UploadDrawer: React.FC<{ data, docPermissions, docPreferences: await getDocPreferences(), - doNotAbort: true, globalSlug, operation: 'update', renderAllFields: true, diff --git a/packages/ui/src/elements/BulkUpload/EditForm/index.tsx b/packages/ui/src/elements/BulkUpload/EditForm/index.tsx index 373151af0..242a1104b 100644 --- a/packages/ui/src/elements/BulkUpload/EditForm/index.tsx +++ b/packages/ui/src/elements/BulkUpload/EditForm/index.tsx @@ -3,7 +3,7 @@ import type { ClientCollectionConfig } from 'payload' import { useRouter, useSearchParams } from 'next/navigation.js' -import React, { useCallback } from 'react' +import React, { useCallback, useEffect } from 'react' import type { EditFormProps } from './types.js' @@ -17,6 +17,7 @@ import { useEditDepth } from '../../../providers/EditDepth/index.js' import { OperationProvider } from '../../../providers/Operation/index.js' import { useServerFunctions } from '../../../providers/ServerFunctions/index.js' import { useUploadEdits } from '../../../providers/UploadEdits/index.js' +import { abortAndIgnore } from '../../../utilities/abortAndIgnore.js' import { formatAdminURL } from '../../../utilities/formatAdminURL.js' import { useDocumentDrawerContext } from '../../DocumentDrawer/Provider.js' import { DocumentFields } from '../../DocumentFields/index.js' @@ -55,8 +56,9 @@ export function EditForm({ submitted }: EditFormProps) { getEntityConfig, } = useConfig() - const collectionConfig = getEntityConfig({ collectionSlug: docSlug }) as ClientCollectionConfig + const formStateAbortControllerRef = React.useRef(null) + const collectionConfig = getEntityConfig({ collectionSlug: docSlug }) as ClientCollectionConfig const router = useRouter() const depth = useEditDepth() const params = useSearchParams() @@ -109,6 +111,11 @@ export function EditForm({ submitted }: EditFormProps) { const onChange: NonNullable[0] = useCallback( async ({ formState: prevFormState }) => { + abortAndIgnore(formStateAbortControllerRef.current) + + const controller = new AbortController() + formStateAbortControllerRef.current = controller + const docPreferences = await getDocPreferences() const { state: newFormState } = await getFormState({ collectionSlug, @@ -117,6 +124,7 @@ export function EditForm({ submitted }: EditFormProps) { formState: prevFormState, operation: 'create', schemaPath, + signal: controller.signal, }) return newFormState @@ -124,6 +132,12 @@ export function EditForm({ submitted }: EditFormProps) { [collectionSlug, schemaPath, getDocPreferences, getFormState, docPermissions], ) + useEffect(() => { + return () => { + abortAndIgnore(formStateAbortControllerRef.current) + } + }, []) + return ( diff --git a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx index 33c0544c8..8dc69f2b3 100644 --- a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx +++ b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx @@ -10,6 +10,7 @@ import { LoadingOverlay } from '../../elements/Loading/index.js' import { useConfig } from '../../providers/Config/index.js' import { useServerFunctions } from '../../providers/ServerFunctions/index.js' import { useTranslation } from '../../providers/Translation/index.js' +import { abortAndIgnore } from '../../utilities/abortAndIgnore.js' import { DocumentDrawerContextProvider } from './Provider.js' export const DocumentDrawerContent: React.FC = ({ @@ -35,6 +36,8 @@ export const DocumentDrawerContent: React.FC = ({ collections.find((collection) => collection.slug === collectionSlug), ) + const documentViewAbortControllerRef = React.useRef(null) + const { closeModal } = useModal() const { t } = useTranslation() @@ -44,31 +47,40 @@ export const DocumentDrawerContent: React.FC = ({ const [isLoading, setIsLoading] = useState(true) const getDocumentView = useCallback( - async (docID?: number | string, doNotAbort?: boolean) => { - setIsLoading(true) + (docID?: number | string) => { + abortAndIgnore(documentViewAbortControllerRef.current) - try { - const result = await renderDocument({ - collectionSlug, - disableActions, - docID, - doNotAbort, - drawerSlug, - initialData, - redirectAfterDelete: redirectAfterDelete !== undefined ? redirectAfterDelete : false, - redirectAfterDuplicate: - redirectAfterDuplicate !== undefined ? redirectAfterDuplicate : false, - }) + const controller = new AbortController() + documentViewAbortControllerRef.current = controller - if (result?.Document) { - setDocumentView(result.Document) - setIsLoading(false) + const fetchDocumentView = async () => { + setIsLoading(true) + + try { + const result = await renderDocument({ + collectionSlug, + disableActions, + docID, + drawerSlug, + initialData, + redirectAfterDelete: redirectAfterDelete !== undefined ? redirectAfterDelete : false, + redirectAfterDuplicate: + redirectAfterDuplicate !== undefined ? redirectAfterDuplicate : false, + signal: controller.signal, + }) + + if (result?.Document) { + setDocumentView(result.Document) + setIsLoading(false) + } + } catch (error) { + toast.error(error?.message || t('error:unspecific')) + closeModal(drawerSlug) + // toast.error(data?.errors?.[0].message || t('error:unspecific')) } - } catch (error) { - toast.error(error?.message || t('error:unspecific')) - closeModal(drawerSlug) - // toast.error(data?.errors?.[0].message || t('error:unspecific')) } + + void fetchDocumentView() }, [ collectionSlug, @@ -79,20 +91,13 @@ export const DocumentDrawerContent: React.FC = ({ redirectAfterDuplicate, renderDocument, closeModal, - initialState, t, ], ) - useEffect(() => { - if (!DocumentView) { - void getDocumentView(existingDocID, true) - } - }, [DocumentView, getDocumentView, existingDocID]) - const onSave = useCallback( (args) => { - void getDocumentView(args.doc.id) + getDocumentView(args.doc.id) if (typeof onSaveFromProps === 'function') { void onSaveFromProps({ @@ -104,9 +109,9 @@ export const DocumentDrawerContent: React.FC = ({ [onSaveFromProps, collectionConfig, getDocumentView], ) - const onDuplicate = useCallback( + const onDuplicate = useCallback( (args) => { - void getDocumentView(args.doc.id) + getDocumentView(args.doc.id) if (typeof onDuplicateFromProps === 'function') { void onDuplicateFromProps({ @@ -133,9 +138,22 @@ export const DocumentDrawerContent: React.FC = ({ ) const clearDoc = useCallback(() => { - void getDocumentView() + getDocumentView() }, [getDocumentView]) + useEffect(() => { + if (!DocumentView) { + getDocumentView(existingDocID) + } + }, [DocumentView, getDocumentView, existingDocID]) + + // Cleanup any pending requests when the component unmounts + useEffect(() => { + return () => { + abortAndIgnore(documentViewAbortControllerRef.current) + } + }, []) + if (isLoading) { return } diff --git a/packages/ui/src/elements/EditMany/index.tsx b/packages/ui/src/elements/EditMany/index.tsx index cd778e97a..a95bfdcb4 100644 --- a/packages/ui/src/elements/EditMany/index.tsx +++ b/packages/ui/src/elements/EditMany/index.tsx @@ -4,7 +4,7 @@ import type { ClientCollectionConfig, FormState } from 'payload' import { useModal } from '@faceless-ui/modal' import { getTranslation } from '@payloadcms/translations' import { useRouter } from 'next/navigation.js' -import React, { useCallback, useState } from 'react' +import React, { useCallback, useEffect, useState } from 'react' import type { FormProps } from '../../forms/Form/index.js' @@ -23,6 +23,7 @@ import { useSearchParams } from '../../providers/SearchParams/index.js' import { SelectAllStatus, useSelection } from '../../providers/Selection/index.js' import { useServerFunctions } from '../../providers/ServerFunctions/index.js' import { useTranslation } from '../../providers/Translation/index.js' +import { abortAndIgnore } from '../../utilities/abortAndIgnore.js' import { Drawer, DrawerToggler } from '../Drawer/index.js' import { FieldSelect } from '../FieldSelect/index.js' import './index.scss' @@ -127,6 +128,7 @@ export const EditMany: React.FC = (props) => { const router = useRouter() const [initialState, setInitialState] = useState() const hasInitializedState = React.useRef(false) + const formStateAbortControllerRef = React.useRef(null) const { clearRouteCache } = useRouteCache() const collectionPermissions = permissions?.collections?.[slug] @@ -135,6 +137,8 @@ export const EditMany: React.FC = (props) => { const drawerSlug = `edit-${slug}` React.useEffect(() => { + const controller = new AbortController() + if (!hasInitializedState.current) { const getInitialState = async () => { const { state: result } = await getFormState({ @@ -144,6 +148,7 @@ export const EditMany: React.FC = (props) => { docPreferences: null, operation: 'update', schemaPath: slug, + signal: controller.signal, }) setInitialState(result) @@ -151,11 +156,20 @@ export const EditMany: React.FC = (props) => { } void getInitialState() + + return () => { + abortAndIgnore(controller) + } } }, [apiRoute, hasInitializedState, serverURL, slug, getFormState, user, collectionPermissions]) const onChange: FormProps['onChange'][0] = useCallback( async ({ formState: prevFormState }) => { + abortAndIgnore(formStateAbortControllerRef.current) + + const controller = new AbortController() + formStateAbortControllerRef.current = controller + const { state } = await getFormState({ collectionSlug: slug, docPermissions: collectionPermissions, @@ -163,6 +177,7 @@ export const EditMany: React.FC = (props) => { formState: prevFormState, operation: 'update', schemaPath: slug, + signal: controller.signal, }) return state @@ -170,6 +185,10 @@ export const EditMany: React.FC = (props) => { [slug, getFormState, collectionPermissions], ) + useEffect(() => { + abortAndIgnore(formStateAbortControllerRef.current) + }, []) + if (selectAll === SelectAllStatus.None || !hasUpdatePermission) { return null } diff --git a/packages/ui/src/exports/shared/index.ts b/packages/ui/src/exports/shared/index.ts index fd85c1b06..041c0253e 100644 --- a/packages/ui/src/exports/shared/index.ts +++ b/packages/ui/src/exports/shared/index.ts @@ -7,6 +7,7 @@ export { WithServerSideProps } from '../../elements/WithServerSideProps/index.js export { reduceToSerializableFields } from '../../forms/Form/reduceToSerializableFields.js' export { PayloadIcon } from '../../graphics/Icon/index.js' export { PayloadLogo } from '../../graphics/Logo/index.js' +export { abortAndIgnore } from '../../utilities/abortAndIgnore.js' export { requests } from '../../utilities/api.js' export { findLocaleFromCode } from '../../utilities/findLocaleFromCode.js' export { formatAdminURL } from '../../utilities/formatAdminURL.js' diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 1954a0c79..edcd61e35 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -29,6 +29,7 @@ import { useLocale } from '../../providers/Locale/index.js' import { useOperation } from '../../providers/Operation/index.js' import { useServerFunctions } from '../../providers/ServerFunctions/index.js' import { useTranslation } from '../../providers/Translation/index.js' +import { abortAndIgnore } from '../../utilities/abortAndIgnore.js' import { requests } from '../../utilities/api.js' import { FormContext, @@ -91,8 +92,7 @@ export const Form: React.FC = (props) => { const [submitted, setSubmitted] = useState(false) const formRef = useRef(null) const contextRef = useRef({} as FormContextType) - const abortControllerRef = useRef(new AbortController()) - const abortControllerRef2 = useRef(new AbortController()) + const resetFormStateAbortControllerRef = useRef(null) const fieldsReducer = useReducer(fieldReducer, {}, () => initialState) @@ -456,16 +456,10 @@ export const Form: React.FC = (props) => { const reset = useCallback( async (data: unknown) => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (error) { - // swallow error - } - } + abortAndIgnore(resetFormStateAbortControllerRef.current) - const abortController = new AbortController() - abortControllerRef.current = abortController + const controller = new AbortController() + resetFormStateAbortControllerRef.current = controller const docPreferences = await getDocPreferences() @@ -479,7 +473,7 @@ export const Form: React.FC = (props) => { operation, renderAllFields: true, schemaPath: collectionSlug ? collectionSlug : globalSlug, - signal: abortController.signal, + signal: controller.signal, }) contextRef.current = { ...initContextState } as FormContextType @@ -548,27 +542,9 @@ export const Form: React.FC = (props) => { [dispatchFields, getDataByPath], ) - // clean on unmount useEffect(() => { - const re1 = abortControllerRef.current - const re2 = abortControllerRef2.current - return () => { - if (re1) { - try { - re1.abort() - } catch (_err) { - // swallow error - } - } - - if (re2) { - try { - re2.abort() - } catch (_err) { - // swallow error - } - } + abortAndIgnore(resetFormStateAbortControllerRef.current) } }, []) diff --git a/packages/ui/src/providers/ServerFunctions/index.tsx b/packages/ui/src/providers/ServerFunctions/index.tsx index 44862c859..2a8afdce2 100644 --- a/packages/ui/src/providers/ServerFunctions/index.tsx +++ b/packages/ui/src/providers/ServerFunctions/index.tsx @@ -6,14 +6,13 @@ import type { ServerFunctionClient, } from 'payload' -import React, { createContext, useCallback, useEffect, useRef } from 'react' +import React, { createContext, useCallback } from 'react' import type { buildFormStateHandler } from '../../utilities/buildFormState.js' import type { buildTableStateHandler } from '../../utilities/buildTableState.js' type GetFormStateClient = ( args: { - doNotAbort?: boolean signal?: AbortSignal } & Omit, ) => ReturnType @@ -28,7 +27,6 @@ type RenderDocument = (args: { collectionSlug: string disableActions?: boolean docID?: number | string - doNotAbort?: boolean drawerSlug?: string initialData?: Data redirectAfterDelete?: boolean @@ -69,10 +67,6 @@ export const ServerFunctionsProvider: React.FC<{ throw new Error('ServerFunctionsProvider requires a serverFunction prop') } - // This is the local abort controller, to abort requests when the _provider_ itself unmounts, etc. - // Each callback also accept a remote signal, to abort requests when each _component_ unmounts, etc. - const abortControllerRef = useRef(new AbortController()) - const getDocumentSlots = useCallback( async (args) => await serverFunction({ @@ -84,39 +78,16 @@ export const ServerFunctionsProvider: React.FC<{ const getFormState = useCallback( async (args) => { - if (args?.doNotAbort) { - try { - const result = (await serverFunction({ - name: 'form-state', - args, - })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled - - return result - } catch (_err) { - console.error(_err) // eslint-disable-line no-console - } - - return { state: null } - } - - if (abortControllerRef.current) { - abortControllerRef.current.abort() - } - - const abortController = new AbortController() - abortControllerRef.current = abortController - const localSignal = abortController.signal - const { signal: remoteSignal, ...rest } = args || {} try { - if (!remoteSignal?.aborted && !localSignal?.aborted) { + if (!remoteSignal?.aborted) { const result = (await serverFunction({ name: 'form-state', args: rest, })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled - if (!remoteSignal?.aborted && !localSignal?.aborted) { + if (!remoteSignal?.aborted) { return result } } @@ -131,15 +102,19 @@ export const ServerFunctionsProvider: React.FC<{ const getTableState = useCallback( async (args) => { - const { ...rest } = args || {} + const { signal: remoteSignal, ...rest } = args || {} try { - const result = (await serverFunction({ - name: 'table-state', - args: rest, - })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled + if (!remoteSignal?.aborted) { + const result = (await serverFunction({ + name: 'table-state', + args: rest, + })) as ReturnType // TODO: infer this type when `strictNullChecks` is enabled - return result + if (!remoteSignal?.aborted) { + return result + } + } } catch (_err) { console.error(_err) // eslint-disable-line no-console } @@ -151,37 +126,16 @@ export const ServerFunctionsProvider: React.FC<{ const renderDocument = useCallback( async (args) => { - if (args?.doNotAbort) { - try { - const result = (await serverFunction({ - name: 'render-document', - args, - })) as { docID: string; Document: React.ReactNode } - - return result - } catch (_err) { - console.error(_err) // eslint-disable-line no-console - return - } - } - if (abortControllerRef.current) { - abortControllerRef.current.abort() - } - - const abortController = new AbortController() - abortControllerRef.current = abortController - const localSignal = abortController.signal - const { signal: remoteSignal, ...rest } = args || {} try { - if (!remoteSignal?.aborted && !localSignal?.aborted) { + if (!remoteSignal?.aborted) { const result = (await serverFunction({ name: 'render-document', args: rest, })) as { docID: string; Document: React.ReactNode } - if (!remoteSignal?.aborted && !localSignal?.aborted) { + if (!remoteSignal?.aborted) { return result } } @@ -192,20 +146,6 @@ export const ServerFunctionsProvider: React.FC<{ [serverFunction], ) - useEffect(() => { - const controller = abortControllerRef.current - - return () => { - if (controller) { - try { - // controller.abort() - } catch (_err) { - // swallow error - } - } - } - }, []) - return ( = ({ const { resetUploadEdits } = useUploadEdits() const { getFormState } = useServerFunctions() - const abortControllerRef = useRef(new AbortController()) + const onChangeAbortControllerRef = useRef(null) const locale = params.get('locale') @@ -261,16 +262,10 @@ export const DefaultEditView: React.FC = ({ const onChange: FormProps['onChange'][0] = useCallback( async ({ formState: prevFormState }) => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (e) { - // swallow error - } - } + abortAndIgnore(onChangeAbortControllerRef.current) - const abortController = new AbortController() - abortControllerRef.current = abortController + const controller = new AbortController() + onChangeAbortControllerRef.current = controller const currentTime = Date.now() const timeSinceLastUpdate = currentTime - editSessionStartTime @@ -296,7 +291,7 @@ export const DefaultEditView: React.FC = ({ renderAllFields: false, returnLockStatus: isLockingEnabled ? true : false, schemaPath: schemaPathSegments.join('.'), - // signal: abortController.signal, + signal: controller.signal, updateLastEdited, }) @@ -352,14 +347,6 @@ export const DefaultEditView: React.FC = ({ // Clean up when the component unmounts or when the document is unlocked useEffect(() => { return () => { - if (abortControllerRef.current) { - try { - abortControllerRef.current.abort() - } catch (e) { - // swallow error - } - } - if (!isLockingEnabled) { return } @@ -403,6 +390,12 @@ export const DefaultEditView: React.FC = ({ setDocumentIsLocked, ]) + useEffect(() => { + return () => { + abortAndIgnore(onChangeAbortControllerRef.current) + } + }, []) + const shouldShowDocumentLockedModal = documentIsLocked && currentEditor && diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index 0e0797191..af4ac5aef 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -481,10 +481,8 @@ describe('access control', () => { await expect(page.locator('.unauthorized')).toBeVisible() - await page.goto(logoutURL) - await page.waitForURL(logoutURL) - // Log back in for the next test + await page.goto(logoutURL) await login({ data: { email: devUser.email, @@ -528,6 +526,19 @@ describe('access control', () => { await page.waitForURL(unauthorizedURL) await expect(page.locator('.unauthorized')).toBeVisible() + + // Log back in for the next test + await context.clearCookies() + await page.goto(logoutURL) + await page.waitForURL(logoutURL) + await login({ + data: { + email: devUser.email, + password: devUser.password, + }, + page, + serverURL, + }) }) })