-
Notifications
You must be signed in to change notification settings - Fork 27.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
7 changed files
with
128 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
test/e2e/i18n-navigations-middleware/i18n-navigations-middleware.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import { nextTestSetup } from 'e2e-utils' | ||
|
||
describe('i18n-navigations-middleware', () => { | ||
const { next } = nextTestSetup({ | ||
files: __dirname, | ||
}) | ||
|
||
it('should respect selected locale when navigating to a dynamic route', async () => { | ||
const browser = await next.browser('/') | ||
// change to "de" locale | ||
await browser.elementByCss("[href='/de']").click() | ||
const dynamicLink = await browser.waitForElementByCss( | ||
"[href='/de/dynamic/1']" | ||
) | ||
expect(await browser.elementById('current-locale').text()).toBe( | ||
'Current locale: de' | ||
) | ||
|
||
// navigate to dynamic route | ||
await dynamicLink.click() | ||
|
||
// the locale should still be "de" | ||
expect(await browser.elementById('dynamic-locale').text()).toBe( | ||
'Locale: de' | ||
) | ||
}) | ||
|
||
it('should respect selected locale when navigating to a static route', async () => { | ||
const browser = await next.browser('/') | ||
// change to "de" locale | ||
await browser.elementByCss("[href='/de']").click() | ||
const staticLink = await browser.waitForElementByCss("[href='/de/static']") | ||
expect(await browser.elementById('current-locale').text()).toBe( | ||
'Current locale: de' | ||
) | ||
|
||
// navigate to static route | ||
await staticLink.click() | ||
|
||
// the locale should still be "de" | ||
expect(await browser.elementById('static-locale').text()).toBe('Locale: de') | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { NextResponse } from 'next/server' | ||
|
||
export const config = { matcher: ['/foo'] } | ||
export async function middleware(req) { | ||
return NextResponse.next() | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/** | ||
* @type {import('next').NextConfig} | ||
*/ | ||
module.exports = { | ||
i18n: { | ||
defaultLocale: 'default', | ||
locales: ['default', 'en', 'de'], | ||
}, | ||
} |
11 changes: 11 additions & 0 deletions
11
test/e2e/i18n-navigations-middleware/pages/dynamic/[id].js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
export const getServerSideProps = async ({ locale }) => { | ||
return { | ||
props: { | ||
locale, | ||
}, | ||
} | ||
} | ||
|
||
export default function Dynamic({ locale }) { | ||
return <div id="dynamic-locale">Locale: {locale}</div> | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import Link from 'next/link' | ||
|
||
export const getServerSideProps = async ({ locale }) => { | ||
return { | ||
props: { | ||
locale, | ||
}, | ||
} | ||
} | ||
|
||
export default function Home({ locale }) { | ||
return ( | ||
<main | ||
style={{ | ||
display: 'flex', | ||
flexDirection: 'column', | ||
gap: '20px', | ||
}} | ||
> | ||
<p id="current-locale">Current locale: {locale}</p> | ||
Locale switch: | ||
<Link href="/" locale="default"> | ||
Default | ||
</Link> | ||
<Link href="/" locale="en"> | ||
English | ||
</Link> | ||
<Link href="/" locale="de"> | ||
German | ||
</Link> | ||
<br /> | ||
Test links: | ||
<Link href="/dynamic/1">Dynamic 1</Link> | ||
<Link href="/static">Static</Link> | ||
</main> | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
export const getServerSideProps = async ({ locale }) => { | ||
return { | ||
props: { | ||
locale, | ||
}, | ||
} | ||
} | ||
|
||
export default function Static({ locale }) { | ||
return <div id="static-locale">Locale: {locale}</div> | ||
} |