From a78bc6c65e695e07d9574b6aeb5603a1bc4a7692 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Wed, 8 Jan 2025 23:57:42 -0500 Subject: [PATCH] fix(next): properly instantiates locale context (#10451) Currently, unless a locale is present in the URL search params, the locale context is instantiated using the default locale until prefs load in client-side. This causes the locale selector to briefly render in with the incorrect (default) locale before being replaced by the proper locale of the request. For example, if the default locale is `en`, and the page is requested in `es`, the locale selector will flash with English before changing to the correct locale, even though the page data itself is properly loaded in Spanish. This is especially evident within slow networks. The fix is to query the user's locale preference server-side and thread it into the locale provider to initialize state. Because search params are not available within server layouts, we cannot pass the locale param in the same way, so we rely on the provider itself to read them from the `useSearchParams` hook. If present, this takes precedence over the user's preference if it exists. Since the root page also queries the user's locale preference to determine the proper locale across navigation, we use React's cache function to dedupe these function calls and ensure only a single query is made to the db for each request. --- packages/next/src/layouts/Root/index.tsx | 7 ++ packages/next/src/utilities/getPreference.ts | 41 +++++++++ .../next/src/utilities/getRequestLocale.ts | 32 +++++++ .../next/src/utilities/getRequestLocales.ts | 45 --------- packages/next/src/utilities/initPage/index.ts | 56 ++--------- packages/next/src/views/Root/index.tsx | 1 + .../src/preferences/requestHandlers/update.ts | 2 +- packages/ui/src/providers/Locale/index.tsx | 12 ++- packages/ui/src/providers/Root/index.tsx | 5 +- test/fields/collections/Text/e2e.spec.ts | 1 + test/helpers/e2e/upsertPrefs.ts | 92 +++++++++++-------- test/localization/e2e.spec.ts | 29 ++++++ tsconfig.base.json | 2 +- 13 files changed, 186 insertions(+), 139 deletions(-) create mode 100644 packages/next/src/utilities/getPreference.ts create mode 100644 packages/next/src/utilities/getRequestLocale.ts delete mode 100644 packages/next/src/utilities/getRequestLocales.ts diff --git a/packages/next/src/layouts/Root/index.tsx b/packages/next/src/layouts/Root/index.tsx index 32e45e782..b6ed4287f 100644 --- a/packages/next/src/layouts/Root/index.tsx +++ b/packages/next/src/layouts/Root/index.tsx @@ -10,6 +10,7 @@ import React from 'react' import { getNavPrefs } from '../../elements/Nav/getNavPrefs.js' import { getRequestLanguage } from '../../utilities/getRequestLanguage.js' +import { getRequestLocale } from '../../utilities/getRequestLocale.js' import { getRequestTheme } from '../../utilities/getRequestTheme.js' import { initReq } from '../../utilities/initReq.js' import { checkDependencies } from './checkDependencies.js' @@ -92,6 +93,11 @@ export const RootLayout = async ({ importMap, }) + const locale = await getRequestLocale({ + payload, + user, + }) + return ( (key: string, payload: Payload, user: User): Promise => { + let result: T = null + + try { + result = await payload + .find({ + collection: 'payload-preferences', + depth: 0, + limit: 1, + user, + where: { + and: [ + { + 'user.relationTo': { + equals: payload.config.admin.user, + }, + }, + { + 'user.value': { + equals: user.id, + }, + }, + { + key: { + equals: key, + }, + }, + ], + }, + }) + ?.then((res) => res.docs?.[0]?.value as T) + } catch (_err) {} // eslint-disable-line no-empty + + return result + }, +) diff --git a/packages/next/src/utilities/getRequestLocale.ts b/packages/next/src/utilities/getRequestLocale.ts new file mode 100644 index 000000000..8c65ad435 --- /dev/null +++ b/packages/next/src/utilities/getRequestLocale.ts @@ -0,0 +1,32 @@ +import type { Locale, Payload, User } from 'payload' + +import { findLocaleFromCode } from '@payloadcms/ui/shared' + +import { getPreference } from './getPreference.js' + +type GetRequestLocalesArgs = { + localeFromParams?: string + payload: Payload + user: User +} + +export async function getRequestLocale({ + localeFromParams, + payload, + user, +}: GetRequestLocalesArgs): Promise { + if (payload.config.localization) { + return ( + findLocaleFromCode( + payload.config.localization, + localeFromParams || (await getPreference('locale', payload, user)), + ) || + findLocaleFromCode( + payload.config.localization, + payload.config.localization.defaultLocale || 'en', + ) + ) + } + + return undefined +} diff --git a/packages/next/src/utilities/getRequestLocales.ts b/packages/next/src/utilities/getRequestLocales.ts deleted file mode 100644 index 35ed73f80..000000000 --- a/packages/next/src/utilities/getRequestLocales.ts +++ /dev/null @@ -1,45 +0,0 @@ -import type { Payload } from 'payload' - -import { sanitizeFallbackLocale } from 'payload' - -type GetRequestLocalesArgs = { - data?: Record - localization: Exclude - searchParams?: URLSearchParams -} -export function getRequestLocales({ data, localization, searchParams }: GetRequestLocalesArgs): { - fallbackLocale: string - locale: string -} { - let locale = searchParams.get('locale') - let fallbackLocale = searchParams.get('fallback-locale') || searchParams.get('fallbackLocale') - - if (data) { - if (data?.locale) { - locale = data.locale - } - if (data?.['fallback-locale']) { - fallbackLocale = data['fallback-locale'] - } - if (data?.['fallbackLocale']) { - fallbackLocale = data['fallbackLocale'] - } - } - - fallbackLocale = sanitizeFallbackLocale({ - fallbackLocale, - locale, - localization, - }) - - if (locale === '*') { - locale = 'all' - } else if (!localization.localeCodes.includes(locale)) { - locale = localization.defaultLocale - } - - return { - fallbackLocale, - locale, - } -} diff --git a/packages/next/src/utilities/initPage/index.ts b/packages/next/src/utilities/initPage/index.ts index 5aa885901..72a907328 100644 --- a/packages/next/src/utilities/initPage/index.ts +++ b/packages/next/src/utilities/initPage/index.ts @@ -1,7 +1,6 @@ import type { I18n } from '@payloadcms/translations' -import type { InitPageResult, Locale, VisibleEntities } from 'payload' +import type { InitPageResult, VisibleEntities } from 'payload' -import { findLocaleFromCode } from '@payloadcms/ui/shared' import { headers as getHeaders } from 'next/headers.js' import { notFound } from 'next/navigation.js' import { createLocalReq, getPayload, isEntityHidden, parseCookies } from 'payload' @@ -9,6 +8,7 @@ import * as qs from 'qs-esm' import type { Args } from './types.js' +import { getRequestLocale } from '../getRequestLocale.js' import { initReq } from '../initReq.js' import { getRouteInfo } from './handleAdminPage.js' import { handleAuthRedirect } from './handleAuthRedirect.js' @@ -28,7 +28,6 @@ export const initPage = async ({ const { collections, globals, - localization, routes: { admin: adminRoute }, } = payload.config @@ -74,52 +73,13 @@ export const initPage = async ({ req.user = user - const localeParam = searchParams?.locale as string - let locale: Locale + const locale = await getRequestLocale({ + localeFromParams: req.query.locale as string, + payload, + user, + }) - if (localization) { - const defaultLocaleCode = localization.defaultLocale ? localization.defaultLocale : 'en' - let localeCode: string = localeParam - - if (!localeCode) { - try { - localeCode = await payload - .find({ - collection: 'payload-preferences', - depth: 0, - limit: 1, - user, - where: { - and: [ - { - 'user.relationTo': { - equals: payload.config.admin.user, - }, - }, - { - 'user.value': { - equals: user.id, - }, - }, - { - key: { - equals: 'locale', - }, - }, - ], - }, - }) - ?.then((res) => res.docs?.[0]?.value as string) - } catch (_err) {} // eslint-disable-line no-empty - } - - locale = findLocaleFromCode(localization, localeCode) - - if (!locale) { - locale = findLocaleFromCode(localization, defaultLocaleCode) - } - req.locale = locale.code - } + req.locale = locale?.code const visibleEntities: VisibleEntities = { collections: collections diff --git a/packages/next/src/views/Root/index.tsx b/packages/next/src/views/Root/index.tsx index 24a0b6732..0685e8e1c 100644 --- a/packages/next/src/views/Root/index.tsx +++ b/packages/next/src/views/Root/index.tsx @@ -48,6 +48,7 @@ export const RootPage = async ({ } = config const params = await paramsPromise + const currentRoute = formatAdminURL({ adminRoute, path: `${Array.isArray(params.segments) ? `/${params.segments.join('/')}` : ''}`, diff --git a/packages/payload/src/preferences/requestHandlers/update.ts b/packages/payload/src/preferences/requestHandlers/update.ts index 8ff33b584..b1a9dd50c 100644 --- a/packages/payload/src/preferences/requestHandlers/update.ts +++ b/packages/payload/src/preferences/requestHandlers/update.ts @@ -12,7 +12,7 @@ export const updateHandler: PayloadHandler = async (incomingReq) => { try { data = await incomingReq.json() - } catch (error) { + } catch (_err) { data = {} } diff --git a/packages/ui/src/providers/Locale/index.tsx b/packages/ui/src/providers/Locale/index.tsx index 3b0cfcb6f..dc9c526c1 100644 --- a/packages/ui/src/providers/Locale/index.tsx +++ b/packages/ui/src/providers/Locale/index.tsx @@ -12,20 +12,24 @@ import { usePreferences } from '../Preferences/index.js' const LocaleContext = createContext({} as Locale) -export const LocaleProvider: React.FC<{ children?: React.ReactNode }> = ({ children }) => { +export const LocaleProvider: React.FC<{ children?: React.ReactNode; locale?: Locale['code'] }> = ({ + children, + locale: localeFromProps, +}) => { const { config: { localization = false }, } = useConfig() const { user } = useAuth() + const defaultLocale = localization && localization.defaultLocale ? localization.defaultLocale : 'en' const { getPreference, setPreference } = usePreferences() - const searchParams = useSearchParams() - const localeFromParams = searchParams.get('locale') + const localeFromParams = useSearchParams().get('locale') - const [localeCode, setLocaleCode] = useState(defaultLocale) + // `localeFromProps` originates from the root layout, which does not have access to search params + const [localeCode, setLocaleCode] = useState(localeFromProps || localeFromParams) const locale: Locale = React.useMemo(() => { if (!localization) { diff --git a/packages/ui/src/providers/Root/index.tsx b/packages/ui/src/providers/Root/index.tsx index 62a680a99..2ff2bcc58 100644 --- a/packages/ui/src/providers/Root/index.tsx +++ b/packages/ui/src/providers/Root/index.tsx @@ -3,6 +3,7 @@ import type { I18nClient, Language } from '@payloadcms/translations' import type { ClientConfig, LanguageOptions, + Locale, SanitizedPermissions, ServerFunctionClient, User, @@ -41,6 +42,7 @@ type Props = { readonly isNavOpen?: boolean readonly languageCode: string readonly languageOptions: LanguageOptions + readonly locale?: Locale['code'] readonly permissions: SanitizedPermissions readonly serverFunction: ServerFunctionClient readonly switchLanguageServerAction?: (lang: string) => Promise @@ -57,6 +59,7 @@ export const RootProvider: React.FC = ({ isNavOpen, languageCode, languageOptions, + locale, permissions, serverFunction, switchLanguageServerAction, @@ -96,7 +99,7 @@ export const RootProvider: React.FC = ({ - + diff --git a/test/fields/collections/Text/e2e.spec.ts b/test/fields/collections/Text/e2e.spec.ts index c610dd90a..418888e82 100644 --- a/test/fields/collections/Text/e2e.spec.ts +++ b/test/fields/collections/Text/e2e.spec.ts @@ -168,6 +168,7 @@ describe('Text', () => { await upsertPrefs>({ payload, user: client.user, + key: 'text-fields-list', value: { columns: [ { diff --git a/test/helpers/e2e/upsertPrefs.ts b/test/helpers/e2e/upsertPrefs.ts index dd92add2a..dc6254ace 100644 --- a/test/helpers/e2e/upsertPrefs.ts +++ b/test/helpers/e2e/upsertPrefs.ts @@ -9,51 +9,65 @@ export const upsertPrefs = async < payload, user, value, + key, }: { + key: string payload: PayloadTestSDK user: TypedUser - value: Record + value: any }): Promise => { - let prefs = await payload - .find({ - collection: 'payload-preferences', - depth: 0, - limit: 1, - where: { - and: [ - { key: { equals: 'text-fields-list' } }, - { 'user.value': { equals: user.id } }, - { 'user.relationTo': { equals: user.collection } }, - ], - }, - }) - ?.then((res) => res.docs?.[0]) + try { + let prefs = await payload + .find({ + collection: 'payload-preferences', + depth: 0, + limit: 1, + where: { + and: [ + { key: { equals: key } }, + { 'user.value': { equals: user.id } }, + { 'user.relationTo': { equals: user.collection } }, + ], + }, + }) + ?.then((res) => res.docs?.[0]) - if (!prefs) { - prefs = await payload.create({ - collection: 'payload-preferences', - depth: 0, - data: { - key: 'text-fields-list', - user: { - relationTo: user.collection, - value: user.id, + if (!prefs) { + prefs = await payload.create({ + collection: 'payload-preferences', + depth: 0, + data: { + key, + user: { + relationTo: user.collection, + value: user.id, + }, + value, }, - value, - }, - }) - } else { - prefs = await payload.update({ - collection: 'payload-preferences', - id: prefs.id, - data: { - value: { - ...(prefs?.value ?? {}), - ...value, + }) + } else { + const newValue = typeof value === 'object' ? { ...(prefs?.value || {}), ...value } : value + + prefs = await payload.update({ + collection: 'payload-preferences', + id: prefs.id, + data: { + key, + user: { + collection: user.collection, + value: user.id, + }, + value: newValue, }, - }, - }) + }) + + if (prefs?.status >= 400) { + throw new Error(prefs.data?.errors?.[0]?.message) + } + + return prefs + } + } catch (e) { + console.error('Error upserting prefs', e) } - - return prefs } diff --git a/test/localization/e2e.spec.ts b/test/localization/e2e.spec.ts index a72fd0090..8de1c0378 100644 --- a/test/localization/e2e.spec.ts +++ b/test/localization/e2e.spec.ts @@ -30,6 +30,9 @@ import { withRequiredLocalizedFields, } from './shared.js' import { navigateToDoc } from 'helpers/e2e/navigateToDoc.js' +import { upsertPrefs } from 'helpers/e2e/upsertPrefs.js' +import { RESTClient } from 'helpers/rest.js' +import { GeneratedTypes } from 'helpers/sdk/types.js' const filename = fileURLToPath(import.meta.url) const dirname = path.dirname(filename) @@ -54,6 +57,7 @@ const description = 'description' let page: Page let payload: PayloadTestSDK +let client: RESTClient let serverURL: string let richTextURL: AdminUrlUtil let context: BrowserContext @@ -73,6 +77,9 @@ describe('Localization', () => { initPageConsoleErrorCatch(page) + client = new RESTClient(null, { defaultSlug: 'users', serverURL }) + await client.login() + await ensureCompilationIsDone({ page, serverURL }) }) @@ -84,8 +91,30 @@ describe('Localization', () => { // }) }) + describe('localizer', async () => { + test('should not render default locale in locale selector when prefs are not default', async () => { + // change prefs to spanish, then load page and check that the localizer label does not say English + await upsertPrefs>({ + payload, + user: client.user, + key: 'locale', + value: 'es', + }) + + await page.goto(url.list) + + const localeLabel = await page + .locator('.localizer.app-header__localizer .localizer-button__current-label') + .innerText() + + expect(localeLabel).not.toEqual('English') + }) + }) + describe('localized text', () => { test('create english post, switch to spanish', async () => { + await changeLocale(page, defaultLocale) + await page.goto(url.create) await fillValues({ description, title }) diff --git a/tsconfig.base.json b/tsconfig.base.json index 872fb4ed3..9743db764 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -28,7 +28,7 @@ } ], "paths": { - "@payload-config": ["./test/admin/config.ts"], + "@payload-config": ["./test/localization/config.ts"], "@payloadcms/live-preview": ["./packages/live-preview/src"], "@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"], "@payloadcms/live-preview-vue": ["./packages/live-preview-vue/src/index.ts"],