Skip to content

Commit

Permalink
fix i18n data pathname resolving (#68947)
Browse files Browse the repository at this point in the history
### 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
ztanner authored and lubieowoce committed Aug 22, 2024
1 parent e98a203 commit a5b6028
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 0 deletions.
11 changes: 11 additions & 0 deletions packages/next/src/server/lib/router-utils/resolve-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,17 @@ export function getResolveRoutes(
normalized = normalizers.postponed.normalize(normalized, true)
}

if (config.i18n) {
const curLocaleResult = normalizeLocalePath(
normalized,
config.i18n.locales
)

if (curLocaleResult.detectedLocale) {
parsedUrl.query.__nextLocale = curLocaleResult.detectedLocale
}
}

// If we updated the pathname, and it had a base path, re-add the
// base path.
if (updated) {
Expand Down
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')
})
})
6 changes: 6 additions & 0 deletions test/e2e/i18n-navigations-middleware/middleware.js
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()
}
9 changes: 9 additions & 0 deletions test/e2e/i18n-navigations-middleware/next.config.js
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 test/e2e/i18n-navigations-middleware/pages/dynamic/[id].js
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>
}
37 changes: 37 additions & 0 deletions test/e2e/i18n-navigations-middleware/pages/index.js
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>
)
}
11 changes: 11 additions & 0 deletions test/e2e/i18n-navigations-middleware/pages/static.js
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>
}

0 comments on commit a5b6028

Please sign in to comment.