Skip to content

Commit

Permalink
i18n Improvements (vercel#47174)
Browse files Browse the repository at this point in the history
This serves to correct a specific issue related to multiple locales being specified in the pathname as well as some general i18n improvements.

- Multiple locales are now parsed correctly (only taking the first locale, treating the rest of the string as the pathname)
- `LocaleRouteNormalizer` has been split into `I18NProvider` and `LocaleRouteNormalizer` (tests added)
- Adjusted the `I18NProvider.analyze` method (previously `LocaleRouteNormalizer.match`) to require the `defaultLocale: string | undefined` to ensure consistent behaviour
- Added more comments around i18n
  • Loading branch information
wyattjoh authored Mar 17, 2023
1 parent 9dbd8d7 commit e29bd49
Show file tree
Hide file tree
Showing 21 changed files with 546 additions and 253 deletions.
108 changes: 43 additions & 65 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ import { isBot } from '../shared/lib/router/utils/is-bot'
import RenderResult from './render-result'
import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-slash'
import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
import * as Log from '../build/output/log'
import { detectDomainLocale } from '../shared/lib/i18n/detect-domain-locale'
import escapePathDelimiters from '../shared/lib/router/utils/escape-path-delimiters'
import { getUtils } from '../build/webpack/loaders/next-serverless-loader/utils'
import isError, { getProperError } from '../lib/is-error'
Expand Down Expand Up @@ -95,6 +93,7 @@ import { ServerManifestLoader } from './future/route-matcher-providers/helpers/m
import { getTracer } from './lib/trace/tracer'
import { BaseServerSpan } from './lib/trace/constants'
import { sendResponse } from './future/route-handlers/app-route-route-handler'
import { I18NProvider } from './future/helpers/i18n-provider'

export type FindComponentsResult = {
components: LoadComponentsReturnType
Expand Down Expand Up @@ -330,6 +329,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// TODO-APP(@wyattjoh): Make protected again. Used for turbopack in route-resolver.ts right now.
public readonly matchers: RouteMatcherManager
protected readonly handlers: RouteHandlerManager
protected readonly i18nProvider?: I18NProvider
protected readonly localeNormalizer?: LocaleRouteNormalizer

public constructor(options: ServerOptions) {
Expand Down Expand Up @@ -363,14 +363,14 @@ export default abstract class Server<ServerOptions extends Options = Options> {
this.publicDir = this.getPublicDir()
this.hasStaticDir = !minimalMode && this.getHasStaticDir()

this.i18nProvider = this.nextConfig.i18n?.locales
? new I18NProvider(this.nextConfig.i18n)
: undefined

// Configure the locale normalizer, it's used for routes inside `pages/`.
this.localeNormalizer =
this.nextConfig.i18n?.locales && this.nextConfig.i18n.defaultLocale
? new LocaleRouteNormalizer(
this.nextConfig.i18n.locales,
this.nextConfig.i18n.defaultLocale
)
: undefined
this.localeNormalizer = this.i18nProvider
? new LocaleRouteNormalizer(this.i18nProvider)
: undefined

// Only serverRuntimeConfig needs the default
// publicRuntimeConfig gets it's default in client/index.js
Expand Down Expand Up @@ -481,7 +481,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
new PagesRouteMatcherProvider(
this.distDir,
manifestLoader,
this.localeNormalizer
this.i18nProvider
)
)

Expand All @@ -490,7 +490,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
new PagesAPIRouteMatcherProvider(
this.distDir,
manifestLoader,
this.localeNormalizer
this.i18nProvider
)
)

Expand Down Expand Up @@ -600,19 +600,19 @@ export default abstract class Server<ServerOptions extends Options = Options> {

this.attachRequestMeta(req, parsedUrl)

const domainLocale = detectDomainLocale(
this.nextConfig.i18n?.domains,
const domainLocale = this.i18nProvider?.detectDomainLocale(
getHostname(parsedUrl, req.headers)
)

const defaultLocale =
domainLocale?.defaultLocale || this.nextConfig.i18n?.defaultLocale
parsedUrl.query.__nextDefaultLocale = defaultLocale

const url = parseUrlUtil(req.url.replace(/^\/+/, '/'))
const pathnameInfo = getNextPathnameInfo(url.pathname, {
nextConfig: this.nextConfig,
i18nProvider: this.i18nProvider,
})

url.pathname = pathnameInfo.pathname

if (pathnameInfo.basePath) {
Expand Down Expand Up @@ -655,7 +655,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {

// Perform locale detection and normalization.
const options: MatchOptions = {
i18n: this.localeNormalizer?.match(matchedPath),
i18n: this.i18nProvider?.analyze(matchedPath, {
defaultLocale: undefined,
}),
}
if (options.i18n?.detectedLocale) {
parsedUrl.query.__nextLocale = options.i18n.detectedLocale
Expand Down Expand Up @@ -683,11 +685,13 @@ export default abstract class Server<ServerOptions extends Options = Options> {
basePath: this.nextConfig.basePath,
rewrites: this.customRoutes.rewrites,
})
// ensure parsedUrl.pathname includes URL before processing
// rewrites or they won't match correctly

// Ensure parsedUrl.pathname includes locale before processing
// rewrites or they won't match correctly.
if (defaultLocale && !pathnameInfo.locale) {
parsedUrl.pathname = `/${defaultLocale}${parsedUrl.pathname}`
}

const pathnameBeforeRewrite = parsedUrl.pathname
const rewriteParams = utils.handleRewrites(req, parsedUrl)
const rewriteParamKeys = Object.keys(rewriteParams)
Expand Down Expand Up @@ -793,7 +797,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {

addRequestMeta(req, '__nextHadTrailingSlash', pathnameInfo.trailingSlash)
addRequestMeta(req, '__nextIsLocaleDomain', Boolean(domainLocale))
parsedUrl.query.__nextDefaultLocale = defaultLocale

if (pathnameInfo.locale) {
req.url = formatUrl(url)
Expand Down Expand Up @@ -951,12 +954,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

private async pipe(
fn: (ctx: RequestContext) => Promise<ResponsePayload | null>,
partialContext: {
req: BaseNextRequest
res: BaseNextResponse
pathname: string
query: NextParsedUrlQuery
}
partialContext: Omit<RequestContext, 'renderOpts'>
): Promise<void> {
return getTracer().trace(BaseServerSpan.pipe, async () =>
this.pipeImpl(fn, partialContext)
Expand All @@ -965,22 +963,17 @@ export default abstract class Server<ServerOptions extends Options = Options> {

private async pipeImpl(
fn: (ctx: RequestContext) => Promise<ResponsePayload | null>,
partialContext: {
req: BaseNextRequest
res: BaseNextResponse
pathname: string
query: NextParsedUrlQuery
}
partialContext: Omit<RequestContext, 'renderOpts'>
): Promise<void> {
const isBotRequest = isBot(partialContext.req.headers['user-agent'] || '')
const ctx = {
const ctx: RequestContext = {
...partialContext,
renderOpts: {
...this.renderOpts,
supportsDynamicHTML: !isBotRequest,
isBot: !!isBotRequest,
},
} as const
}
const payload = await fn(ctx)
if (payload === null) {
return
Expand All @@ -1005,20 +998,16 @@ export default abstract class Server<ServerOptions extends Options = Options> {

private async getStaticHTML(
fn: (ctx: RequestContext) => Promise<ResponsePayload | null>,
partialContext: {
req: BaseNextRequest
res: BaseNextResponse
pathname: string
query: ParsedUrlQuery
}
partialContext: Omit<RequestContext, 'renderOpts'>
): Promise<string | null> {
const payload = await fn({
const ctx: RequestContext = {
...partialContext,
renderOpts: {
...this.renderOpts,
supportsDynamicHTML: false,
},
})
}
const payload = await fn(ctx)
if (payload === null) {
return null
}
Expand Down Expand Up @@ -1352,10 +1341,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

urlPathname = removeTrailingSlash(urlPathname)
resolvedUrlPathname = normalizeLocalePath(
removeTrailingSlash(resolvedUrlPathname),
this.nextConfig.i18n?.locales
).pathname
resolvedUrlPathname = removeTrailingSlash(resolvedUrlPathname)
if (this.localeNormalizer) {
resolvedUrlPathname = this.localeNormalizer.normalize(resolvedUrlPathname)
}

const handleRedirect = (pageData: any) => {
const redirect = {
Expand Down Expand Up @@ -1787,15 +1776,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
if (this.renderOpts.dev) {
query.__nextNotFoundSrcPage = pathname
}
await this.render404(
req,
res,
{
pathname,
query,
} as UrlWithParsedQuery,
false
)
await this.render404(req, res, { pathname, query }, false)
return null
}
} else if (cachedData.kind === 'REDIRECT') {
Expand Down Expand Up @@ -1864,9 +1845,8 @@ export default abstract class Server<ServerOptions extends Options = Options> {
path = denormalizePagePath(splitPath.replace(/\.json$/, ''))
}

if (this.nextConfig.i18n && stripLocale) {
const { locales } = this.nextConfig.i18n
return normalizeLocalePath(path, locales).pathname
if (this.localeNormalizer && stripLocale) {
return this.localeNormalizer.normalize(path)
}
return path
}
Expand Down Expand Up @@ -1941,7 +1921,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
delete query._nextBubbleNoFallback

const options: MatchOptions = {
i18n: this.localeNormalizer?.match(pathname),
i18n: this.i18nProvider?.analyze(pathname, {
defaultLocale: undefined,
}),
}

try {
Expand Down Expand Up @@ -2305,18 +2287,14 @@ export default abstract class Server<ServerOptions extends Options = Options> {
public async render404(
req: BaseNextRequest,
res: BaseNextResponse,
parsedUrl?: NextUrlWithParsedQuery,
parsedUrl?: Pick<NextUrlWithParsedQuery, 'pathname' | 'query'>,
setHeaders = true
): Promise<void> {
const { pathname, query }: NextUrlWithParsedQuery = parsedUrl
? parsedUrl
: parseUrl(req.url!, true)
const { pathname, query } = parsedUrl ? parsedUrl : parseUrl(req.url!, true)

if (this.nextConfig.i18n) {
query.__nextLocale =
query.__nextLocale || this.nextConfig.i18n.defaultLocale
query.__nextDefaultLocale =
query.__nextDefaultLocale || this.nextConfig.i18n.defaultLocale
query.__nextLocale ||= this.nextConfig.i18n.defaultLocale
query.__nextDefaultLocale ||= this.nextConfig.i18n.defaultLocale
}

res.statusCode = 404
Expand Down
14 changes: 8 additions & 6 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ export default class DevServer extends Server {
distDir: this.distDir,
buildId: this.buildId,
}
) // In development we can't give a default path mapping
)

// In development we can't give a default path mapping
for (const path in exportPathMap) {
const { page, query = {} } = exportPathMap[path]

Expand Down Expand Up @@ -1159,6 +1161,7 @@ export default class DevServer extends Server {
const { basePath } = this.nextConfig
let originalPathname: string | null = null

// TODO: see if we can remove this in the future
if (basePath && pathHasPrefix(parsedUrl.pathname || '/', basePath)) {
// strip basePath before handling dev bundles
// If replace ends up replacing the full url it'll be `undefined`, meaning we have to default it to `/`
Expand All @@ -1174,15 +1177,14 @@ export default class DevServer extends Server {
}
}

const { finished = false } =
(await this.hotReloader?.run(
if (this.hotReloader) {
const { finished = false } = await this.hotReloader.run(
req.originalRequest,
res.originalResponse,
parsedUrl
)) || {}
)

if (finished) {
return
if (finished) return
}

if (originalPathname) {
Expand Down
63 changes: 63 additions & 0 deletions packages/next/src/server/future/helpers/i18n-provider.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { I18NProvider } from './i18n-provider'

describe('I18NProvider', () => {
const provider = new I18NProvider({
defaultLocale: 'en',
locales: ['en', 'fr', 'en-CA'],
domains: [
{
domain: 'example.com',
defaultLocale: 'en',
locales: ['en-CA'],
},
{
domain: 'example.fr',
defaultLocale: 'fr',
},
],
})

it('should detect the correct domain locale', () => {
expect(provider.detectDomainLocale('example.com')).toEqual({
domain: 'example.com',
defaultLocale: 'en',
locales: ['en-CA'],
})
expect(provider.detectDomainLocale('example.fr')).toEqual({
domain: 'example.fr',
defaultLocale: 'fr',
})
expect(provider.detectDomainLocale('example.de')).toBeUndefined()
})

describe('analyze', () => {
it('should detect the correct default locale', () => {
expect(provider.analyze('/fr', { defaultLocale: undefined })).toEqual({
pathname: '/',
detectedLocale: 'fr',
})
expect(
provider.analyze('/fr/another/page', { defaultLocale: undefined })
).toEqual({
pathname: '/another/page',
detectedLocale: 'fr',
})
expect(
provider.analyze('/another/page', {
defaultLocale: 'en-CA',
})
).toEqual({
pathname: '/another/page',
detectedLocale: 'en-CA',
})
expect(
provider.analyze('/en/another/page', {
defaultLocale: 'en-CA',
})
).toEqual({
pathname: '/another/page',
detectedLocale: 'en',
})
})
})
})
Loading

0 comments on commit e29bd49

Please sign in to comment.