Skip to content

Commit 13af91f

Browse files
authored
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
1 parent 8a7124a commit 13af91f

File tree

7 files changed

+95
-64
lines changed

7 files changed

+95
-64
lines changed

packages/next/src/views/Root/SyncClientConfig.tsx

Lines changed: 0 additions & 21 deletions
This file was deleted.

packages/next/src/views/Root/index.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
SanitizedGlobalConfig,
1111
} from 'payload'
1212

13+
import { RootPageConfigProvider } from '@payloadcms/ui'
1314
import { RenderServerComponent } from '@payloadcms/ui/elements/RenderServerComponent'
1415
import { getClientConfig } from '@payloadcms/ui/utilities/getClientConfig'
1516
import { notFound, redirect } from 'next/navigation.js'
@@ -27,7 +28,6 @@ import { isCustomAdminView } from '../../utilities/isCustomAdminView.js'
2728
import { isPublicAdminRoute } from '../../utilities/isPublicAdminRoute.js'
2829
import { getCustomViewByRoute } from './getCustomViewByRoute.js'
2930
import { getRouteData } from './getRouteData.js'
30-
import { SyncClientConfig } from './SyncClientConfig.js'
3131

3232
export type GenerateViewMetadata = (args: {
3333
config: SanitizedConfig
@@ -300,8 +300,7 @@ export const RootPage = async ({
300300
})
301301

302302
return (
303-
<React.Fragment>
304-
<SyncClientConfig clientConfig={clientConfig} />
303+
<RootPageConfigProvider config={clientConfig}>
305304
{!templateType && <React.Fragment>{RenderedView}</React.Fragment>}
306305
{templateType === 'minimal' && (
307306
<MinimalTemplate className={templateClassName}>{RenderedView}</MinimalTemplate>
@@ -332,6 +331,6 @@ export const RootPage = async ({
332331
{RenderedView}
333332
</DefaultTemplate>
334333
)}
335-
</React.Fragment>
334+
</RootPageConfigProvider>
336335
)
337336
}

packages/payload/src/config/client.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import type {
1212

1313
import {
1414
type ClientCollectionConfig,
15-
createClientCollectionConfig,
1615
createClientCollectionConfigs,
1716
} from '../collections/config/client.js'
1817
import { createClientBlocks } from '../fields/config/client.js'
@@ -43,16 +42,6 @@ export type ServerOnlyRootProperties = keyof Pick<
4342

4443
export type ServerOnlyRootAdminProperties = keyof Pick<SanitizedConfig['admin'], 'components'>
4544

46-
export type UnsanitizedClientConfig = {
47-
admin: {
48-
livePreview?: Omit<RootLivePreviewConfig, ServerOnlyLivePreviewProperties>
49-
} & Omit<SanitizedConfig['admin'], 'components' | 'dependencies' | 'livePreview'>
50-
blocks: ClientBlock[]
51-
collections: ClientCollectionConfig[]
52-
custom?: Record<string, any>
53-
globals: ClientGlobalConfig[]
54-
} & Omit<SanitizedConfig, 'admin' | 'collections' | 'globals' | 'i18n' | ServerOnlyRootProperties>
55-
5645
export type ClientConfig = {
5746
admin: {
5847
livePreview?: Omit<RootLivePreviewConfig, ServerOnlyLivePreviewProperties>
@@ -62,6 +51,7 @@ export type ClientConfig = {
6251
collections: ClientCollectionConfig[]
6352
custom?: Record<string, any>
6453
globals: ClientGlobalConfig[]
54+
unauthenticated?: boolean
6555
} & Omit<SanitizedConfig, 'admin' | 'collections' | 'globals' | 'i18n' | ServerOnlyRootProperties>
6656

6757
export type UnauthenticatedClientConfig = {
@@ -73,6 +63,7 @@ export type UnauthenticatedClientConfig = {
7363
globals: []
7464
routes: ClientConfig['routes']
7565
serverURL: ClientConfig['serverURL']
66+
unauthenticated: ClientConfig['unauthenticated']
7667
}
7768

7869
export const serverOnlyAdminConfigProperties: readonly Partial<ServerOnlyRootAdminProperties>[] = []
@@ -132,6 +123,7 @@ export const createUnauthenticatedClientConfig = ({
132123
globals: [],
133124
routes: clientConfig.routes,
134125
serverURL: clientConfig.serverURL,
126+
unauthenticated: true,
135127
}
136128
}
137129

@@ -189,6 +181,17 @@ export const createClientConfig = ({
189181
importMap,
190182
}).filter((block) => typeof block !== 'string') as ClientBlock[]
191183

184+
clientConfig.blocksMap = {}
185+
if (clientConfig.blocks?.length) {
186+
for (const block of clientConfig.blocks) {
187+
if (!block?.slug) {
188+
continue
189+
}
190+
191+
clientConfig.blocksMap[block.slug] = block as ClientBlock
192+
}
193+
}
194+
192195
break
193196
}
194197

packages/payload/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,6 @@ export {
12741274
serverOnlyAdminConfigProperties,
12751275
serverOnlyConfigProperties,
12761276
type UnauthenticatedClientConfig,
1277-
type UnsanitizedClientConfig,
12781277
} from './config/client.js'
12791278
export { defaults } from './config/defaults.js'
12801279

packages/ui/src/exports/client/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export {
303303
RouteTransitionProvider,
304304
useRouteTransition,
305305
} from '../../providers/RouteTransition/index.js'
306-
export { ConfigProvider, useConfig } from '../../providers/Config/index.js'
306+
export { ConfigProvider, RootPageConfigProvider, useConfig } from '../../providers/Config/index.js'
307307
export { DocumentEventsProvider, useDocumentEvents } from '../../providers/DocumentEvents/index.js'
308308
export { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js'
309309
export { useDocumentTitle } from '../../providers/DocumentTitle/index.js'

packages/ui/src/providers/Config/index.tsx

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import type {
66
ClientGlobalConfig,
77
CollectionSlug,
88
GlobalSlug,
9-
UnsanitizedClientConfig,
109
} from 'payload'
1110

1211
import React, { createContext, use, useCallback, useEffect, useMemo, useRef, useState } from 'react'
@@ -40,36 +39,14 @@ export type ClientConfigContext = {
4039

4140
const RootConfigContext = createContext<ClientConfigContext | undefined>(undefined)
4241

43-
function sanitizeClientConfig(
44-
unSanitizedConfig: ClientConfig | UnsanitizedClientConfig,
45-
): ClientConfig {
46-
if (!unSanitizedConfig?.blocks?.length || (unSanitizedConfig as ClientConfig).blocksMap) {
47-
;(unSanitizedConfig as ClientConfig).blocksMap = {}
48-
return unSanitizedConfig as ClientConfig
49-
}
50-
const sanitizedConfig: ClientConfig = { ...unSanitizedConfig } as ClientConfig
51-
52-
sanitizedConfig.blocksMap = {}
53-
54-
for (const block of unSanitizedConfig.blocks) {
55-
sanitizedConfig.blocksMap[block.slug] = block
56-
}
57-
58-
return sanitizedConfig
59-
}
60-
6142
export const ConfigProvider: React.FC<{
6243
readonly children: React.ReactNode
63-
readonly config: ClientConfig | UnsanitizedClientConfig
44+
readonly config: ClientConfig
6445
}> = ({ children, config: configFromProps }) => {
65-
const [config, setConfigFn] = useState<ClientConfig>(() => sanitizeClientConfig(configFromProps))
46+
const [config, setConfig] = useState<ClientConfig>(configFromProps)
6647

6748
const isFirstRenderRef = useRef(true)
6849

69-
const setConfig = useCallback((newConfig: ClientConfig | UnsanitizedClientConfig) => {
70-
setConfigFn(sanitizeClientConfig(newConfig))
71-
}, [])
72-
7350
// Need to update local config state if config from props changes, for HMR.
7451
// That way, config changes will be updated in the UI immediately without needing a refresh.
7552
useEffect(() => {
@@ -112,9 +89,52 @@ export const ConfigProvider: React.FC<{
11289
[collectionsBySlug, globalsBySlug],
11390
)
11491

115-
const value = useMemo(() => ({ config, getEntityConfig, setConfig }), [config, getEntityConfig])
92+
const value = useMemo(
93+
() => ({ config, getEntityConfig, setConfig }),
94+
[config, getEntityConfig, setConfig],
95+
)
11696

11797
return <RootConfigContext value={value}>{children}</RootConfigContext>
11898
}
11999

120100
export const useConfig = (): ClientConfigContext => use(RootConfigContext)
101+
102+
/**
103+
* This provider shadows the ConfigProvider on the _page_ level, allowing us to
104+
* update the config when needed, e.g. after authentication.
105+
* The layout ConfigProvider is not updated on page navigation / authentication,
106+
* as the layout does not re-render in those cases.
107+
*
108+
* If the config here has the same reference as the config from the layout, we
109+
* simply reuse the context from the layout to avoid unnecessary re-renders.
110+
*/
111+
export const RootPageConfigProvider: React.FC<{
112+
readonly children: React.ReactNode
113+
readonly config: ClientConfig
114+
}> = ({ children, config: configFromProps }) => {
115+
const rootLayoutConfig = useConfig()
116+
const { config, getEntityConfig, setConfig } = rootLayoutConfig
117+
118+
/**
119+
* This useEffect is required in order for the _page_ to be able to refresh the client config,
120+
* which may have been cached on the _layout_ level, where the ConfigProvider is managed.
121+
* Since the layout does not re-render on page navigation / authentication, we need to manually
122+
* update the config, as the user may have been authenticated in the process, which affects the client config.
123+
*/
124+
useEffect(() => {
125+
setConfig(configFromProps)
126+
}, [configFromProps, setConfig])
127+
128+
if (config !== configFromProps && config.unauthenticated !== configFromProps.unauthenticated) {
129+
// Between the unauthenticated config becoming authenticated (or the other way around) and the useEffect
130+
// running, the config will be stale. In order to avoid having the wrong config in the context in that
131+
// brief moment, we shadow the context here on the _page_ level and provide the updated config immediately.
132+
return (
133+
<RootConfigContext value={{ config: configFromProps, getEntityConfig, setConfig }}>
134+
{children}
135+
</RootConfigContext>
136+
)
137+
}
138+
139+
return <RootConfigContext value={rootLayoutConfig}>{children}</RootConfigContext>
140+
}

test/auth/e2e.spec.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ describe('Auth', () => {
5252
page = await context.newPage()
5353
initPageConsoleErrorCatch(page)
5454
})
55+
5556
describe('create first user', () => {
5657
beforeAll(async () => {
5758
await reInitializeDB({
@@ -386,6 +387,36 @@ describe('Auth', () => {
386387

387388
await saveDocAndAssert(page)
388389
})
390+
391+
test('ensure login page with redirect to users document redirects properly after login, without client error', async () => {
392+
await page.goto(url.admin)
393+
394+
await page.goto(`${serverURL}/admin/logout`)
395+
396+
await expect(page.locator('.login')).toBeVisible()
397+
398+
const users = await payload.find({
399+
collection: slug,
400+
limit: 1,
401+
})
402+
const userDocumentRoute = `${serverURL}/admin/collections/users/${users?.docs?.[0]?.id}`
403+
404+
await page.goto(userDocumentRoute)
405+
406+
await expect(page.locator('#field-email')).toBeVisible()
407+
await expect(page.locator('#field-password')).toBeVisible()
408+
409+
await page.locator('.form-submit > button').click()
410+
411+
// Expect to be redirected to the correct page
412+
await expect
413+
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
414+
.toBe(userDocumentRoute)
415+
416+
// Previously, this would crash the page with a "Cannot read properties of undefined (reading 'match')" error
417+
418+
await expect(page.locator('#field-roles')).toBeVisible()
419+
})
389420
})
390421
})
391422
})

0 commit comments

Comments
 (0)