Skip to content

Commit

Permalink
fix: add locale prefix to middleware preflight url (vercel#35911)
Browse files Browse the repository at this point in the history
Fixes part of vercel#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`
  • Loading branch information
nkzawa authored and colinhacks committed Apr 14, 2022
1 parent c4859f9 commit 85110e8
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 13 deletions.
4 changes: 3 additions & 1 deletion packages/next/shared/lib/router/router.ts
Expand Up @@ -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,
})
Expand Down
@@ -0,0 +1,6 @@
module.exports = {
i18n: {
locales: ['ja', 'en', 'fr'],
defaultLocale: 'ja',
},
}
@@ -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)
}
40 changes: 40 additions & 0 deletions test/integration/middleware/with-i18n-preflight/pages/index.js
@@ -0,0 +1,40 @@
import Link from 'next/link'

export default function Home({ locale }) {
return (
<div>
<div className={locale}>{locale}</div>
<ul>
<li>
<Link href="/" locale="en">
<a id="link-en">Go to en</a>
</Link>
</li>
<li>
<Link href="/en" locale={false}>
<a id="link-en2">Go to en2</a>
</Link>
</li>
<li>
<Link href="/" locale="ja">
<a id="link-ja">Go to ja</a>
</Link>
</li>
<li>
<Link href="/ja" locale={false}>
<a id="link-ja2">Go to ja2</a>
</Link>
</li>
<li>
<Link href="/" locale="fr">
<a id="link-fr">Go to fr</a>
</Link>
</li>
</ul>
</div>
)
}

export const getServerSideProps = ({ query }) => ({
props: { locale: query.locale },
})
69 changes: 69 additions & 0 deletions 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')
}
)
}
2 changes: 1 addition & 1 deletion test/lib/browsers/base.ts
Expand Up @@ -18,7 +18,7 @@ export class BrowserInterface {
return this
}

async setup(browserName: string): Promise<void> {}
async setup(browserName: string, locale?: string): Promise<void> {}
async close(): Promise<void> {}
async quit(): Promise<void> {}

Expand Down
4 changes: 2 additions & 2 deletions test/lib/browsers/playwright.ts
Expand Up @@ -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

Expand All @@ -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<void> {
Expand Down
3 changes: 2 additions & 1 deletion test/lib/browsers/selenium.ts
Expand Up @@ -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

Expand Down
24 changes: 16 additions & 8 deletions test/lib/next-webdriver.ts
Expand Up @@ -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
Expand All @@ -54,6 +60,7 @@ export default async function webdriver(
retryWaitHydration?: boolean
disableCache?: boolean
beforePageLoad?: (page: any) => void
locale?: string
}
): Promise<BrowserInterface> {
let CurrentInterface: typeof BrowserInterface
Expand All @@ -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
Expand All @@ -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(
Expand Down

0 comments on commit 85110e8

Please sign in to comment.