From 358861d3153d6192786293c34c8694db75d78732 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 7 Dec 2020 11:36:46 -0600 Subject: [PATCH] Ensure trailingSlash redirect applies correctly for i18n (#19859) This ensures locales aren't applied for the trailing slash redirect un-necessarily Fixes: https://github.com/vercel/next.js/issues/19784 --- packages/next/lib/load-custom-routes.ts | 5 +++++ .../next/next-server/server/next-server.ts | 1 + packages/next/next-server/server/router.ts | 7 ++++++- .../custom-routes/test/index.test.js | 1 + test/integration/i18n-support/test/shared.js | 20 +++++++++++++++++++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index bf7372db39..9b5fcb9e3b 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -466,12 +466,14 @@ export default async function loadCustomRoutes( destination: '/:file', permanent: true, locale: config.i18n ? false : undefined, + internal: true, } as Redirect, { source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)', destination: '/:notfile/', permanent: true, locale: config.i18n ? false : undefined, + internal: true, } as Redirect ) if (config.basePath) { @@ -481,6 +483,7 @@ export default async function loadCustomRoutes( permanent: true, basePath: false, locale: config.i18n ? false : undefined, + internal: true, } as Redirect) } } else { @@ -489,6 +492,7 @@ export default async function loadCustomRoutes( destination: '/:path+', permanent: true, locale: config.i18n ? false : undefined, + internal: true, } as Redirect) if (config.basePath) { redirects.unshift({ @@ -497,6 +501,7 @@ export default async function loadCustomRoutes( permanent: true, basePath: false, locale: config.i18n ? false : undefined, + internal: true, } as Redirect) } } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index d766f51a1c..622c615099 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -720,6 +720,7 @@ export default class Server { const redirects = this.customRoutes.redirects.map((redirect) => { const redirectRoute = getCustomRoute(redirect, 'redirect') return { + internal: redirectRoute.internal, type: redirectRoute.type, match: redirectRoute.match, statusCode: redirectRoute.statusCode, diff --git a/packages/next/next-server/server/router.ts b/packages/next/next-server/server/router.ts index effed50a4e..e8b22b09d1 100644 --- a/packages/next/next-server/server/router.ts +++ b/packages/next/next-server/server/router.ts @@ -24,6 +24,7 @@ export type Route = { statusCode?: number name: string requireBasePath?: false + internal?: true fn: ( req: IncomingMessage, res: ServerResponse, @@ -197,7 +198,11 @@ export default class Router { const activeBasePath = keepBasePath ? this.basePath : '' if (keepLocale) { - if (!localePathResult.detectedLocale && parsedUrl.query.__nextLocale) { + if ( + !testRoute.internal && + parsedUrl.query.__nextLocale && + !localePathResult.detectedLocale + ) { currentPathname = `${activeBasePath}/${parsedUrl.query.__nextLocale}${ currentPathnameNoBasePath === '/' ? '' : currentPathnameNoBasePath }` diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 4c23b44882..eba6e200df 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -644,6 +644,7 @@ const runTests = (isDev = false) => { ), source: '/:path+/', statusCode: 308, + internal: true, }, { destination: '/:lang/about', diff --git a/test/integration/i18n-support/test/shared.js b/test/integration/i18n-support/test/shared.js index b6ceb35b48..b4c41bf022 100644 --- a/test/integration/i18n-support/test/shared.js +++ b/test/integration/i18n-support/test/shared.js @@ -406,6 +406,26 @@ export function runTests(ctx) { } }) + it('should apply trailingSlash redirect correctly', async () => { + for (const [testPath, path, hostname, query] of [ + ['/first/', '/first', 'localhost', {}], + ['/en/', '/en', 'localhost', {}], + ['/en/another/', '/en/another', 'localhost', {}], + ['/fr/', '/fr', 'localhost', {}], + ['/fr/another/', '/fr/another', 'localhost', {}], + ]) { + const res = await fetchViaHTTP(ctx.appPort, testPath, undefined, { + redirect: 'manual', + }) + expect(res.status).toBe(308) + + const parsed = url.parse(res.headers.get('location'), true) + expect(parsed.pathname).toBe(path) + expect(parsed.hostname).toBe(hostname) + expect(parsed.query).toEqual(query) + } + }) + it('should apply redirects correctly', async () => { for (const [path, shouldRedirect, locale] of [ ['/en-US/redirect-1', true], -- GitLab