From ca590c4cb91cef4fb164c6e1211386145038eded Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 17 Nov 2020 15:46:46 -0600 Subject: [PATCH] Ensure i18n + trailingSlash: true handles correctly (#19149) This ensures redirects are handled properly with i18n + `trailingSlash: true`, additional tests have also been added to ensure this is covered Fixes: https://github.com/vercel/next.js/issues/19069 --- .../next/next-server/server/next-server.ts | 30 +++-- packages/next/next-server/server/router.ts | 7 ++ test/integration/i18n-support/next.config.js | 1 + .../i18n-support/test/index.test.js | 111 ++++++++++++++++++ 4 files changed, 139 insertions(+), 10 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 89ee282ab1..4a6d00c38d 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -342,6 +342,11 @@ export default class Server { detectedLocale = detectedLocale || acceptPreferredLocale let localeDomainRedirect: string | undefined + ;(req as any).__nextHadTrailingSlash = pathname!.endsWith('/') + + if (pathname === '/') { + ;(req as any).__nextHadTrailingSlash = this.nextConfig.trailingSlash + } const localePathResult = normalizeLocalePath(pathname!, i18n.locales) if (localePathResult.detectedLocale) { @@ -351,7 +356,12 @@ export default class Server { pathname: localePathResult.pathname, }) ;(req as any).__nextStrippedLocale = true - parsedUrl.pathname = `${basePath || ''}${localePathResult.pathname}` + parsedUrl.pathname = `${basePath || ''}${localePathResult.pathname}${ + (req as any).__nextHadTrailingSlash && + localePathResult.pathname !== '/' + ? '/' + : '' + }` } // If a detected locale is a domain specific locale and we aren't already @@ -426,15 +436,15 @@ export default class Server { res.setHeader( 'Location', - formatUrl({ - // make sure to include any query values when redirecting - ...parsed, - pathname: localeDomainRedirect - ? localeDomainRedirect - : shouldStripDefaultLocale - ? basePath || `/` - : `${basePath || ''}/${detectedLocale}`, - }) + localeDomainRedirect + ? localeDomainRedirect + : formatUrl({ + // make sure to include any query values when redirecting + ...parsed, + pathname: shouldStripDefaultLocale + ? basePath || `/` + : `${basePath || ''}/${detectedLocale}`, + }) ) res.statusCode = 307 res.end() diff --git a/packages/next/next-server/server/router.ts b/packages/next/next-server/server/router.ts index cd1c2bd6d8..5360806a09 100644 --- a/packages/next/next-server/server/router.ts +++ b/packages/next/next-server/server/router.ts @@ -194,6 +194,13 @@ export default class Router { currentPathname === '/' ? '' : currentPathname }` + if ( + (req as any).__nextHadTrailingSlash && + !currentPathname.endsWith('/') + ) { + currentPathname += '/' + } + if (keepBasePath) { currentPathname = `${this.basePath}${currentPathname}` } diff --git a/test/integration/i18n-support/next.config.js b/test/integration/i18n-support/next.config.js index 18f254b1ae..cbb272c278 100644 --- a/test/integration/i18n-support/next.config.js +++ b/test/integration/i18n-support/next.config.js @@ -1,6 +1,7 @@ module.exports = { // target: 'experimental-serverless-trace', // basePath: '/docs', + // trailingSlash: true, i18n: { // localeDetection: false, locales: ['nl-NL', 'nl-BE', 'nl', 'fr-BE', 'fr', 'en-US', 'en'], diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js index 5a3107f155..3002246a1a 100644 --- a/test/integration/i18n-support/test/index.test.js +++ b/test/integration/i18n-support/test/index.test.js @@ -1,8 +1,10 @@ +import url from 'url' import http from 'http' import fs from 'fs-extra' import { join } from 'path' import cheerio from 'cheerio' import { runTests, locales } from './shared' +import webdriver from 'next-webdriver' import { nextBuild, nextStart, @@ -275,4 +277,113 @@ describe('i18n Support', () => { } }) }) + + describe('with trailingSlash: true', () => { + const curCtx = { + ...ctx, + isDev: true, + } + beforeAll(async () => { + await fs.remove(join(appDir, '.next')) + nextConfig.replace('// trailingSlash', 'trailingSlash') + + curCtx.appPort = await findPort() + curCtx.app = await launchApp(appDir, curCtx.appPort) + }) + afterAll(async () => { + nextConfig.restore() + await killApp(curCtx.app) + }) + + it('should redirect correctly', async () => { + for (const locale of locales) { + const res = await fetchViaHTTP(curCtx.appPort, '/', undefined, { + redirect: 'manual', + headers: { + 'accept-language': locale, + }, + }) + + if (locale === 'en-US') { + expect(res.status).toBe(200) + } else { + expect(res.status).toBe(307) + + const parsed = url.parse(res.headers.get('location'), true) + expect(parsed.pathname).toBe(`/${locale}`) + expect(parsed.query).toEqual({}) + } + } + }) + + it('should serve pages correctly with locale prefix', async () => { + for (const locale of locales) { + const res = await fetchViaHTTP( + curCtx.appPort, + `/${locale}/`, + undefined, + { + redirect: 'manual', + } + ) + expect(res.status).toBe(200) + + const $ = cheerio.load(await res.text()) + + expect($('#router-pathname').text()).toBe('/') + expect($('#router-as-path').text()).toBe('/') + expect($('#router-locale').text()).toBe(locale) + expect(JSON.parse($('#router-locales').text())).toEqual(locales) + expect($('#router-default-locale').text()).toBe('en-US') + } + }) + + it('should navigate between pages correctly', async () => { + for (const locale of locales) { + const localePath = `/${locale !== 'en-US' ? `${locale}/` : ''}` + const browser = await webdriver(curCtx.appPort, localePath) + + await browser.eval('window.beforeNav = 1') + await browser.elementByCss('#to-gsp').click() + await browser.waitForElementByCss('#gsp') + + expect(await browser.elementByCss('#router-pathname').text()).toBe( + '/gsp' + ) + expect(await browser.elementByCss('#router-as-path').text()).toBe( + '/gsp/' + ) + expect(await browser.elementByCss('#router-locale').text()).toBe(locale) + expect(await browser.eval('window.beforeNav')).toBe(1) + expect(await browser.eval('window.location.pathname')).toBe( + `${localePath}gsp/` + ) + + await browser.back().waitForElementByCss('#index') + + expect(await browser.elementByCss('#router-pathname').text()).toBe('/') + expect(await browser.elementByCss('#router-as-path').text()).toBe('/') + expect(await browser.elementByCss('#router-locale').text()).toBe(locale) + expect(await browser.eval('window.beforeNav')).toBe(1) + expect(await browser.eval('window.location.pathname')).toBe( + `${localePath}` + ) + + await browser.elementByCss('#to-gssp-slug').click() + await browser.waitForElementByCss('#gssp') + + expect(await browser.elementByCss('#router-pathname').text()).toBe( + '/gssp/[slug]' + ) + expect(await browser.elementByCss('#router-as-path').text()).toBe( + '/gssp/first/' + ) + expect(await browser.elementByCss('#router-locale').text()).toBe(locale) + expect(await browser.eval('window.beforeNav')).toBe(1) + expect(await browser.eval('window.location.pathname')).toBe( + `${localePath}gssp/first/` + ) + } + }) + }) }) -- GitLab