Skip to content

Commit

Permalink
Migrate locale redirect handling to router-server (#62606)
Browse files Browse the repository at this point in the history
This moves the locale redirect handling out of `base-server` as it
shouldn't be handled here and should be at the routing level. This
avoids the duplicate handling with middleware that causes the incorrect
detection/infinite looping. Test case from separate PR was carried over
to prevent regression.

Fixes: #55648
Closes: #62435
Closes: NEXT-2627
Closes: NEXT-2628

---------

Co-authored-by: Nourman Hajar <nourmanhajar@gmail.com>
Co-authored-by: samcx <sam@vercel.com>
  • Loading branch information
3 people committed Mar 6, 2024
1 parent 2f210d4 commit 960b738
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 31 deletions.
31 changes: 0 additions & 31 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import {
PAGES_MANIFEST,
STATIC_STATUS_PAGES,
} from '../shared/lib/constants'
import { RedirectStatusCode } from '../client/components/redirect-status-code'
import { isDynamicRoute } from '../shared/lib/router/utils'
import { checkIsOnDemandRevalidate } from './api-utils'
import { setConfig } from '../shared/lib/runtime-config.external'
Expand Down Expand Up @@ -1198,36 +1197,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
}

if (
// Edge runtime always has minimal mode enabled.
process.env.NEXT_RUNTIME !== 'edge' &&
!this.minimalMode &&
defaultLocale
) {
const { getLocaleRedirect } =
require('../shared/lib/i18n/get-locale-redirect') as typeof import('../shared/lib/i18n/get-locale-redirect')
const redirect = getLocaleRedirect({
defaultLocale,
domainLocale,
headers: req.headers,
nextConfig: this.nextConfig,
pathLocale: pathnameInfo.locale,
urlParsed: {
...url,
pathname: pathnameInfo.locale
? `/${pathnameInfo.locale}${url.pathname}`
: url.pathname,
},
})

if (redirect) {
return res
.redirect(redirect, RedirectStatusCode.TemporaryRedirect)
.body(redirect)
.send()
}
}

addRequestMeta(req, 'isLocaleDomain', Boolean(domainLocale))

if (pathnameInfo.locale) {
Expand Down
55 changes: 55 additions & 0 deletions packages/next/src/server/lib/router-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import setupCompression from 'next/dist/compiled/compression'
import { NoFallbackError } from '../base-server'
import { signalFromNodeResponse } from '../web/spec-extension/adapters/next-request'
import { isPostpone } from './router-utils/is-postpone'
import { parseUrl as parseUrlUtil } from '../../shared/lib/router/utils/parse-url'

import {
PHASE_PRODUCTION_SERVER,
Expand All @@ -34,6 +35,9 @@ import { RedirectStatusCode } from '../../client/components/redirect-status-code
import { DevBundlerService } from './dev-bundler-service'
import { type Span, trace } from '../../trace'
import { ensureLeadingSlash } from '../../shared/lib/page-path/ensure-leading-slash'
import { getNextPathnameInfo } from '../../shared/lib/router/utils/get-next-pathname-info'
import { getHostname } from '../../shared/lib/get-hostname'
import { detectDomainLocale } from '../../shared/lib/i18n/detect-domain-locale'

const debug = setupDebug('next:router-server:main')
const isNextFont = (pathname: string | null) =>
Expand Down Expand Up @@ -140,6 +144,57 @@ export async function initialize(opts: {
require('./render-server') as typeof import('./render-server')

const requestHandlerImpl: WorkerRequestHandler = async (req, res) => {
if (
!opts.minimalMode &&
config.i18n &&
config.i18n.localeDetection !== false
) {
const urlParts = (req.url || '').split('?', 1)
let urlNoQuery = urlParts[0] || ''

if (config.basePath) {
urlNoQuery = removePathPrefix(urlNoQuery, config.basePath)
}

const pathnameInfo = getNextPathnameInfo(urlNoQuery, {
nextConfig: config,
})

const domainLocale = detectDomainLocale(
config.i18n.domains,
getHostname({ hostname: urlNoQuery }, req.headers)
)

const defaultLocale =
domainLocale?.defaultLocale || config.i18n.defaultLocale

const { getLocaleRedirect } =
require('../../shared/lib/i18n/get-locale-redirect') as typeof import('../../shared/lib/i18n/get-locale-redirect')

const parsedUrl = parseUrlUtil((req.url || '')?.replace(/^\/+/, '/'))

const redirect = getLocaleRedirect({
defaultLocale,
domainLocale,
headers: req.headers,
nextConfig: config,
pathLocale: pathnameInfo.locale,
urlParsed: {
...parsedUrl,
pathname: pathnameInfo.locale
? `/${pathnameInfo.locale}${urlNoQuery}`
: urlNoQuery,
},
})

if (redirect) {
res.setHeader('Location', redirect)
res.statusCode = RedirectStatusCode.TemporaryRedirect
res.end(redirect)
return
}
}

if (compress) {
// @ts-expect-error not express req/res
compress(req, res, () => {})
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/i18n-preferred-locale-detection/app/middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export async function middleware() {
const noop = () => {}
noop()
}
10 changes: 10 additions & 0 deletions test/e2e/i18n-preferred-locale-detection/app/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
i18n: {
locales: ['en', 'id'],
defaultLocale: 'en',
},
experimental: {
clientRouterFilter: true,
clientRouterFilterRedirects: true,
},
}
21 changes: 21 additions & 0 deletions test/e2e/i18n-preferred-locale-detection/app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Link from 'next/link'

export const getServerSideProps = async ({ locale }) => {
return {
props: {
locale,
},
}
}

export default function Home({ locale }) {
return (
<div>
<div id="index">Index</div>
<div id="current-locale">{locale}</div>
<Link href="/new" id="to-new">
To new
</Link>
</div>
)
}
21 changes: 21 additions & 0 deletions test/e2e/i18n-preferred-locale-detection/app/pages/new.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Link from 'next/link'

export const getServerSideProps = async ({ locale }) => {
return {
props: {
locale,
},
}
}

export default function New({ locale }) {
return (
<div>
<div id="new">New</div>
<div id="current-locale">{locale}</div>
<Link href="/" id="to-index">
To index (No Locale Specified)
</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import type { Request } from 'playwright'
import { join } from 'path'
import { FileRef, nextTestSetup } from 'e2e-utils'

describe('i18-preferred-locale-redirect', () => {
const { next } = nextTestSetup({
files: new FileRef(join(__dirname, './app/')),
})

it('should request a path prefixed with my preferred detected locale when accessing index', async () => {
const browser = await next.browser('/new', {
locale: 'id',
})

let requestedPreferredLocalePathCount = 0
browser.on('request', (request: Request) => {
if (new URL(request.url(), 'http://n').pathname === '/id') {
requestedPreferredLocalePathCount++
}
})

const goToIndex = async () => {
await browser.get(next.url)
}

await expect(goToIndex()).resolves.not.toThrow(/ERR_TOO_MANY_REDIRECTS/)

await browser.waitForElementByCss('#index')

expect(await browser.elementByCss('#index').text()).toBe('Index')
expect(await browser.elementByCss('#current-locale').text()).toBe('id')

expect(requestedPreferredLocalePathCount).toBe(1)
})

it('should not request a path prefixed with my preferred detected locale when clicking link to index from a non-locale-prefixed path', async () => {
const browser = await next.browser('/new', {
locale: 'id',
})

await browser
.waitForElementByCss('#to-index')
.click()
.waitForElementByCss('#index')

expect(await browser.elementByCss('#index').text()).toBe('Index')
expect(await browser.elementByCss('#current-locale').text()).toBe('en')
})

it('should request a path prefixed with my preferred detected locale when clicking link to index from a locale-prefixed path', async () => {
const browser = await next.browser('/id/new', {
locale: 'id',
})

await browser
.waitForElementByCss('#to-index')
.click()
.waitForElementByCss('#index')

expect(await browser.elementByCss('#index').text()).toBe('Index')
expect(await browser.elementByCss('#current-locale').text()).toBe('id')
})
})

0 comments on commit 960b738

Please sign in to comment.