Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure index rewrite is matched with i18n correctly #20509

Merged
merged 4 commits into from
Dec 28, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/next/lib/load-custom-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,9 @@ function processRoutes<T>(

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${
Expand Down
5 changes: 4 additions & 1 deletion packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
31 changes: 31 additions & 0 deletions test/integration/i18n-support-index-rewrite/next.config.js
Original file line number Diff line number Diff line change
@@ -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: '/',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also match /nl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it matches all locales unless locale: false is specified, updated tests to reflect/test this

destination: '/company/about-us',
},
]
},
}
42 changes: 42 additions & 0 deletions test/integration/i18n-support-index-rewrite/pages/[...slug].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React from 'react'

const DynamicPage = (props) => (
<>
<div>DynamicPage</div>
<div id="props">{JSON.stringify(props)}</div>
</>
)

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
97 changes: 97 additions & 0 deletions test/integration/i18n-support-index-rewrite/test/index.test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})