Skip to content

Commit 5e6f511

Browse files
committedAug 26, 2024
fix i18n data pathname resolving (#68947)
### What When using middleware + i18n in pages router, and upon navigation to a dynamic route, the wrong locale would be served in `getServerSideProps`. ### Why The route resolver code handles detecting the initial locale by splitting the path and looking at the first non-empty index to determine which locale was set. However, it does this assuming it has a regular path name, and not a `/_next/data` URL. When middleware is present, the route resolver code hits the `middleware_next_data` branch, which doesn't have any logic to properly set the locale. This means that resolveRoutes will return the default locale rather than the locale from the path. In the non-middleware case, this would normally flow through `handleNextDataRequest` in base-server which has handling to infer i18n via `i18nProvider`. This branch is functionally very similar to what we're doing in `resolveRoutes` but it does so in a different way: it reconstructs the URL without the `/_next/data` information and then attaches locale information. However because `middleware_next_data` rewrites the pathname to the actual route (ie `/_next/data/development/foo.json` -> `/foo`), `handleNextDataRequest` won't handle that request since it's no longer a data request. It's strange to me that `handleNextDataRequest` and `middleware_next_data` are doing similar path normalization in completely different ways, but that was a deeper rabbit hole and simply removing the normalization logic in `resolveRoutes` causes other problems. ### How Since data requests that flow through this middleware matcher logic aren't going to be handled by `handleNextDataRequest`, I've updated `middleware_next_data` to perform the logic of attaching the locale information to the query. Initially I was going to do this for anything that calls `normalizeLocalePath` but it seems like other branches of `resolveRoutes` do it in the matcher function directly. Fixes #54217 Closes NDX-116

File tree

7 files changed

+128
-0
lines changed

7 files changed

+128
-0
lines changed
 

‎packages/next/src/server/lib/router-utils/resolve-routes.ts

+11
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,17 @@ export function getResolveRoutes(
391391
normalized = normalizers.postponed.normalize(normalized, true)
392392
}
393393

394+
if (config.i18n) {
395+
const curLocaleResult = normalizeLocalePath(
396+
normalized,
397+
config.i18n.locales
398+
)
399+
400+
if (curLocaleResult.detectedLocale) {
401+
parsedUrl.query.__nextLocale = curLocaleResult.detectedLocale
402+
}
403+
}
404+
394405
// If we updated the pathname, and it had a base path, re-add the
395406
// base path.
396407
if (updated) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { nextTestSetup } from 'e2e-utils'
2+
3+
describe('i18n-navigations-middleware', () => {
4+
const { next } = nextTestSetup({
5+
files: __dirname,
6+
})
7+
8+
it('should respect selected locale when navigating to a dynamic route', async () => {
9+
const browser = await next.browser('/')
10+
// change to "de" locale
11+
await browser.elementByCss("[href='/de']").click()
12+
const dynamicLink = await browser.waitForElementByCss(
13+
"[href='/de/dynamic/1']"
14+
)
15+
expect(await browser.elementById('current-locale').text()).toBe(
16+
'Current locale: de'
17+
)
18+
19+
// navigate to dynamic route
20+
await dynamicLink.click()
21+
22+
// the locale should still be "de"
23+
expect(await browser.elementById('dynamic-locale').text()).toBe(
24+
'Locale: de'
25+
)
26+
})
27+
28+
it('should respect selected locale when navigating to a static route', async () => {
29+
const browser = await next.browser('/')
30+
// change to "de" locale
31+
await browser.elementByCss("[href='/de']").click()
32+
const staticLink = await browser.waitForElementByCss("[href='/de/static']")
33+
expect(await browser.elementById('current-locale').text()).toBe(
34+
'Current locale: de'
35+
)
36+
37+
// navigate to static route
38+
await staticLink.click()
39+
40+
// the locale should still be "de"
41+
expect(await browser.elementById('static-locale').text()).toBe('Locale: de')
42+
})
43+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { NextResponse } from 'next/server'
2+
3+
export const config = { matcher: ['/foo'] }
4+
export async function middleware(req) {
5+
return NextResponse.next()
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/**
2+
* @type {import('next').NextConfig}
3+
*/
4+
module.exports = {
5+
i18n: {
6+
defaultLocale: 'default',
7+
locales: ['default', 'en', 'de'],
8+
},
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export const getServerSideProps = async ({ locale }) => {
2+
return {
3+
props: {
4+
locale,
5+
},
6+
}
7+
}
8+
9+
export default function Dynamic({ locale }) {
10+
return <div id="dynamic-locale">Locale: {locale}</div>
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import Link from 'next/link'
2+
3+
export const getServerSideProps = async ({ locale }) => {
4+
return {
5+
props: {
6+
locale,
7+
},
8+
}
9+
}
10+
11+
export default function Home({ locale }) {
12+
return (
13+
<main
14+
style={{
15+
display: 'flex',
16+
flexDirection: 'column',
17+
gap: '20px',
18+
}}
19+
>
20+
<p id="current-locale">Current locale: {locale}</p>
21+
Locale switch:
22+
<Link href="/" locale="default">
23+
Default
24+
</Link>
25+
<Link href="/" locale="en">
26+
English
27+
</Link>
28+
<Link href="/" locale="de">
29+
German
30+
</Link>
31+
<br />
32+
Test links:
33+
<Link href="/dynamic/1">Dynamic 1</Link>
34+
<Link href="/static">Static</Link>
35+
</main>
36+
)
37+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export const getServerSideProps = async ({ locale }) => {
2+
return {
3+
props: {
4+
locale,
5+
},
6+
}
7+
}
8+
9+
export default function Static({ locale }) {
10+
return <div id="static-locale">Locale: {locale}</div>
11+
}

0 commit comments

Comments
 (0)
Please sign in to comment.