From db329fe9b0e13a389a84cb98fd936e8a671ba8ec Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 28 Dec 2020 12:21:28 -0600 Subject: [PATCH] Ensure index rewrite is matched with i18n correctly (#20509) This makes sure we don't generate the wrong locale source variant for the rewrite requiring a `/` on the end which won't ever be added causing the rewrite to never match. Additional tests have been added to ensure this specific rewrite is working correctly. Fixes: https://github.com/vercel/next.js/issues/20508 --- packages/next/lib/load-custom-routes.ts | 4 +- .../next/next-server/server/next-server.ts | 5 +- .../i18n-support-index-rewrite/next.config.js | 31 ++++++ .../pages/[...slug].js | 42 ++++++++ .../test/index.test.js | 97 +++++++++++++++++++ 5 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 test/integration/i18n-support-index-rewrite/next.config.js create mode 100644 test/integration/i18n-support-index-rewrite/pages/[...slug].js create mode 100644 test/integration/i18n-support-index-rewrite/test/index.test.js diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index 448ba62cb91f3..61b388d1cc872 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -407,7 +407,9 @@ function processRoutes( r.source = `/:nextInternalLocale(${config.i18n.locales .map((locale: string) => escapeStringRegexp(locale)) - .join('|')})${r.source}` + .join('|')})${ + r.source === '/' && !config.trailingSlash ? '' : r.source + }` if (r.destination && r.destination?.startsWith('/')) { r.destination = `/:nextInternalLocale${ diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ffc3e298cddfe..8eb9b990572e3 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1366,8 +1366,11 @@ export default class Server { ? (req as any)._nextRewroteUrl : urlPathname - resolvedUrlPathname = removePathTrailingSlash(resolvedUrlPathname) urlPathname = removePathTrailingSlash(urlPathname) + resolvedUrlPathname = normalizeLocalePath( + removePathTrailingSlash(resolvedUrlPathname), + this.nextConfig.i18n?.locales + ).pathname const stripNextDataPath = (path: string) => { if (path.includes(this.buildId)) { diff --git a/test/integration/i18n-support-index-rewrite/next.config.js b/test/integration/i18n-support-index-rewrite/next.config.js new file mode 100644 index 0000000000000..f5cb940230994 --- /dev/null +++ b/test/integration/i18n-support-index-rewrite/next.config.js @@ -0,0 +1,31 @@ +module.exports = { + i18n: { + // localeDetection: false, + locales: ['nl-NL', 'nl-BE', 'nl', 'fr-BE', 'fr', 'en'], + defaultLocale: 'en', + domains: [ + { + // 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'], + }, + ], + }, + async rewrites() { + return [ + { + source: '/', + destination: '/company/about-us', + }, + ] + }, +} diff --git a/test/integration/i18n-support-index-rewrite/pages/[...slug].js b/test/integration/i18n-support-index-rewrite/pages/[...slug].js new file mode 100644 index 0000000000000..efc514023ee84 --- /dev/null +++ b/test/integration/i18n-support-index-rewrite/pages/[...slug].js @@ -0,0 +1,42 @@ +import React from 'react' + +const DynamicPage = (props) => ( + <> +
DynamicPage
+
{JSON.stringify(props)}
+ +) + +export const getStaticPaths = async ({ locales }) => { + const paths = [] + + for (const locale of locales) { + paths.push({ + params: { + slug: ['hello'], + }, + locale, + }) + paths.push({ + params: { + slug: ['company', 'about-us'], + }, + locale, + }) + } + + return { + fallback: false, + paths, + } +} + +export const getStaticProps = async ({ params, locale }) => ({ + props: { + locale, + params, + hello: 'world', + }, +}) + +export default DynamicPage diff --git a/test/integration/i18n-support-index-rewrite/test/index.test.js b/test/integration/i18n-support-index-rewrite/test/index.test.js new file mode 100644 index 0000000000000..2dd434ebc4e14 --- /dev/null +++ b/test/integration/i18n-support-index-rewrite/test/index.test.js @@ -0,0 +1,97 @@ +/* eslint-env jest */ + +import { join } from 'path' +import assert from 'assert' +import cheerio from 'cheerio' +import webdriver from 'next-webdriver' +import { + launchApp, + killApp, + findPort, + nextBuild, + nextStart, + renderViaHTTP, + check, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) + +const appDir = join(__dirname, '..') +const locales = ['nl-NL', 'nl-BE', 'nl', 'fr-BE', 'fr', 'en'] +let appPort +let app + +const runTests = () => { + it('should rewrite index route correctly', async () => { + for (const locale of locales) { + const html = await renderViaHTTP( + appPort, + `/${locale === 'en' ? '' : locale}` + ) + const $ = cheerio.load(html) + + expect(JSON.parse($('#props').text())).toEqual({ + params: { + slug: ['company', 'about-us'], + }, + locale, + hello: 'world', + }) + } + }) + + it('should handle index rewrite on client correctly', async () => { + for (const locale of locales) { + const browser = await webdriver( + appPort, + `${locale === 'en' ? '' : `/${locale}`}/hello` + ) + + expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ + params: { + slug: ['hello'], + }, + locale, + hello: 'world', + }) + await browser.eval(`(function() { + window.beforeNav = 1 + window.next.router.push('/') + })()`) + + await check(async () => { + const html = await browser.eval('document.documentElement.innerHTML') + const props = JSON.parse(cheerio.load(html)('#props').text()) + assert.deepEqual(props, { + params: { + slug: ['company', 'about-us'], + }, + locale, + hello: 'world', + }) + return 'success' + }, 'success') + } + }) +} + +describe('Custom routes i18n', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterAll(() => killApp(app)) + runTests(true) + }) + + describe('production mode', () => { + beforeAll(async () => { + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(() => killApp(app)) + runTests() + }) +})