Migrate locale redirect handling to router-server (#62606)

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: https://github.com/vercel/next.js/issues/55648
Closes: https://github.com/vercel/next.js/pull/62435
Closes: NEXT-2627
Closes: NEXT-2628

---------

Co-authored-by: Nourman Hajar <nourmanhajar@gmail.com>
Co-authored-by: samcx <sam@vercel.com>
This commit is contained in:
JJ Kasper 2024-02-27 16:37:11 -08:00 committed by GitHub
parent 69d1edf6d0
commit 26de5ca269
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 174 additions and 31 deletions

View file

@ -50,7 +50,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'
@ -1167,36 +1166,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) {

View file

@ -27,6 +27,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,
@ -36,6 +37,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) =>
@ -142,6 +146,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, () => {})

View file

@ -0,0 +1,4 @@
export async function middleware() {
const noop = () => {}
noop()
}

View file

@ -0,0 +1,10 @@
module.exports = {
i18n: {
locales: ['en', 'id'],
defaultLocale: 'en',
},
experimental: {
clientRouterFilter: true,
clientRouterFilterRedirects: true,
},
}

View file

@ -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>
)
}

View file

@ -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>
)
}

View file

@ -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')
})
})