Skip to content

Commit

Permalink
Re-add domain locale redirect handling (#18274)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ijjk
ijjk committed Oct 27, 2020
1 parent 4026d9b commit 7cb68f7
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 58 deletions.
30 changes: 13 additions & 17 deletions packages/next/build/webpack/loaders/next-serverless-loader.ts
View file
Expand Up @@ -262,33 +262,29 @@ const nextServerlessLoader: loader.Loader = function () {
...parsedUrl,
pathname: localePathResult.pathname,
})
req.__nextStrippedLocale = true
parsedUrl.pathname = localePathResult.pathname
}
// If a detected locale is a domain specific locale and we aren't already
// on that domain and path prefix redirect to it to prevent duplicate
// content from multiple domains
if (detectedDomain) {
const localeToCheck = localePathResult.detectedLocale
? detectedLocale
: acceptPreferredLocale
// check if the locale prefix matches a domain's defaultLocale
// and we're on a locale specific domain if so redirect to that domain
// if (detectedDomain) {
// const matchedDomain = detectDomainLocale(
// i18n.domains,
// undefined,
// detectedLocale
// )
// if (matchedDomain) {
// localeDomainRedirect = \`http\${
// matchedDomain.http ? '' : 's'
// }://\${matchedDomain.domain}\`
// }
// }
} else if (detectedDomain) {
const matchedDomain = detectDomainLocale(
i18n.domains,
undefined,
acceptPreferredLocale
localeToCheck
)
if (matchedDomain && matchedDomain.domain !== detectedDomain.domain) {
localeDomainRedirect = \`http\${matchedDomain.http ? '' : 's'}://\${
matchedDomain.domain
}/\${
localeToCheck === matchedDomain.defaultLocale ? '' : localeToCheck
}\`
}
}
Expand Down
9 changes: 8 additions & 1 deletion packages/next/next-server/lib/i18n/detect-domain-locale.ts
View file
Expand Up @@ -3,6 +3,7 @@ export function detectDomainLocale(
| Array<{
http?: boolean
domain: string
locales?: string[]
defaultLocale: string
}>
| undefined,
Expand All @@ -13,17 +14,23 @@ export function detectDomainLocale(
| {
http?: boolean
domain: string
locales?: string[]
defaultLocale: string
}
| undefined

if (domainItems) {
if (detectedLocale) {
detectedLocale = detectedLocale.toLowerCase()
}

for (const item of domainItems) {
// remove port if present
const domainHostname = item.domain?.split(':')[0].toLowerCase()
if (
hostname === domainHostname ||
detectedLocale?.toLowerCase() === item.defaultLocale.toLowerCase()
detectedLocale === item.defaultLocale.toLowerCase() ||
item.locales?.some((locale) => locale.toLowerCase() === detectedLocale)
) {
domainItem = item
break
Expand Down
21 changes: 20 additions & 1 deletion packages/next/next-server/server/config.ts
View file
Expand Up @@ -340,7 +340,26 @@ function assignDefaults(userConfig: { [key: string]: any }) {
if (!item.defaultLocale) return true
if (!item.domain || typeof item.domain !== 'string') return true

return false
let hasInvalidLocale = false

if (Array.isArray(item.locales)) {
for (const locale of item.locales) {
if (typeof locale !== 'string') hasInvalidLocale = true

for (const domainItem of i18n.domains) {
if (domainItem === item) continue
if (domainItem.locales && domainItem.locales.includes(locale)) {
console.warn(
`Both ${item.domain} and ${domainItem.domain} configured the locale (${locale}) but only one can. Remove it from one i18n.domains config to continue`
)
hasInvalidLocale = true
break
}
}
}
}

return hasInvalidLocale
})

if (invalidDomainItems.length > 0) {
Expand Down
36 changes: 18 additions & 18 deletions packages/next/next-server/server/next-server.ts
View file
Expand Up @@ -342,32 +342,32 @@ export default class Server {
})
;(req as any).__nextStrippedLocale = true
parsedUrl.pathname = localePathResult.pathname
}

// If a detected locale is a domain specific locale and we aren't already
// on that domain and path prefix redirect to it to prevent duplicate
// content from multiple domains
if (detectedDomain && parsedUrl.pathname === '/') {
const localeToCheck = acceptPreferredLocale
// const localeToCheck = localePathResult.detectedLocale
// ? detectedLocale
// : acceptPreferredLocale

// check if the locale prefix matches a domain's defaultLocale
// and we're on a locale specific domain if so redirect to that domain
// if (detectedDomain) {
// const matchedDomain = detectDomainLocale(
// i18n.domains,
// undefined,
// detectedLocale
// )

// if (matchedDomain) {
// localeDomainRedirect = `http${matchedDomain.http ? '' : 's'}://${
// matchedDomain?.domain
// }`
// }
// }
} else if (detectedDomain) {
const matchedDomain = detectDomainLocale(
i18n.domains,
undefined,
acceptPreferredLocale
localeToCheck
)

if (matchedDomain && matchedDomain.domain !== detectedDomain.domain) {
if (
matchedDomain &&
(matchedDomain.domain !== detectedDomain.domain ||
localeToCheck !== matchedDomain.defaultLocale)
) {
localeDomainRedirect = `http${matchedDomain.http ? '' : 's'}://${
matchedDomain.domain
}/${
localeToCheck === matchedDomain.defaultLocale ? '' : localeToCheck
}`
}
}
Expand Down
2 changes: 2 additions & 0 deletions test/integration/i18n-support/next.config.js
View file
Expand Up @@ -11,11 +11,13 @@ module.exports = {
http: true,
domain: 'example.be',
defaultLocale: 'nl-BE',
locales: ['nl', 'nl-NL', 'nl-BE'],
},
{
http: true,
domain: 'example.fr',
defaultLocale: 'fr',
locales: ['fr-BE'],
},
],
},
Expand Down
76 changes: 55 additions & 21 deletions test/integration/i18n-support/test/index.test.js
View file
Expand Up @@ -52,11 +52,13 @@ function runTests(isDev) {
http: true,
domain: 'example.be',
defaultLocale: 'nl-BE',
locales: ['nl', 'nl-NL', 'nl-BE'],
},
{
http: true,
domain: 'example.fr',
defaultLocale: 'fr',
locales: ['fr-BE'],
},
],
})
Expand Down Expand Up @@ -661,27 +663,56 @@ function runTests(isDev) {
})

it('should handle locales with domain', async () => {
const checkDomainLocales = async (domainDefault = '', domain = '') => {
for (const locale of locales) {
// skip other domains' default locale since we redirect these
if (['fr', 'nl-BE'].includes(locale) && locale !== domainDefault) {
continue
}

const res = await fetchViaHTTP(
appPort,
`/${locale === domainDefault ? '' : locale}`,
undefined,
{
headers: {
host: domain,
},
redirect: 'manual',
}
)
const domainItems = [
{
// used for testing, this should not be needed in most cases
// as production domains should always use https
http: true,
domain: 'example.be',
defaultLocale: 'nl-BE',
locales: ['nl', 'nl-NL', 'nl-BE'],
},
{
http: true,
domain: 'example.fr',
defaultLocale: 'fr',
locales: ['fr-BE'],
},
]
const domainLocales = domainItems.reduce((prev, cur) => {
return [...prev, ...cur.locales]
}, [])

const checkDomainLocales = async (
domainDefault = '',
domain = '',
locale = ''
) => {
const res = await fetchViaHTTP(appPort, `/`, undefined, {
headers: {
host: domain,
'accept-language': locale,
},
redirect: 'manual',
})
const expectedDomainItem = domainItems.find(
(item) => item.defaultLocale === locale || item.locales.includes(locale)
)
const shouldRedirect =
expectedDomainItem.domain !== domain ||
locale !== expectedDomainItem.defaultLocale

expect(res.status).toBe(200)
expect(res.status).toBe(shouldRedirect ? 307 : 200)

if (shouldRedirect) {
const parsedUrl = url.parse(res.headers.get('location'), true)

expect(parsedUrl.pathname).toBe(
`/${expectedDomainItem.defaultLocale === locale ? '' : locale}`
)
expect(parsedUrl.query).toEqual({})
expect(parsedUrl.hostname).toBe(expectedDomainItem.domain)
} else {
const html = await res.text()
const $ = cheerio.load(html)

Expand All @@ -691,8 +722,11 @@ function runTests(isDev) {
}
}

await checkDomainLocales('nl-BE', 'example.be')
await checkDomainLocales('fr', 'example.fr')
for (const item of domainItems) {
for (const locale of domainLocales) {
await checkDomainLocales(item.defaultLocale, item.domain, locale)
}
}
})

it('should generate AMP pages with all locales', async () => {
Expand Down

0 comments on commit 7cb68f7

Please sign in to comment.