Skip to content

Commit

Permalink
Handle defaultLocale on client router filter (#47180)
Browse files Browse the repository at this point in the history
x-ref: [slack
thread](https://vercel.slack.com/archives/C03S8ED1DKM/p1678838567947919)

Follow-up to #46317. The issue is
that, if:

- `experimental.clientRouterFilter` is enabled
- `i18n` is enabled with `defaultLocale` set
- Next.js router navigates to a path that (1) is the same as
`defaultLocale` and (2) will be redirected,

then:

- **Expected:** Should hard-navigate to this path without any locale
prefix (and then redirect occurs)
- **Actual:** Hard-navigates to this path with `defaultLocale` prefix,
even though it's not needed (and then redirect occurrs)

### Solution

This PR fixes the above issue by adding `defaultLocale` to `addLocale`
which is passed to `handleHardNavigation`. [`addLocale` skips adding the
locale if `locale` is equal to
`defaultLocale`](https://github.com/vercel/next.js/blob/02125cf3b1dfba52b240fd6e0f959d896e4b6195/packages/next/src/shared/lib/router/utils/add-locale.ts#L17).

### Fixing a bug

- [x] Related issues linked using `fixes #number`
- [x] Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
chibicode and ijjk authored Mar 16, 2023
1 parent 1255e19 commit 723626c
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 2 deletions.
4 changes: 3 additions & 1 deletion packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,9 @@ export default class Router implements BaseRouter {
// a hard navigation
if (matchesBflStatic || matchesBflDynamic) {
handleHardNavigation({
url: addBasePath(addLocale(as, locale || this.locale)),
url: addBasePath(
addLocale(as, locale || this.locale, this.defaultLocale)
),
router: this,
})
return new Promise(() => {})
Expand Down
19 changes: 19 additions & 0 deletions test/e2e/i18n-default-locale-redirect/app/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module.exports = {
i18n: {
locales: ['en', 'nl'],
defaultLocale: 'en',
},
experimental: {
clientRouterFilter: true,
clientRouterFilterRedirects: true,
},
redirects() {
return [
{
source: '/to-new',
destination: '/new',
permanent: false,
},
]
},
}
18 changes: 18 additions & 0 deletions test/e2e/i18n-default-locale-redirect/app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Link from 'next/link'

export default function Home() {
return (
<div>
<div>
<Link href="/to-new" id="to-new">
To new (Default Locale)
</Link>
</div>
<div>
<Link href="/to-new" id="to-new-nl" locale="nl">
To new (NL)
</Link>
</div>
</div>
)
}
3 changes: 3 additions & 0 deletions test/e2e/i18n-default-locale-redirect/app/pages/new.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function New() {
return <div id="new">New</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { join } from 'path'
import { createNextDescribe, FileRef } from 'e2e-utils'

createNextDescribe(
'i18-default-locale-redirect',
{
files: {
pages: new FileRef(join(__dirname, './app/pages')),
'next.config.js': new FileRef(join(__dirname, './app/next.config.js')),
},
},
({ next }) => {
it('should not request a path prefixed with default locale', async () => {
const browser = await next.browser('/')
let requestedDefaultLocalePath = false
browser.on('request', (request: any) => {
if (new URL(request.url(), 'http://n').pathname === '/en/to-new') {
requestedDefaultLocalePath = true
}
})

await browser.elementByCss('#to-new').click().waitForElementByCss('#new')
expect(await browser.elementByCss('#new').text()).toBe('New')
expect(requestedDefaultLocalePath).toBe(false)
})

it('should request a path prefixed with non-default locale', async () => {
const browser = await next.browser('/')
let requestedNonDefaultLocalePath = false
browser.on('request', (request: any) => {
if (new URL(request.url(), 'http://n').pathname === '/nl/to-new') {
requestedNonDefaultLocalePath = true
}
})

await browser
.elementByCss('#to-new-nl')
.click()
.waitForElementByCss('#new')
expect(await browser.elementByCss('#new').text()).toBe('New')
expect(requestedNonDefaultLocalePath).toBe(true)
})
}
)
2 changes: 1 addition & 1 deletion test/lib/browsers/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class Playwright extends BrowserInterface {
if (!this.eventCallbacks[event]) {
throw new Error(
`Invalid event passed to browser.on, received ${event}. Valid events are ${Object.keys(
event
this.eventCallbacks
)}`
)
}
Expand Down

0 comments on commit 723626c

Please sign in to comment.