From 2ab34451b7f36745f33a69fb35256f40fd13eac1 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 21 Oct 2020 04:35:01 -0500 Subject: [PATCH] Pass locales to getStaticPaths for i18n (#18077) This makes sure the `locales` are passed to `getStaticPaths` and also disables the removing the default locale from the path when the default locale is the preferred header. It also updates tests to ensure the domain redirects are working as expected. x-ref: https://github.com/vercel/next.js/pull/17370 --- packages/next/build/utils.ts | 2 +- .../webpack/loaders/next-serverless-loader.ts | 40 +++++++---- .../next/next-server/server/next-server.ts | 42 +++++++---- packages/next/types/index.d.ts | 10 ++- .../i18n-support/pages/gsp/fallback/[slug].js | 9 ++- .../i18n-support/test/index.test.js | 71 ++++++++++--------- 6 files changed, 105 insertions(+), 69 deletions(-) diff --git a/packages/next/build/utils.ts b/packages/next/build/utils.ts index 7e1eb11cc531f88..9216875d7e9806c 100644 --- a/packages/next/build/utils.ts +++ b/packages/next/build/utils.ts @@ -544,7 +544,7 @@ export async function buildStaticPaths( // Get the default list of allowed params. const _validParamKeys = Object.keys(_routeMatcher(page)) - const staticPathsResult = await getStaticPaths() + const staticPathsResult = await getStaticPaths({ locales }) const expectedReturnVal = `Expected: { paths: [], fallback: boolean }\n` + diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 3f2c092357418b7..739a40d3ce7477d 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -264,18 +264,30 @@ const nextServerlessLoader: loader.Loader = function () { // 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 (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 + ) - if (matchedDomain) { - localeDomainRedirect = \`http\${ - matchedDomain.http ? '' : 's' - }://\${matchedDomain.domain}\` - } + if (matchedDomain && matchedDomain.domain !== detectedDomain.domain) { + localeDomainRedirect = \`http\${matchedDomain.http ? '' : 's'}://\${ + matchedDomain.domain + }\` } } @@ -283,9 +295,9 @@ const nextServerlessLoader: loader.Loader = function () { const detectedDefaultLocale = !detectedLocale || detectedLocale.toLowerCase() === defaultLocale.toLowerCase() - const shouldStripDefaultLocale = - detectedDefaultLocale && - denormalizedPagePath.toLowerCase() === \`/\${i18n.defaultLocale.toLowerCase()}\` + const shouldStripDefaultLocale = false + // detectedDefaultLocale && + // denormalizedPagePath.toLowerCase() === \`/\${i18n.defaultLocale.toLowerCase()}\` const shouldAddLocalePrefix = !detectedDefaultLocale && denormalizedPagePath === '/' diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index aec5000a4573db7..4102a0f48c2e2c5 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -345,18 +345,30 @@ export default class Server { // 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 (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 + ) - if (matchedDomain) { - localeDomainRedirect = `http${matchedDomain.http ? '' : 's'}://${ - matchedDomain?.domain - }` - } + if (matchedDomain && matchedDomain.domain !== detectedDomain.domain) { + localeDomainRedirect = `http${matchedDomain.http ? '' : 's'}://${ + matchedDomain.domain + }` } } @@ -364,10 +376,10 @@ export default class Server { const detectedDefaultLocale = !detectedLocale || detectedLocale.toLowerCase() === defaultLocale.toLowerCase() - const shouldStripDefaultLocale = - detectedDefaultLocale && - denormalizedPagePath.toLowerCase() === - `/${i18n.defaultLocale.toLowerCase()}` + const shouldStripDefaultLocale = false + // detectedDefaultLocale && + // denormalizedPagePath.toLowerCase() === + // `/${i18n.defaultLocale.toLowerCase()}` const shouldAddLocalePrefix = !detectedDefaultLocale && denormalizedPagePath === '/' diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index e89990f38a8598f..1f10532657a9764 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -105,14 +105,18 @@ export type InferGetStaticPropsType = T extends GetStaticProps ? P : never +export type GetStaticPathsContext = { + locales?: string[] +} + export type GetStaticPathsResult

= { paths: Array fallback: boolean | 'unstable_blocking' } -export type GetStaticPaths< - P extends ParsedUrlQuery = ParsedUrlQuery -> = () => Promise> +export type GetStaticPaths

= ( + context: GetStaticPathsContext +) => Promise> export type GetServerSidePropsContext< Q extends ParsedUrlQuery = ParsedUrlQuery diff --git a/test/integration/i18n-support/pages/gsp/fallback/[slug].js b/test/integration/i18n-support/pages/gsp/fallback/[slug].js index ec236891864f732..d4486b24493b382 100644 --- a/test/integration/i18n-support/pages/gsp/fallback/[slug].js +++ b/test/integration/i18n-support/pages/gsp/fallback/[slug].js @@ -33,7 +33,14 @@ export const getStaticProps = ({ params, locale, locales }) => { } } -export const getStaticPaths = () => { +export const getStaticPaths = ({ locales }) => { + // make sure locales were provided correctly + if (!locales || locales.length !== 7) { + throw new Error( + 'locales missing in getStaticPaths!! got: ' + JSON.stringify(locales) + ) + } + return { // the default locale will be used since one isn't defined here paths: ['first', 'second'].map((slug) => ({ diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js index cfabe66298492cf..74ce600571973f5 100644 --- a/test/integration/i18n-support/test/index.test.js +++ b/test/integration/i18n-support/test/index.test.js @@ -397,7 +397,7 @@ function runTests(isDev) { expect(JSON.parse($2('#router-locales').text())).toEqual(locales) }) - it('should strip locale prefix for default locale with locale domains', async () => { + it('should not strip locale prefix for default locale with locale domains', async () => { const res = await fetchViaHTTP(appPort, '/fr', undefined, { headers: { host: 'example.fr', @@ -405,11 +405,11 @@ function runTests(isDev) { redirect: 'manual', }) - expect(res.status).toBe(307) + expect(res.status).toBe(200) - const result = url.parse(res.headers.get('location'), true) - expect(result.pathname).toBe('/') - expect(result.query).toEqual({}) + // const result = url.parse(res.headers.get('location'), true) + // expect(result.pathname).toBe('/') + // expect(result.query).toEqual({}) const res2 = await fetchViaHTTP(appPort, '/nl-BE', undefined, { headers: { @@ -418,28 +418,28 @@ function runTests(isDev) { redirect: 'manual', }) - expect(res2.status).toBe(307) + expect(res2.status).toBe(200) - const result2 = url.parse(res2.headers.get('location'), true) - expect(result2.pathname).toBe('/') - expect(result2.query).toEqual({}) + // const result2 = url.parse(res2.headers.get('location'), true) + // expect(result2.pathname).toBe('/') + // expect(result2.query).toEqual({}) }) - it('should set locale cookie when removing default locale and accept-lang doesnt match', async () => { - const res = await fetchViaHTTP(appPort, '/en-US', undefined, { - headers: { - 'accept-language': 'nl', - }, - redirect: 'manual', - }) + // ('should set locale cookie when removing default locale and accept-lang doesnt match', async () => { + // const res = await fetchViaHTTP(appPort, '/en-US', undefined, { + // headers: { + // 'accept-language': 'nl', + // }, + // redirect: 'manual', + // }) - expect(res.status).toBe(307) + // expect(res.status).toBe(307) - const parsedUrl = url.parse(res.headers.get('location'), true) - expect(parsedUrl.pathname).toBe('/') - expect(parsedUrl.query).toEqual({}) - expect(res.headers.get('set-cookie')).toContain('NEXT_LOCALE=en-US') - }) + // const parsedUrl = url.parse(res.headers.get('location'), true) + // expect(parsedUrl.pathname).toBe('/') + // expect(parsedUrl.query).toEqual({}) + // expect(res.headers.get('set-cookie')).toContain('NEXT_LOCALE=en-US') + // }) it('should not redirect to accept-lang preferred locale with locale cookie', async () => { const res = await fetchViaHTTP(appPort, '/', undefined, { @@ -465,18 +465,19 @@ function runTests(isDev) { it('should redirect to correct locale domain', async () => { const checks = [ // test domain, locale prefix, redirect result - ['example.be', 'nl-BE', 'http://example.be/'], + // ['example.be', 'nl-BE', 'http://example.be/'], ['example.be', 'fr', 'http://example.fr/'], ['example.fr', 'nl-BE', 'http://example.be/'], - ['example.fr', 'fr', 'http://example.fr/'], + // ['example.fr', 'fr', 'http://example.fr/'], ] for (const check of checks) { - const [domain, localePath, location] = check + const [domain, locale, location] = check - const res = await fetchViaHTTP(appPort, `/${localePath}`, undefined, { + const res = await fetchViaHTTP(appPort, `/`, undefined, { headers: { host: domain, + 'accept-language': locale, }, redirect: 'manual', }) @@ -705,7 +706,7 @@ function runTests(isDev) { expect(await browser.eval('window.beforeNav')).toBe(1) }) - it('should remove un-necessary locale prefix for default locale', async () => { + it('should not remove locale prefix for default locale', async () => { const res = await fetchViaHTTP(appPort, '/en-US', undefined, { redirect: 'manual', headers: { @@ -713,12 +714,12 @@ function runTests(isDev) { }, }) - expect(res.status).toBe(307) + expect(res.status).toBe(200) - const parsedUrl = url.parse(res.headers.get('location'), true) + // const parsedUrl = url.parse(res.headers.get('location'), true) - expect(parsedUrl.pathname).toBe('/') - expect(parsedUrl.query).toEqual({}) + // expect(parsedUrl.pathname).toBe('/') + // expect(parsedUrl.query).toEqual({}) // make sure locale is case-insensitive const res2 = await fetchViaHTTP(appPort, '/eN-Us', undefined, { @@ -728,12 +729,12 @@ function runTests(isDev) { }, }) - expect(res2.status).toBe(307) + expect(res2.status).toBe(200) - const parsedUrl2 = url.parse(res.headers.get('location'), true) + // const parsedUrl2 = url.parse(res.headers.get('location'), true) - expect(parsedUrl2.pathname).toBe('/') - expect(parsedUrl2.query).toEqual({}) + // expect(parsedUrl2.pathname).toBe('/') + // expect(parsedUrl2.query).toEqual({}) }) it('should load getStaticProps page correctly SSR (default locale no prefix)', async () => {