Re-add domain locale redirect handling (#18274)

vercel · Oct 27, 2020 · 7cb68f7 · 7cb68f7
1 parent 4026d9b
commit 7cb68f7
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 58 deletions.
diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts
@@ -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
           }\`
         }
       }

diff --git a/packages/next/next-server/lib/i18n/detect-domain-locale.ts b/packages/next/next-server/lib/i18n/detect-domain-locale.ts
@@ -3,6 +3,7 @@ export function detectDomainLocale(
     | Array<{
         http?: boolean
         domain: string
+        locales?: string[]
         defaultLocale: string
       }>
     | undefined,
@@ -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

diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts
@@ -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) {

diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
@@ -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
           }`
         }
       }

diff --git a/test/integration/i18n-support/next.config.js b/test/integration/i18n-support/next.config.js
@@ -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'],
         },
       ],
     },

diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js
@@ -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'],
           },
         ],
       })
@@ -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)
 
@@ -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 () => {