fix(ui): significantly optimizes relationship field (#8063)
## Description
Reduces the number of client-side requests made by the relationship
field component, and fixes the visual "blink" of the field's value on
initial load. Does so through a new `useIgnoredEffect` hook that allows
this component's effects to be precisely triggered based on whether a
only _subset_ of its dependencies have changed, which looks something
like this:
```tsx
// ...
useIgnoredEffect(() => {
// Do something
}, [deps], [ignoredDeps])
```
"Ignored deps" are still treated as normal dependencies of the
underlying `useEffect` hook, but they do not cause the provided function
to execute. This is useful if you have a list of dependencies that
change often, but need to scope your effect's logic to explicit
dependencies within that list. This is a typical pattern in React using
refs, just standardized within a reusable hook.
This significantly reduces the overall number of re-renders and
duplicative API requests within the relationship field because the
`useEffect` hooks that control the fetching of these related documents
were running unnecessarily often. In the future, we really ought to
leverage the `RelationshipProvider` used in the List View so that we can
also reduce the number of duplicative requests across _unrelated fields_
within the same document.
Before:
https://github.com/user-attachments/assets/ece7c85e-20fb-49f6-b393-c5e9d5176192
After:
https://github.com/user-attachments/assets/9f0a871e-f10f-4fd6-a58b-8146ece288c4
- [x] I have read and understand the
[CONTRIBUTING.md](https://github.com/payloadcms/payload/blob/main/CONTRIBUTING.md)
document in this repository.
## Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
## Checklist:
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] Existing test suite passes locally with my changes
This commit is contained in:
@@ -14,6 +14,7 @@ import { useFieldProps } from '../../forms/FieldPropsProvider/index.js'
|
||||
import { useField } from '../../forms/useField/index.js'
|
||||
import { withCondition } from '../../forms/withCondition/index.js'
|
||||
import { useDebouncedCallback } from '../../hooks/useDebouncedCallback.js'
|
||||
import { useIgnoredEffect } from '../../hooks/useIgnoredEffect.js'
|
||||
import { useAuth } from '../../providers/Auth/index.js'
|
||||
import { useConfig } from '../../providers/Config/index.js'
|
||||
import { useLocale } from '../../providers/Locale/index.js'
|
||||
@@ -74,14 +75,13 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
const { permissions } = useAuth()
|
||||
const { code: locale } = useLocale()
|
||||
const hasMultipleRelations = Array.isArray(relationTo)
|
||||
const [options, dispatchOptions] = useReducer(optionsReducer, [])
|
||||
const [lastFullyLoadedRelation, setLastFullyLoadedRelation] = useState(-1)
|
||||
const [lastLoadedPage, setLastLoadedPage] = useState<Record<string, number>>({})
|
||||
const [errorLoading, setErrorLoading] = useState('')
|
||||
const [search, setSearch] = useState('')
|
||||
const [isLoading, setIsLoading] = useState(false)
|
||||
const [enableWordBoundarySearch, setEnableWordBoundarySearch] = useState(false)
|
||||
const menuIsOpen = useRef(false)
|
||||
const [menuIsOpen, setMenuIsOpen] = useState(false)
|
||||
const hasLoadedFirstPageRef = useRef(false)
|
||||
|
||||
const memoizedValidate = useCallback(
|
||||
@@ -107,6 +107,7 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
path: pathFromContext ?? pathFromProps ?? name,
|
||||
validate: memoizedValidate,
|
||||
})
|
||||
const [options, dispatchOptions] = useReducer(optionsReducer, [])
|
||||
|
||||
const readOnly = readOnlyFromProps || readOnlyFromContext || formInitializing
|
||||
|
||||
@@ -117,6 +118,7 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
|
||||
const getResults: GetResults = useCallback(
|
||||
async ({
|
||||
filterOptions,
|
||||
lastFullyLoadedRelation: lastFullyLoadedRelationArg,
|
||||
lastLoadedPage: lastLoadedPageArg,
|
||||
onSuccess,
|
||||
@@ -273,7 +275,6 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
search,
|
||||
collections,
|
||||
locale,
|
||||
filterOptions,
|
||||
serverURL,
|
||||
sortOptions,
|
||||
api,
|
||||
@@ -284,7 +285,13 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
)
|
||||
|
||||
const updateSearch = useDebouncedCallback((searchArg: string, valueArg: Value | Value[]) => {
|
||||
void getResults({ lastLoadedPage: {}, search: searchArg, sort: true, value: valueArg })
|
||||
void getResults({
|
||||
filterOptions,
|
||||
lastLoadedPage: {},
|
||||
search: searchArg,
|
||||
sort: true,
|
||||
value: valueArg,
|
||||
})
|
||||
setSearch(searchArg)
|
||||
}, 300)
|
||||
|
||||
@@ -302,7 +309,8 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
// Ensure we have an option for each value
|
||||
// ///////////////////////////////////
|
||||
|
||||
useEffect(() => {
|
||||
useIgnoredEffect(
|
||||
() => {
|
||||
const relationMap = createRelationMap({
|
||||
hasMany,
|
||||
relationTo,
|
||||
@@ -367,9 +375,10 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
}
|
||||
}
|
||||
}, Promise.resolve())
|
||||
}, [
|
||||
},
|
||||
[value],
|
||||
[
|
||||
options,
|
||||
value,
|
||||
hasMany,
|
||||
errorLoading,
|
||||
collections,
|
||||
@@ -380,7 +389,8 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
relationTo,
|
||||
locale,
|
||||
config,
|
||||
])
|
||||
],
|
||||
)
|
||||
|
||||
// Determine if we should switch to word boundary search
|
||||
useEffect(() => {
|
||||
@@ -395,14 +405,16 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
|
||||
// When (`relationTo` || `filterOptions` || `locale`) changes, reset component
|
||||
// Note - effect should not run on first run
|
||||
useEffect(() => {
|
||||
useIgnoredEffect(
|
||||
|
||||
() => {
|
||||
// If the menu is open while filterOptions changes
|
||||
// due to latency of getFormState and fast clicking into this field,
|
||||
// re-fetch options
|
||||
|
||||
if (hasLoadedFirstPageRef.current && menuIsOpen.current) {
|
||||
if (hasLoadedFirstPageRef.current && menuIsOpen) {
|
||||
setIsLoading(true)
|
||||
void getResults({
|
||||
filterOptions,
|
||||
lastLoadedPage: {},
|
||||
onSuccess: () => {
|
||||
hasLoadedFirstPageRef.current = true
|
||||
@@ -413,23 +425,18 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
}
|
||||
|
||||
// If the menu is not open, still reset the field state
|
||||
// because we need to get new options next time the menu
|
||||
// opens by the user
|
||||
// because we need to get new options next time the menu opens
|
||||
dispatchOptions({
|
||||
type: 'CLEAR',
|
||||
exemptValues: valueRef.current,
|
||||
})
|
||||
|
||||
dispatchOptions({ type: 'CLEAR' })
|
||||
setLastFullyLoadedRelation(-1)
|
||||
setLastLoadedPage({})
|
||||
hasLoadedFirstPageRef.current = false
|
||||
}, [
|
||||
relationTo,
|
||||
filterOptions,
|
||||
locale,
|
||||
menuIsOpen,
|
||||
getResults,
|
||||
valueRef,
|
||||
hasLoadedFirstPageRef,
|
||||
path,
|
||||
])
|
||||
},
|
||||
[relationTo, filterOptions, locale, path, menuIsOpen],
|
||||
[getResults],
|
||||
)
|
||||
|
||||
const onSave = useCallback<DocumentDrawerProps['onSave']>(
|
||||
(args) => {
|
||||
@@ -565,14 +572,15 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
}
|
||||
onInputChange={(newSearch) => handleInputChange(newSearch, value)}
|
||||
onMenuClose={() => {
|
||||
menuIsOpen.current = false
|
||||
setMenuIsOpen(false)
|
||||
}}
|
||||
onMenuOpen={() => {
|
||||
menuIsOpen.current = true
|
||||
setMenuIsOpen(true)
|
||||
|
||||
if (!hasLoadedFirstPageRef.current) {
|
||||
setIsLoading(true)
|
||||
void getResults({
|
||||
filterOptions,
|
||||
lastLoadedPage: {},
|
||||
onSuccess: () => {
|
||||
hasLoadedFirstPageRef.current = true
|
||||
@@ -584,6 +592,7 @@ const RelationshipFieldComponent: React.FC<RelationshipFieldProps> = (props) =>
|
||||
}}
|
||||
onMenuScrollToBottom={() => {
|
||||
void getResults({
|
||||
filterOptions,
|
||||
lastFullyLoadedRelation,
|
||||
lastLoadedPage,
|
||||
search,
|
||||
|
||||
@@ -29,7 +29,29 @@ const sortOptions = (options: Option[]): Option[] =>
|
||||
export const optionsReducer = (state: OptionGroup[], action: Action): OptionGroup[] => {
|
||||
switch (action.type) {
|
||||
case 'CLEAR': {
|
||||
return []
|
||||
const exemptValues = Array.isArray(action.exemptValues)
|
||||
? action.exemptValues
|
||||
: [action.exemptValues]
|
||||
|
||||
const clearedStateWithExemptValues = state.filter((optionGroup) => {
|
||||
const clearedOptions = optionGroup.options.filter((option) => {
|
||||
if (exemptValues) {
|
||||
return exemptValues.some((exemptValue) => {
|
||||
return (
|
||||
option.value === (typeof exemptValue === 'object' ? exemptValue.value : exemptValue)
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
return false
|
||||
})
|
||||
|
||||
optionGroup.options = clearedOptions
|
||||
|
||||
return clearedOptions.length > 0
|
||||
})
|
||||
|
||||
return clearedStateWithExemptValues
|
||||
}
|
||||
|
||||
case 'UPDATE': {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { I18nClient } from '@payloadcms/translations'
|
||||
import type { ClientCollectionConfig, SanitizedConfig } from 'payload'
|
||||
import type { ClientCollectionConfig, FilterOptionsResult, SanitizedConfig } from 'payload'
|
||||
|
||||
export type Option = {
|
||||
label: string
|
||||
@@ -21,6 +21,7 @@ export type ValueWithRelation = {
|
||||
export type Value = number | string | ValueWithRelation
|
||||
|
||||
type CLEAR = {
|
||||
exemptValues?: Value | Value[]
|
||||
type: 'CLEAR'
|
||||
}
|
||||
|
||||
@@ -45,6 +46,7 @@ type ADD = {
|
||||
export type Action = ADD | CLEAR | UPDATE
|
||||
|
||||
export type GetResults = (args: {
|
||||
filterOptions?: FilterOptionsResult
|
||||
lastFullyLoadedRelation?: number
|
||||
lastLoadedPage: Record<string, number>
|
||||
onSuccess?: () => void
|
||||
|
||||
41
packages/ui/src/hooks/useIgnoredEffect.ts
Normal file
41
packages/ui/src/hooks/useIgnoredEffect.ts
Normal file
@@ -0,0 +1,41 @@
|
||||
import { dequal } from 'dequal/lite'
|
||||
import { useEffect, useRef } from 'react'
|
||||
|
||||
/**
|
||||
* Allows for a `useEffect` hook to be precisely triggered based on whether a only _subset_ of its dependencies have changed, as opposed to all of them. This is useful if you have a list of dependencies that change often, but need to scope your effect's logic to only explicit dependencies within that list.
|
||||
* @constructor
|
||||
* @param {React.EffectCallback} effect - The effect to run
|
||||
* @param {React.DependencyList} deps - Dependencies that should trigger the effect
|
||||
* @param {React.DependencyList} ignoredDeps - Dependencies that should _not_ trigger the effect
|
||||
* @param {Object} options - Additional options to configure the hook
|
||||
* @param {boolean} options.runOnFirstRender - Whether the effect should run on the first render
|
||||
* @example
|
||||
* useIgnoredEffect(() => {
|
||||
* console.log('This will run when `foo` changes, but not when `bar` changes')
|
||||
* }, [foo], [bar])
|
||||
*/
|
||||
export function useIgnoredEffect(
|
||||
effect: React.EffectCallback,
|
||||
deps: React.DependencyList,
|
||||
ignoredDeps: React.DependencyList,
|
||||
options?: { runOnFirstRender?: boolean },
|
||||
) {
|
||||
const hasInitialized = useRef(
|
||||
typeof options?.runOnFirstRender !== 'undefined' ? Boolean(!options?.runOnFirstRender) : false,
|
||||
)
|
||||
|
||||
const prevDeps = useRef(deps)
|
||||
|
||||
useEffect(() => {
|
||||
const depsHaveChanged = deps.some(
|
||||
(dep, index) => !ignoredDeps.includes(dep) && !dequal(dep, prevDeps.current[index]),
|
||||
)
|
||||
|
||||
if (depsHaveChanged || !hasInitialized.current) {
|
||||
effect()
|
||||
}
|
||||
|
||||
prevDeps.current = deps
|
||||
hasInitialized.current = true
|
||||
}, deps)
|
||||
}
|
||||
@@ -1,8 +1,10 @@
|
||||
// DO NOT MODIFY. This file is automatically generated in initDevAndTest.ts
|
||||
|
||||
import { mongooseAdapter } from '@payloadcms/db-mongodb'
|
||||
// DO NOT MODIFY. This file is automatically generated in initDevAndTest.ts
|
||||
|
||||
export const databaseAdapter = mongooseAdapter({
|
||||
|
||||
import { mongooseAdapter } from '@payloadcms/db-mongodb'
|
||||
|
||||
export const databaseAdapter = mongooseAdapter({
|
||||
url:
|
||||
process.env.MONGODB_MEMORY_SERVER_URI ||
|
||||
process.env.DATABASE_URI ||
|
||||
@@ -10,4 +12,5 @@ export const databaseAdapter = mongooseAdapter({
|
||||
collation: {
|
||||
strength: 1,
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
@@ -6,6 +6,7 @@ import type { CollectionConfig, FilterOptionsProps } from 'payload'
|
||||
|
||||
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
|
||||
import { devUser } from '../credentials.js'
|
||||
import { VersionedRelationshipFieldCollection } from './collections/VersionedRelationshipField/index.js'
|
||||
import {
|
||||
collection1Slug,
|
||||
collection2Slug,
|
||||
@@ -21,7 +22,6 @@ import {
|
||||
slug,
|
||||
videoCollectionSlug,
|
||||
} from './collectionSlugs.js'
|
||||
import { VersionedRelationshipFieldCollection } from './collections/VersionedRelationshipField/index.js'
|
||||
|
||||
export interface FieldsRelationship {
|
||||
createdAt: Date
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
saveDocAndAssert,
|
||||
} from '../helpers.js'
|
||||
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
|
||||
import { trackNetworkRequests } from '../helpers/e2e/trackNetworkRequests.js'
|
||||
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
|
||||
import { POLL_TOPASS_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
|
||||
import {
|
||||
@@ -169,6 +170,22 @@ describe('fields - relationship', () => {
|
||||
await saveDocAndAssert(page)
|
||||
})
|
||||
|
||||
test('should only make a single request for relationship values', async () => {
|
||||
await page.goto(url.create)
|
||||
const field = page.locator('#field-relationship')
|
||||
await expect(field.locator('input')).toBeEnabled()
|
||||
await field.click({ delay: 100 })
|
||||
const options = page.locator('.rs__option')
|
||||
await expect(options).toHaveCount(2) // two docs
|
||||
await options.nth(0).click()
|
||||
await expect(field).toContainText(relationOneDoc.id)
|
||||
await saveDocAndAssert(page)
|
||||
await wait(200)
|
||||
await trackNetworkRequests(page, `/api/${relationOneSlug}`, {
|
||||
beforePoll: async () => await page.reload(),
|
||||
})
|
||||
})
|
||||
|
||||
// TODO: Flaky test in CI - fix this. https://github.com/payloadcms/payload/actions/runs/8559547748/job/23456806365
|
||||
test.skip('should create relations to multiple collections', async () => {
|
||||
await page.goto(url.create)
|
||||
|
||||
49
test/helpers/e2e/trackNetworkRequests.ts
Normal file
49
test/helpers/e2e/trackNetworkRequests.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import type { Page, Request } from '@playwright/test'
|
||||
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
// Allows you to test the number of network requests triggered by an action
|
||||
// This can be used to ensure various actions do not trigger unnecessary requests
|
||||
// For example, an effect within a component might fetch data multiple times unnecessarily
|
||||
export const trackNetworkRequests = async (
|
||||
page: Page,
|
||||
url: string,
|
||||
options?: {
|
||||
allowedNumberOfRequests?: number
|
||||
beforePoll?: () => Promise<any> | void
|
||||
interval?: number
|
||||
timeout?: number
|
||||
},
|
||||
): Promise<Array<Request>> => {
|
||||
const { beforePoll, allowedNumberOfRequests = 1, timeout = 5000, interval = 1000 } = options || {}
|
||||
|
||||
const matchedRequests = []
|
||||
|
||||
// begin tracking network requests
|
||||
page.on('request', (request) => {
|
||||
if (request.url().includes(url)) {
|
||||
matchedRequests.push(request)
|
||||
}
|
||||
})
|
||||
|
||||
if (typeof beforePoll === 'function') {
|
||||
await beforePoll()
|
||||
}
|
||||
|
||||
const startTime = Date.now()
|
||||
|
||||
// continuously poll even after a request has been matched
|
||||
// this will ensure no subsequent requests are made
|
||||
// such as a result of a `useEffect` within a component
|
||||
while (Date.now() - startTime < timeout) {
|
||||
if (matchedRequests.length > 0) {
|
||||
expect(matchedRequests.length).toBeLessThanOrEqual(allowedNumberOfRequests)
|
||||
}
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, interval))
|
||||
}
|
||||
|
||||
expect(matchedRequests.length).toBe(allowedNumberOfRequests)
|
||||
|
||||
return matchedRequests
|
||||
}
|
||||
Reference in New Issue
Block a user