From 85110e823594113fa0b5f568bad58cd0da8ecf73 Mon Sep 17 00:00:00 2001 From: Naoyuki Kanezawa Date: Thu, 7 Apr 2022 18:39:49 +0700 Subject: [PATCH] fix: add locale prefix to middleware preflight url (#35911) Fixes part of https://github.com/vercel/next.js/issues/34274 Navigating to `/` causes to redirect preflight request to a url of browser language like `/en`. This PR fixes to add the locale prefix always so that the redirect does not happen anymore and middleware can get a correct locale. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint` --- packages/next/shared/lib/router/router.ts | 4 +- .../with-i18n-preflight/next.config.js | 6 ++ .../with-i18n-preflight/pages/_middleware.js | 7 ++ .../with-i18n-preflight/pages/index.js | 40 +++++++++++ .../with-i18n-preflight/test/index.test.js | 69 +++++++++++++++++++ test/lib/browsers/base.ts | 2 +- test/lib/browsers/playwright.ts | 4 +- test/lib/browsers/selenium.ts | 3 +- test/lib/next-webdriver.ts | 24 ++++--- 9 files changed, 146 insertions(+), 13 deletions(-) create mode 100644 test/integration/middleware/with-i18n-preflight/next.config.js create mode 100644 test/integration/middleware/with-i18n-preflight/pages/_middleware.js create mode 100644 test/integration/middleware/with-i18n-preflight/pages/index.js create mode 100644 test/integration/middleware/with-i18n-preflight/test/index.test.js diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 2816ab180e804a4..35ef048c771de4d 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1897,10 +1897,12 @@ export default class Router implements BaseRouter { return { type: 'next' } } + const preflightHref = addLocale(options.as, options.locale) + let preflight: PreflightData | undefined try { preflight = await this._getPreflightData({ - preflightHref: options.as, + preflightHref, shouldCache: options.cache, isPreview: options.isPreview, }) diff --git a/test/integration/middleware/with-i18n-preflight/next.config.js b/test/integration/middleware/with-i18n-preflight/next.config.js new file mode 100644 index 000000000000000..51c5585977f1926 --- /dev/null +++ b/test/integration/middleware/with-i18n-preflight/next.config.js @@ -0,0 +1,6 @@ +module.exports = { + i18n: { + locales: ['ja', 'en', 'fr'], + defaultLocale: 'ja', + }, +} diff --git a/test/integration/middleware/with-i18n-preflight/pages/_middleware.js b/test/integration/middleware/with-i18n-preflight/pages/_middleware.js new file mode 100644 index 000000000000000..3c3baeee67a0b02 --- /dev/null +++ b/test/integration/middleware/with-i18n-preflight/pages/_middleware.js @@ -0,0 +1,7 @@ +import { NextResponse } from 'next/server' + +export function middleware(req) { + const url = req.nextUrl.clone() + url.searchParams.set('locale', url.locale) + return NextResponse.rewrite(url) +} diff --git a/test/integration/middleware/with-i18n-preflight/pages/index.js b/test/integration/middleware/with-i18n-preflight/pages/index.js new file mode 100644 index 000000000000000..60ada1770af049e --- /dev/null +++ b/test/integration/middleware/with-i18n-preflight/pages/index.js @@ -0,0 +1,40 @@ +import Link from 'next/link' + +export default function Home({ locale }) { + return ( +
+
{locale}
+ +
+ ) +} + +export const getServerSideProps = ({ query }) => ({ + props: { locale: query.locale }, +}) diff --git a/test/integration/middleware/with-i18n-preflight/test/index.test.js b/test/integration/middleware/with-i18n-preflight/test/index.test.js new file mode 100644 index 000000000000000..ceee0a39474d6fb --- /dev/null +++ b/test/integration/middleware/with-i18n-preflight/test/index.test.js @@ -0,0 +1,69 @@ +/* eslint-env jest */ + +import { join } from 'path' +import webdriver, { USE_SELENIUM } from 'next-webdriver' +import { + findPort, + killApp, + launchApp, + nextBuild, + nextStart, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) + +const context = {} +context.appDir = join(__dirname, '../') + +const itif = (condition) => (condition ? it : it.skip) + +describe('Middleware base tests', () => { + describe('dev mode', () => { + beforeAll(async () => { + context.appPort = await findPort() + context.app = await launchApp(context.appDir, context.appPort) + }) + + afterAll(() => killApp(context.app)) + + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await nextBuild(context.appDir, undefined, { + stderr: true, + stdout: true, + }) + + context.appPort = await findPort() + context.app = await nextStart(context.appDir, context.appPort) + }) + + afterAll(() => killApp(context.app)) + + runTests() + }) +}) + +function runTests() { + itif(!USE_SELENIUM)( + `should send preflight for specified locale`, + async () => { + const browser = await webdriver(context.appPort, '/', { + locale: 'en-US,en', + }) + await browser.waitForElementByCss('.en') + await browser.elementByCss('#link-ja').click() + await browser.waitForElementByCss('.ja') + await browser.elementByCss('#link-en').click() + await browser.waitForElementByCss('.en') + await browser.elementByCss('#link-fr').click() + await browser.waitForElementByCss('.fr') + await browser.elementByCss('#link-ja2').click() + await browser.waitForElementByCss('.ja') + await browser.elementByCss('#link-en2').click() + await browser.waitForElementByCss('.en') + } + ) +} diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts index 0f0065f6039adb6..a456d884d372731 100644 --- a/test/lib/browsers/base.ts +++ b/test/lib/browsers/base.ts @@ -18,7 +18,7 @@ export class BrowserInterface { return this } - async setup(browserName: string): Promise {} + async setup(browserName: string, locale?: string): Promise {} async close(): Promise {} async quit(): Promise {} diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index d9ffafb4ff81dd1..9099ed8102ec72d 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -46,7 +46,7 @@ class Playwright extends BrowserInterface { this.eventCallbacks[event]?.delete(cb) } - async setup(browserName: string) { + async setup(browserName: string, locale?: string) { if (browser) return const headless = !!process.env.HEADLESS @@ -57,7 +57,7 @@ class Playwright extends BrowserInterface { } else { browser = await chromium.launch({ headless, devtools: !headless }) } - context = await browser.newContext() + context = await browser.newContext({ locale }) } async get(url: string): Promise { diff --git a/test/lib/browsers/selenium.ts b/test/lib/browsers/selenium.ts index b292839c21799db..b85f0c6b148a538 100644 --- a/test/lib/browsers/selenium.ts +++ b/test/lib/browsers/selenium.ts @@ -45,7 +45,8 @@ export async function quit() { class Selenium extends BrowserInterface { private browserName: string - async setup(browserName: string) { + // TODO: support setting locale + async setup(browserName: string, locale?: string) { if (browser) return this.browserName = browserName diff --git a/test/lib/next-webdriver.ts b/test/lib/next-webdriver.ts index 2627ed17bdc1c5d..e05ada504130b3a 100644 --- a/test/lib/next-webdriver.ts +++ b/test/lib/next-webdriver.ts @@ -36,6 +36,12 @@ if (typeof afterAll === 'function') { }) } +export const USE_SELENIUM = Boolean( + process.env.LEGACY_SAFARI || + process.env.BROWSER_NAME === 'internet explorer' || + process.env.SKIP_LOCAL_SELENIUM_SERVER +) + /** * * @param appPort can either be the port or the full URL @@ -54,6 +60,7 @@ export default async function webdriver( retryWaitHydration?: boolean disableCache?: boolean beforePageLoad?: (page: any) => void + locale?: string } ): Promise { let CurrentInterface: typeof BrowserInterface @@ -64,15 +71,16 @@ export default async function webdriver( disableCache: false, } options = Object.assign(defaultOptions, options) - const { waitHydration, retryWaitHydration, disableCache, beforePageLoad } = - options + const { + waitHydration, + retryWaitHydration, + disableCache, + beforePageLoad, + locale, + } = options // we import only the needed interface - if ( - process.env.LEGACY_SAFARI || - process.env.BROWSER_NAME === 'internet explorer' || - process.env.SKIP_LOCAL_SELENIUM_SERVER - ) { + if (USE_SELENIUM) { const browserMod = require('./browsers/selenium') CurrentInterface = browserMod.default browserQuit = browserMod.quit @@ -84,7 +92,7 @@ export default async function webdriver( const browser = new CurrentInterface() const browserName = process.env.BROWSER_NAME || 'chrome' - await browser.setup(browserName) + await browser.setup(browserName, locale) ;(global as any).browserName = browserName const fullUrl = getFullUrl(