Skip to content

Commit

Permalink
fix: Make sure cookie value stays up to date when the Next.js Router …
Browse files Browse the repository at this point in the history
…Cache is used (#790)

Fixes #786

If you frequently change the locale, Next.js will at some point use the
Router Cache to return a previous response immediately, not making a
request to the server. This has the side effect that no `set-cookie`
response header is returned that will update the locale cookie.

To address this, `next-intl` navigation APIs will now manually keep the
cookie in sync.

Note that a related issue described in #786 was addressed in Next.js
14.1, so upgrading both `next` and `next-intl` is recommended.
  • Loading branch information
amannn committed Jan 19, 2024
1 parent 8ce7df8 commit 977b973
Show file tree
Hide file tree
Showing 16 changed files with 216 additions and 64 deletions.
4 changes: 2 additions & 2 deletions examples/example-app-router-playground/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"scripts": {
"dev": "next dev",
"lint": "eslint src && tsc",
"test": "pnpm run test:playwright && pnpm run test:jest",
"test": "pnpm run test:playwright && pnpm run test:playwright:locale-prefix-never && pnpm run test:jest",
"test:playwright": "playwright test",
"test:playwright:watch": "chokidar 'tests/main.spec.ts' -c 'npm run test:playwright'",
"test:playwright:locale-prefix-never": "NEXT_PUBLIC_LOCALE_PREFIX=never pnpm build && NEXT_PUBLIC_LOCALE_PREFIX=never playwright test",
"test:jest": "jest",
"build": "next build",
"start": "next start"
Expand Down
4 changes: 4 additions & 0 deletions examples/example-app-router-playground/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ const PORT = process.env.CI ? 3003 : 3000;

const config: PlaywrightTestConfig = {
retries: process.env.CI ? 1 : 0,
testMatch:
process.env.NEXT_PUBLIC_LOCALE_PREFIX === 'never'
? 'locale-prefix-never.spec.ts'
: 'main.spec.ts',
testDir: './tests',
projects: [
{
Expand Down
3 changes: 2 additions & 1 deletion examples/example-app-router-playground/src/navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ export const defaultLocale = 'en';

export const locales = ['en', 'de', 'es'] as const;

export const localePrefix = 'as-needed';
export const localePrefix =
process.env.NEXT_PUBLIC_LOCALE_PREFIX === 'never' ? 'never' : 'as-needed';

export const pathnames = {
'/': '/',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {test as it, expect} from '@playwright/test';

it.describe.configure({mode: 'parallel'});

it('clears the router cache when changing the locale', async ({page}) => {
await page.goto('/');

async function expectDocumentLang(lang: string) {
await expect(page.locator(`html[lang="${lang}"]`)).toBeAttached();
}

await expectDocumentLang('en');

await page.getByRole('link', {name: 'Client page'}).click();
await expectDocumentLang('en');
await expect(page).toHaveURL('/client');
await expect(
page.getByText('This page hydrates on the client side.')
).toBeAttached();

await page.getByRole('link', {name: 'Go to home'}).click();
await expectDocumentLang('en');
await expect(page).toHaveURL('/');

await page.getByRole('link', {name: 'Switch to German'}).click();

await expectDocumentLang('de');

await page.getByRole('link', {name: 'Client-Seite'}).click();
await expectDocumentLang('de');
await expect(page).toHaveURL('/client');
await expect(
page.getByText('Dise Seite wird auf der Client-Seite initialisiert.')
).toBeAttached();
});
2 changes: 1 addition & 1 deletion examples/example-app-router/messages/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
},
"LocaleSwitcher": {
"label": "Sprache ändern",
"locale": "{locale, select, de {Deutsch} en {Englisch} other {Unbekannt}}"
"locale": "{locale, select, de {🇩🇪 Deutsch} en {🇺🇸 English} other {Unbekannt}}"
},
"Navigation": {
"home": "Start",
Expand Down
2 changes: 1 addition & 1 deletion examples/example-app-router/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
},
"LocaleSwitcher": {
"label": "Change language",
"locale": "{locale, select, de {German} en {English} other {Unknown}}"
"locale": "{locale, select, de {🇩🇪 Deutsch} en {🇺🇸 English} other {Unknown}}"
},
"Navigation": {
"home": "Home",
Expand Down
27 changes: 26 additions & 1 deletion examples/example-app-router/tests/main.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ it('handles i18n routing', async ({page}) => {
await expect(page).toHaveURL('/de');
await page
.getByRole('combobox', {name: 'Sprache ändern'})
.selectOption({label: 'Englisch'});
.selectOption({value: 'en'});

await expect(page).toHaveURL('/en');
page.getByRole('heading', {name: 'next-intl example'});
Expand Down Expand Up @@ -58,13 +58,38 @@ it('can be used to localize the page', async ({page}) => {
});

it('sets a cookie', async ({page}) => {
function getCookieValue() {
return page.evaluate(() => document.cookie);
}

const response = await page.goto('/en');
const value = await response?.headerValue('set-cookie');
expect(value).toContain('NEXT_LOCALE=en;');
expect(value).toContain('Path=/;');
expect(value).toContain('SameSite=strict');
expect(value).toContain('Max-Age=31536000;');
expect(value).toContain('Expires=');
expect(await getCookieValue()).toBe('NEXT_LOCALE=en');

await page
.getByRole('combobox', {name: 'Change language'})
.selectOption({value: 'de'});
await expect(page).toHaveURL('/de');
expect(await getCookieValue()).toBe('NEXT_LOCALE=de');

await page
.getByRole('combobox', {name: 'Sprache ändern'})
.selectOption({value: 'en'});
await expect(page).toHaveURL('/en');
expect(await getCookieValue()).toBe('NEXT_LOCALE=en');

// The Next.js Router cache kicks in here
// https://nextjs.org/docs/app/building-your-application/caching#router-cache
await page
.getByRole('combobox', {name: 'Change language'})
.selectOption({value: 'de'});
await expect(page).toHaveURL('/de');
expect(await getCookieValue()).toBe('NEXT_LOCALE=de');
});

it('serves a robots.txt', async ({page}) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/next-intl/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,19 @@
"size-limit": [
{
"path": "dist/production/index.react-client.js",
"limit": "12.855 KB"
"limit": "12.865 KB"
},
{
"path": "dist/production/index.react-server.js",
"limit": "14.15 KB"
},
{
"path": "dist/production/navigation.react-client.js",
"limit": "2.63 KB"
"limit": "2.825 KB"
},
{
"path": "dist/production/navigation.react-server.js",
"limit": "2.82 KB"
"limit": "2.935 KB"
},
{
"path": "dist/production/server.react-client.js",
Expand Down
11 changes: 8 additions & 3 deletions packages/next-intl/src/middleware/middleware.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {NextRequest, NextResponse} from 'next/server';
import {COOKIE_LOCALE_NAME, HEADER_LOCALE_NAME} from '../shared/constants';
import {
COOKIE_LOCALE_NAME,
COOKIE_MAX_AGE,
COOKIE_SAME_SITE,
HEADER_LOCALE_NAME
} from '../shared/constants';
import {AllLocales} from '../shared/types';
import {matchesPathname} from '../shared/utils';
import MiddlewareConfig, {
Expand Down Expand Up @@ -258,8 +263,8 @@ export default function createMiddleware<Locales extends AllLocales>(
if (hasOutdatedCookie) {
response.cookies.set(COOKIE_LOCALE_NAME, locale, {
path: request.nextUrl.basePath || undefined,
sameSite: 'strict',
maxAge: 31536000 // 1 year
sameSite: COOKIE_SAME_SITE,
maxAge: COOKIE_MAX_AGE
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ function ClientLink<Locales extends AllLocales>(
) {
const defaultLocale = useLocale();
const linkLocale = locale || defaultLocale;
return (
<BaseLink ref={ref} hrefLang={linkLocale} locale={linkLocale} {...rest} />
);
return <BaseLink ref={ref} locale={linkLocale} {...rest} />;
}

/**
Expand Down
73 changes: 32 additions & 41 deletions packages/next-intl/src/navigation/react-client/useBaseRouter.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {useRouter as useNextRouter} from 'next/navigation';
import {useRouter as useNextRouter, usePathname} from 'next/navigation';
import {useMemo} from 'react';
import useLocale from '../../react-client/useLocale';
import {AllLocales} from '../../shared/types';
import {localizeHref} from '../../shared/utils';
import syncLocaleCookie from '../shared/syncLocaleCookie';

type IntlNavigateOptions<Locales extends AllLocales> = {
locale?: Locales[number];
Expand Down Expand Up @@ -30,6 +31,7 @@ type IntlNavigateOptions<Locales extends AllLocales> = {
export default function useBaseRouter<Locales extends AllLocales>() {
const router = useNextRouter();
const locale = useLocale();
const pathname = usePathname();

return useMemo(() => {
function localize(href: string, nextLocale?: string) {
Expand All @@ -41,56 +43,45 @@ export default function useBaseRouter<Locales extends AllLocales>() {
);
}

return {
...router,
push(
function createHandler<
Options,
Fn extends (href: string, options?: Options) => void
>(fn: Fn) {
return function handler(
href: string,
options?: Parameters<typeof router.push>[1] &
IntlNavigateOptions<Locales>
) {
options?: Options & IntlNavigateOptions<Locales>
): void {
const {locale: nextLocale, ...rest} = options || {};

syncLocaleCookie(pathname, locale, nextLocale);

const args: [
href: string,
options?: Parameters<typeof router.push>[1]
] = [localize(href, nextLocale)];
if (Object.keys(rest).length > 0) {
args.push(rest);
}
return router.push(...args);
},

replace(
href: string,
options?: Parameters<typeof router.replace>[1] &
IntlNavigateOptions<Locales>
) {
const {locale: nextLocale, ...rest} = options || {};
const args: [
href: string,
options?: Parameters<typeof router.replace>[1]
] = [localize(href, nextLocale)];
if (Object.keys(rest).length > 0) {
args.push(rest);
}
return router.replace(...args);
},
// @ts-expect-error -- This is ok
return fn(...args);
};
}

prefetch(
href: string,
options?: Parameters<typeof router.prefetch>[1] &
IntlNavigateOptions<Locales>
) {
const {locale: nextLocale, ...rest} = options || {};
const args: [
href: string,
options?: Parameters<typeof router.prefetch>[1]
] = [localize(href, nextLocale)];
if (Object.keys(rest).length > 0) {
// @ts-expect-error TypeScript thinks `rest` can be an empty object
args.push(rest);
}
return router.prefetch(...args);
}
return {
...router,
push: createHandler<
Parameters<typeof router.push>[1],
typeof router.push
>(router.push),
replace: createHandler<
Parameters<typeof router.replace>[1],
typeof router.replace
>(router.replace),
prefetch: createHandler<
Parameters<typeof router.prefetch>[1],
typeof router.prefetch
>(router.prefetch)
};
}, [locale, router]);
}, [locale, pathname, router]);

This comment has been minimized.

Copy link
@ff-ali

ff-ali Feb 9, 2024

I believe this addition of pathname to these dependencies has potential to cause some issues.

Using a router method such as router.replace inside of a useEffect causes a change in pathname which in turn causes router.replace reference to change. With router.replace as a dependency in the useEffect this has potential to create an infinite loop.

Trivial example:

useEffect(() => {
  router.replace("/")
}, [router.replace])

Will create an issue as well but thought I would add this comment where the change was introduced 👍

}
25 changes: 22 additions & 3 deletions packages/next-intl/src/navigation/shared/BaseLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,25 @@

import NextLink from 'next/link';
import {usePathname} from 'next/navigation';
import React, {ComponentProps, forwardRef, useEffect, useState} from 'react';
import React, {
ComponentProps,
MouseEvent,
forwardRef,
useEffect,
useState
} from 'react';
import useLocale from '../../react-client/useLocale';
import {LocalePrefix} from '../../shared/types';
import {isLocalHref, localizeHref, prefixHref} from '../../shared/utils';
import syncLocaleCookie from './syncLocaleCookie';

type Props = Omit<ComponentProps<typeof NextLink>, 'locale'> & {
locale: string;
localePrefix?: LocalePrefix;
};

function BaseLink(
{href, locale, localePrefix, prefetch, ...rest}: Props,
{href, locale, localePrefix, onClick, prefetch, ...rest}: Props,
ref: Props['ref']
) {
// The types aren't entirely correct here. Outside of Next.js
Expand All @@ -39,6 +46,11 @@ function BaseLink(
: href
);

function onLinkClick(event: MouseEvent<HTMLAnchorElement>) {
syncLocaleCookie(pathname, defaultLocale, locale);
if (onClick) onClick(event);
}

useEffect(() => {
if (!pathname || localePrefix === 'never') return;

Expand All @@ -57,7 +69,14 @@ function BaseLink(
}

return (
<NextLink ref={ref} href={localizedHref} prefetch={prefetch} {...rest} />
<NextLink
ref={ref}
href={localizedHref}
hrefLang={isChangingLocale ? locale : undefined}
onClick={onLinkClick}
prefetch={prefetch}
{...rest}
/>
);
}

Expand Down
35 changes: 35 additions & 0 deletions packages/next-intl/src/navigation/shared/syncLocaleCookie.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {
COOKIE_LOCALE_NAME,
COOKIE_MAX_AGE,
COOKIE_SAME_SITE
} from '../../shared/constants';

/**
* We have to keep the cookie value in sync as Next.js might
* skip a request to the server due to its router cache.
* See https://github.com/amannn/next-intl/issues/786.
*/
export default function syncLocaleCookie(
pathname: string | null,
locale: string,
nextLocale?: string
) {
const isSwitchingLocale = nextLocale !== locale;

if (
!isSwitchingLocale ||
// Theoretical case, we always have a pathname in a real app,
// only not when running e.g. in a simulated test environment
!pathname
) {
return;
}

const basePath = window.location.pathname.replace(pathname, '');
const hasBasePath = basePath !== '';
const path = hasBasePath ? basePath : '/';

// Note that writing to `document.cookie` doesn't overwrite all
// cookies, but only the ones referenced via the name here.
document.cookie = `${COOKIE_LOCALE_NAME}=${nextLocale}; path=${path}; max-age=${COOKIE_MAX_AGE}; sameSite=${COOKIE_SAME_SITE}`;
}
4 changes: 4 additions & 0 deletions packages/next-intl/src/shared/constants.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Reuse the legacy cookie name
// https://nextjs.org/docs/advanced-features/i18n-routing#leveraging-the-next_locale-cookie
export const COOKIE_LOCALE_NAME = 'NEXT_LOCALE';
export const COOKIE_MAX_AGE = 31536000; // 1 year
export const COOKIE_SAME_SITE = 'strict';

export const COOKIE_BASE_PATH_NAME = 'NEXT_INTL_BASE_PATH';

// Should take precedence over the cookie
export const HEADER_LOCALE_NAME = 'X-NEXT-INTL-LOCALE';
Expand Down

2 comments on commit 977b973

@vercel
Copy link

@vercel vercel bot commented on 977b973 Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 977b973 Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

next-intl-docs – ./docs

next-intl-docs-git-main-next-intl.vercel.app
next-intl-docs-next-intl.vercel.app
next-intl-docs.vercel.app

Please sign in to comment.