fix(next): login redirect crashes page (#13786)
## Problem When logging in with a `?redirect=`, the target page may crash. This happens because the update from `SyncClientConfig` is applied in a `useEffect`, which runs **_after_** the target page's client components render. As a result, those components read the unauthenticated client config on first render - even though they expect the authenticated one. ## Steps To Reproduce 1. logout 2. login with ?redirect= 3. document client component incorrectly still receives unauthenticated client config on render 4. THEN the SyncClientConfig useEffect runs. Too late 5. Potential error (depending on the page, e.g. document view) - document client component expects sth to be there, but it is not ## Solution This PR replaces `SyncClientConfig` with a `RootPageConfigProvider`. This new provider shadows the root layout’s `ConfigProvider` and ensures the correct client config is available **_immediately_**. It still updates the config using `useEffect` (the same way`SyncClientConfig` did), but with one key difference: - During the brief window between the redirect and the effect running, it overrides the root layout’s config and provides the fresh, authenticated config from the root page via the `RootConfigContext`. This guarantees that client components on the target page receive the correct config on first render, preventing errors caused by reading the outdated unauthenticated config. ## Additional change - get rid of `UnsanitizedClientConfig` and `sanitizeClientConfig` Those functions added unnecessary complexity, just to build the blocksMap. I removed those and perform the building of the `blocksMap` server-side - directly in `createClientConfig`. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211334752795621
This commit is contained in:
@@ -1,21 +0,0 @@
|
|||||||
'use client'
|
|
||||||
import type { ClientConfig } from 'payload'
|
|
||||||
|
|
||||||
import { useConfig } from '@payloadcms/ui'
|
|
||||||
import { useEffect } from 'react'
|
|
||||||
|
|
||||||
/**
|
|
||||||
* This component is required in order for the _page_ to be able to refresh the client config,
|
|
||||||
* which may have been cached on the _layout_ level, where the ConfigProvider is managed.
|
|
||||||
* Since the layout does not re-render on page navigation / authentication, we need to manually
|
|
||||||
* update the config, as the user may have been authenticated in the process, which affects the client config.
|
|
||||||
*/
|
|
||||||
export const SyncClientConfig = ({ clientConfig }: { clientConfig: ClientConfig }) => {
|
|
||||||
const { setConfig } = useConfig()
|
|
||||||
|
|
||||||
useEffect(() => {
|
|
||||||
setConfig(clientConfig)
|
|
||||||
}, [clientConfig, setConfig])
|
|
||||||
|
|
||||||
return null
|
|
||||||
}
|
|
||||||
@@ -10,6 +10,7 @@ import type {
|
|||||||
SanitizedGlobalConfig,
|
SanitizedGlobalConfig,
|
||||||
} from 'payload'
|
} from 'payload'
|
||||||
|
|
||||||
|
import { RootPageConfigProvider } from '@payloadcms/ui'
|
||||||
import { RenderServerComponent } from '@payloadcms/ui/elements/RenderServerComponent'
|
import { RenderServerComponent } from '@payloadcms/ui/elements/RenderServerComponent'
|
||||||
import { getClientConfig } from '@payloadcms/ui/utilities/getClientConfig'
|
import { getClientConfig } from '@payloadcms/ui/utilities/getClientConfig'
|
||||||
import { notFound, redirect } from 'next/navigation.js'
|
import { notFound, redirect } from 'next/navigation.js'
|
||||||
@@ -27,7 +28,6 @@ import { isCustomAdminView } from '../../utilities/isCustomAdminView.js'
|
|||||||
import { isPublicAdminRoute } from '../../utilities/isPublicAdminRoute.js'
|
import { isPublicAdminRoute } from '../../utilities/isPublicAdminRoute.js'
|
||||||
import { getCustomViewByRoute } from './getCustomViewByRoute.js'
|
import { getCustomViewByRoute } from './getCustomViewByRoute.js'
|
||||||
import { getRouteData } from './getRouteData.js'
|
import { getRouteData } from './getRouteData.js'
|
||||||
import { SyncClientConfig } from './SyncClientConfig.js'
|
|
||||||
|
|
||||||
export type GenerateViewMetadata = (args: {
|
export type GenerateViewMetadata = (args: {
|
||||||
config: SanitizedConfig
|
config: SanitizedConfig
|
||||||
@@ -300,8 +300,7 @@ export const RootPage = async ({
|
|||||||
})
|
})
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<React.Fragment>
|
<RootPageConfigProvider config={clientConfig}>
|
||||||
<SyncClientConfig clientConfig={clientConfig} />
|
|
||||||
{!templateType && <React.Fragment>{RenderedView}</React.Fragment>}
|
{!templateType && <React.Fragment>{RenderedView}</React.Fragment>}
|
||||||
{templateType === 'minimal' && (
|
{templateType === 'minimal' && (
|
||||||
<MinimalTemplate className={templateClassName}>{RenderedView}</MinimalTemplate>
|
<MinimalTemplate className={templateClassName}>{RenderedView}</MinimalTemplate>
|
||||||
@@ -332,6 +331,6 @@ export const RootPage = async ({
|
|||||||
{RenderedView}
|
{RenderedView}
|
||||||
</DefaultTemplate>
|
</DefaultTemplate>
|
||||||
)}
|
)}
|
||||||
</React.Fragment>
|
</RootPageConfigProvider>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,7 +12,6 @@ import type {
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
type ClientCollectionConfig,
|
type ClientCollectionConfig,
|
||||||
createClientCollectionConfig,
|
|
||||||
createClientCollectionConfigs,
|
createClientCollectionConfigs,
|
||||||
} from '../collections/config/client.js'
|
} from '../collections/config/client.js'
|
||||||
import { createClientBlocks } from '../fields/config/client.js'
|
import { createClientBlocks } from '../fields/config/client.js'
|
||||||
@@ -43,16 +42,6 @@ export type ServerOnlyRootProperties = keyof Pick<
|
|||||||
|
|
||||||
export type ServerOnlyRootAdminProperties = keyof Pick<SanitizedConfig['admin'], 'components'>
|
export type ServerOnlyRootAdminProperties = keyof Pick<SanitizedConfig['admin'], 'components'>
|
||||||
|
|
||||||
export type UnsanitizedClientConfig = {
|
|
||||||
admin: {
|
|
||||||
livePreview?: Omit<RootLivePreviewConfig, ServerOnlyLivePreviewProperties>
|
|
||||||
} & Omit<SanitizedConfig['admin'], 'components' | 'dependencies' | 'livePreview'>
|
|
||||||
blocks: ClientBlock[]
|
|
||||||
collections: ClientCollectionConfig[]
|
|
||||||
custom?: Record<string, any>
|
|
||||||
globals: ClientGlobalConfig[]
|
|
||||||
} & Omit<SanitizedConfig, 'admin' | 'collections' | 'globals' | 'i18n' | ServerOnlyRootProperties>
|
|
||||||
|
|
||||||
export type ClientConfig = {
|
export type ClientConfig = {
|
||||||
admin: {
|
admin: {
|
||||||
livePreview?: Omit<RootLivePreviewConfig, ServerOnlyLivePreviewProperties>
|
livePreview?: Omit<RootLivePreviewConfig, ServerOnlyLivePreviewProperties>
|
||||||
@@ -62,6 +51,7 @@ export type ClientConfig = {
|
|||||||
collections: ClientCollectionConfig[]
|
collections: ClientCollectionConfig[]
|
||||||
custom?: Record<string, any>
|
custom?: Record<string, any>
|
||||||
globals: ClientGlobalConfig[]
|
globals: ClientGlobalConfig[]
|
||||||
|
unauthenticated?: boolean
|
||||||
} & Omit<SanitizedConfig, 'admin' | 'collections' | 'globals' | 'i18n' | ServerOnlyRootProperties>
|
} & Omit<SanitizedConfig, 'admin' | 'collections' | 'globals' | 'i18n' | ServerOnlyRootProperties>
|
||||||
|
|
||||||
export type UnauthenticatedClientConfig = {
|
export type UnauthenticatedClientConfig = {
|
||||||
@@ -73,6 +63,7 @@ export type UnauthenticatedClientConfig = {
|
|||||||
globals: []
|
globals: []
|
||||||
routes: ClientConfig['routes']
|
routes: ClientConfig['routes']
|
||||||
serverURL: ClientConfig['serverURL']
|
serverURL: ClientConfig['serverURL']
|
||||||
|
unauthenticated: ClientConfig['unauthenticated']
|
||||||
}
|
}
|
||||||
|
|
||||||
export const serverOnlyAdminConfigProperties: readonly Partial<ServerOnlyRootAdminProperties>[] = []
|
export const serverOnlyAdminConfigProperties: readonly Partial<ServerOnlyRootAdminProperties>[] = []
|
||||||
@@ -132,6 +123,7 @@ export const createUnauthenticatedClientConfig = ({
|
|||||||
globals: [],
|
globals: [],
|
||||||
routes: clientConfig.routes,
|
routes: clientConfig.routes,
|
||||||
serverURL: clientConfig.serverURL,
|
serverURL: clientConfig.serverURL,
|
||||||
|
unauthenticated: true,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -189,6 +181,17 @@ export const createClientConfig = ({
|
|||||||
importMap,
|
importMap,
|
||||||
}).filter((block) => typeof block !== 'string') as ClientBlock[]
|
}).filter((block) => typeof block !== 'string') as ClientBlock[]
|
||||||
|
|
||||||
|
clientConfig.blocksMap = {}
|
||||||
|
if (clientConfig.blocks?.length) {
|
||||||
|
for (const block of clientConfig.blocks) {
|
||||||
|
if (!block?.slug) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
clientConfig.blocksMap[block.slug] = block as ClientBlock
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1274,7 +1274,6 @@ export {
|
|||||||
serverOnlyAdminConfigProperties,
|
serverOnlyAdminConfigProperties,
|
||||||
serverOnlyConfigProperties,
|
serverOnlyConfigProperties,
|
||||||
type UnauthenticatedClientConfig,
|
type UnauthenticatedClientConfig,
|
||||||
type UnsanitizedClientConfig,
|
|
||||||
} from './config/client.js'
|
} from './config/client.js'
|
||||||
export { defaults } from './config/defaults.js'
|
export { defaults } from './config/defaults.js'
|
||||||
|
|
||||||
|
|||||||
@@ -303,7 +303,7 @@ export {
|
|||||||
RouteTransitionProvider,
|
RouteTransitionProvider,
|
||||||
useRouteTransition,
|
useRouteTransition,
|
||||||
} from '../../providers/RouteTransition/index.js'
|
} from '../../providers/RouteTransition/index.js'
|
||||||
export { ConfigProvider, useConfig } from '../../providers/Config/index.js'
|
export { ConfigProvider, RootPageConfigProvider, useConfig } from '../../providers/Config/index.js'
|
||||||
export { DocumentEventsProvider, useDocumentEvents } from '../../providers/DocumentEvents/index.js'
|
export { DocumentEventsProvider, useDocumentEvents } from '../../providers/DocumentEvents/index.js'
|
||||||
export { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js'
|
export { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js'
|
||||||
export { useDocumentTitle } from '../../providers/DocumentTitle/index.js'
|
export { useDocumentTitle } from '../../providers/DocumentTitle/index.js'
|
||||||
|
|||||||
@@ -6,7 +6,6 @@ import type {
|
|||||||
ClientGlobalConfig,
|
ClientGlobalConfig,
|
||||||
CollectionSlug,
|
CollectionSlug,
|
||||||
GlobalSlug,
|
GlobalSlug,
|
||||||
UnsanitizedClientConfig,
|
|
||||||
} from 'payload'
|
} from 'payload'
|
||||||
|
|
||||||
import React, { createContext, use, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
import React, { createContext, use, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||||
@@ -40,36 +39,14 @@ export type ClientConfigContext = {
|
|||||||
|
|
||||||
const RootConfigContext = createContext<ClientConfigContext | undefined>(undefined)
|
const RootConfigContext = createContext<ClientConfigContext | undefined>(undefined)
|
||||||
|
|
||||||
function sanitizeClientConfig(
|
|
||||||
unSanitizedConfig: ClientConfig | UnsanitizedClientConfig,
|
|
||||||
): ClientConfig {
|
|
||||||
if (!unSanitizedConfig?.blocks?.length || (unSanitizedConfig as ClientConfig).blocksMap) {
|
|
||||||
;(unSanitizedConfig as ClientConfig).blocksMap = {}
|
|
||||||
return unSanitizedConfig as ClientConfig
|
|
||||||
}
|
|
||||||
const sanitizedConfig: ClientConfig = { ...unSanitizedConfig } as ClientConfig
|
|
||||||
|
|
||||||
sanitizedConfig.blocksMap = {}
|
|
||||||
|
|
||||||
for (const block of unSanitizedConfig.blocks) {
|
|
||||||
sanitizedConfig.blocksMap[block.slug] = block
|
|
||||||
}
|
|
||||||
|
|
||||||
return sanitizedConfig
|
|
||||||
}
|
|
||||||
|
|
||||||
export const ConfigProvider: React.FC<{
|
export const ConfigProvider: React.FC<{
|
||||||
readonly children: React.ReactNode
|
readonly children: React.ReactNode
|
||||||
readonly config: ClientConfig | UnsanitizedClientConfig
|
readonly config: ClientConfig
|
||||||
}> = ({ children, config: configFromProps }) => {
|
}> = ({ children, config: configFromProps }) => {
|
||||||
const [config, setConfigFn] = useState<ClientConfig>(() => sanitizeClientConfig(configFromProps))
|
const [config, setConfig] = useState<ClientConfig>(configFromProps)
|
||||||
|
|
||||||
const isFirstRenderRef = useRef(true)
|
const isFirstRenderRef = useRef(true)
|
||||||
|
|
||||||
const setConfig = useCallback((newConfig: ClientConfig | UnsanitizedClientConfig) => {
|
|
||||||
setConfigFn(sanitizeClientConfig(newConfig))
|
|
||||||
}, [])
|
|
||||||
|
|
||||||
// Need to update local config state if config from props changes, for HMR.
|
// Need to update local config state if config from props changes, for HMR.
|
||||||
// That way, config changes will be updated in the UI immediately without needing a refresh.
|
// That way, config changes will be updated in the UI immediately without needing a refresh.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
@@ -112,9 +89,52 @@ export const ConfigProvider: React.FC<{
|
|||||||
[collectionsBySlug, globalsBySlug],
|
[collectionsBySlug, globalsBySlug],
|
||||||
)
|
)
|
||||||
|
|
||||||
const value = useMemo(() => ({ config, getEntityConfig, setConfig }), [config, getEntityConfig])
|
const value = useMemo(
|
||||||
|
() => ({ config, getEntityConfig, setConfig }),
|
||||||
|
[config, getEntityConfig, setConfig],
|
||||||
|
)
|
||||||
|
|
||||||
return <RootConfigContext value={value}>{children}</RootConfigContext>
|
return <RootConfigContext value={value}>{children}</RootConfigContext>
|
||||||
}
|
}
|
||||||
|
|
||||||
export const useConfig = (): ClientConfigContext => use(RootConfigContext)
|
export const useConfig = (): ClientConfigContext => use(RootConfigContext)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* This provider shadows the ConfigProvider on the _page_ level, allowing us to
|
||||||
|
* update the config when needed, e.g. after authentication.
|
||||||
|
* The layout ConfigProvider is not updated on page navigation / authentication,
|
||||||
|
* as the layout does not re-render in those cases.
|
||||||
|
*
|
||||||
|
* If the config here has the same reference as the config from the layout, we
|
||||||
|
* simply reuse the context from the layout to avoid unnecessary re-renders.
|
||||||
|
*/
|
||||||
|
export const RootPageConfigProvider: React.FC<{
|
||||||
|
readonly children: React.ReactNode
|
||||||
|
readonly config: ClientConfig
|
||||||
|
}> = ({ children, config: configFromProps }) => {
|
||||||
|
const rootLayoutConfig = useConfig()
|
||||||
|
const { config, getEntityConfig, setConfig } = rootLayoutConfig
|
||||||
|
|
||||||
|
/**
|
||||||
|
* This useEffect is required in order for the _page_ to be able to refresh the client config,
|
||||||
|
* which may have been cached on the _layout_ level, where the ConfigProvider is managed.
|
||||||
|
* Since the layout does not re-render on page navigation / authentication, we need to manually
|
||||||
|
* update the config, as the user may have been authenticated in the process, which affects the client config.
|
||||||
|
*/
|
||||||
|
useEffect(() => {
|
||||||
|
setConfig(configFromProps)
|
||||||
|
}, [configFromProps, setConfig])
|
||||||
|
|
||||||
|
if (config !== configFromProps && config.unauthenticated !== configFromProps.unauthenticated) {
|
||||||
|
// Between the unauthenticated config becoming authenticated (or the other way around) and the useEffect
|
||||||
|
// running, the config will be stale. In order to avoid having the wrong config in the context in that
|
||||||
|
// brief moment, we shadow the context here on the _page_ level and provide the updated config immediately.
|
||||||
|
return (
|
||||||
|
<RootConfigContext value={{ config: configFromProps, getEntityConfig, setConfig }}>
|
||||||
|
{children}
|
||||||
|
</RootConfigContext>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return <RootConfigContext value={rootLayoutConfig}>{children}</RootConfigContext>
|
||||||
|
}
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ describe('Auth', () => {
|
|||||||
page = await context.newPage()
|
page = await context.newPage()
|
||||||
initPageConsoleErrorCatch(page)
|
initPageConsoleErrorCatch(page)
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('create first user', () => {
|
describe('create first user', () => {
|
||||||
beforeAll(async () => {
|
beforeAll(async () => {
|
||||||
await reInitializeDB({
|
await reInitializeDB({
|
||||||
@@ -386,6 +387,36 @@ describe('Auth', () => {
|
|||||||
|
|
||||||
await saveDocAndAssert(page)
|
await saveDocAndAssert(page)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('ensure login page with redirect to users document redirects properly after login, without client error', async () => {
|
||||||
|
await page.goto(url.admin)
|
||||||
|
|
||||||
|
await page.goto(`${serverURL}/admin/logout`)
|
||||||
|
|
||||||
|
await expect(page.locator('.login')).toBeVisible()
|
||||||
|
|
||||||
|
const users = await payload.find({
|
||||||
|
collection: slug,
|
||||||
|
limit: 1,
|
||||||
|
})
|
||||||
|
const userDocumentRoute = `${serverURL}/admin/collections/users/${users?.docs?.[0]?.id}`
|
||||||
|
|
||||||
|
await page.goto(userDocumentRoute)
|
||||||
|
|
||||||
|
await expect(page.locator('#field-email')).toBeVisible()
|
||||||
|
await expect(page.locator('#field-password')).toBeVisible()
|
||||||
|
|
||||||
|
await page.locator('.form-submit > button').click()
|
||||||
|
|
||||||
|
// Expect to be redirected to the correct page
|
||||||
|
await expect
|
||||||
|
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
|
||||||
|
.toBe(userDocumentRoute)
|
||||||
|
|
||||||
|
// Previously, this would crash the page with a "Cannot read properties of undefined (reading 'match')" error
|
||||||
|
|
||||||
|
await expect(page.locator('#field-roles')).toBeVisible()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user