Skip to content

Commit

Permalink
Ensure i18n index prefetch is correct with trailingSlash (#22746)
Browse files Browse the repository at this point in the history 
This fixes index data route loading for i18n with `trailingSlash: true` enabled and also fixes prerendering `asPath` values not containing a trailingSlash when enabled. 


Fixes: #17813
Fixes: #22747
  • Loading branch information
@ijjk
ijjk committed Mar 14, 2021
1 parent c1b2b3f commit 9afcdfc
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 103 deletions.
1 change: 0 additions & 1 deletion packages/next/build/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,6 @@ export default async function build(
}
return defaultMap
},
trailingSlash: false,
}

await exportApp(dir, exportOptions, exportConfig)
Expand Down
6 changes: 5 additions & 1 deletion packages/next/client/page-loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
import getAssetPathFromRoute from '../next-server/lib/router/utils/get-asset-path-from-route'
import { isDynamicRoute } from '../next-server/lib/router/utils/is-dynamic'
import { parseRelativeUrl } from '../next-server/lib/router/utils/parse-relative-url'
import { removePathTrailingSlash } from './normalize-trailing-slash'
import createRouteLoader, {
getClientBuildManifest,
RouteLoader,
Expand Down Expand Up @@ -96,7 +97,10 @@ export default class PageLoader {
const route = normalizeRoute(hrefPathname)

const getHrefForSlug = (path: string) => {
const dataRoute = getAssetPathFromRoute(addLocale(path, locale), '.json')
const dataRoute = getAssetPathFromRoute(
removePathTrailingSlash(addLocale(path, locale)),
'.json'
)
return addBasePath(
`/_next/data/${this.buildId}${dataRoute}${ssg ? '' : search}`
)
Expand Down
3 changes: 2 additions & 1 deletion packages/next/export/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export default async function exportApp(
)
}

const subFolders = nextConfig.trailingSlash
const subFolders = nextConfig.trailingSlash && !options.buildExport
const isLikeServerless = nextConfig.target !== 'server'

if (!options.silent && !options.buildExport) {
Expand Down Expand Up @@ -367,6 +367,7 @@ export default async function exportApp(
locale: i18n?.defaultLocale,
defaultLocale: i18n?.defaultLocale,
domainLocales: i18n?.domains,
trailingSlash: nextConfig.trailingSlash,
}

const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig
Expand Down
5 changes: 5 additions & 0 deletions packages/next/export/worker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ interface RenderOpts {
locales?: string[]
locale?: string
defaultLocale?: string
trailingSlash?: boolean
}

type ComponentModule = ComponentType<{}> & {
Expand Down Expand Up @@ -184,6 +185,10 @@ export default async function exportPage({
res.statusCode = 500
}

if (renderOpts.trailingSlash && !req.url?.endsWith('/')) {
req.url += '/'
}

envConfig.setConfig({
serverRuntimeConfig,
publicRuntimeConfig: renderOpts.runtimeConfig,
Expand Down
5 changes: 5 additions & 0 deletions test/integration/i18n-support-base-path/pages/mixed.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export default function Page(props) {
<Link href="/gsp" locale="fr">
<a id="to-gsp-fr">to /gsp</a>
</Link>
<br />

<Link href="/" locale="fr">
<a id="to-index-fr">to /</a>
</Link>
</>
)
}
5 changes: 5 additions & 0 deletions test/integration/i18n-support/pages/mixed.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export default function Page(props) {
<Link href="/gsp" locale="fr">
<a id="to-gsp-fr">to /gsp</a>
</Link>
<br />

<Link href="/" locale="fr">
<a id="to-index-fr">to /</a>
</Link>
</>
)
}
266 changes: 167 additions & 99 deletions test/integration/i18n-support/test/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import {
fetchViaHTTP,
File,
launchApp,
check,
} from 'next-test-utils'
import assert from 'assert'

const appDir = join(__dirname, '../')
const nextConfig = new File(join(appDir, 'next.config.js'))
Expand Down Expand Up @@ -291,121 +293,187 @@ 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)
})
const runSlashTests = (curCtx) => {
if (!curCtx.isDev) {
it('should preload all locales data correctly', async () => {
const browser = await webdriver(
curCtx.appPort,
`${curCtx.basePath}/mixed`
)

it('should redirect correctly', async () => {
for (const locale of nonDomainLocales) {
const res = await fetchViaHTTP(curCtx.appPort, '/', undefined, {
redirect: 'manual',
headers: {
'accept-language': locale,
},
await browser.eval(`(function() {
document.querySelector('#to-gsp-en-us').scrollIntoView()
document.querySelector('#to-gsp-nl-nl').scrollIntoView()
document.querySelector('#to-gsp-fr').scrollIntoView()
})()`)

await check(async () => {
const hrefs = await browser.eval(
`Object.keys(window.next.router.sdc)`
)
hrefs.sort()

assert.deepEqual(
hrefs.map((href) =>
new URL(href).pathname
.replace(ctx.basePath, '')
.replace(/^\/_next\/data\/[^/]+/, '')
),
['/en-US/gsp.json', '/fr.json', '/fr/gsp.json', '/nl-NL/gsp.json']
)
return 'yes'
}, 'yes')
})
}

if (locale === 'en-US') {
expect(res.status).toBe(200)
} else {
expect(res.status).toBe(307)
it('should redirect correctly', async () => {
for (const locale of nonDomainLocales) {
const res = await fetchViaHTTP(curCtx.appPort, '/', undefined, {
redirect: 'manual',
headers: {
'accept-language': locale,
},
})

const parsed = url.parse(res.headers.get('location'), true)
expect(parsed.pathname).toBe(`/${locale}`)
expect(parsed.query).toEqual({})
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 nonDomainLocales) {
for (const [pathname, asPath] of [
['/', '/'],
['/links', '/links/'],
['/auto-export', '/auto-export/'],
['/gsp', '/gsp/'],
['/gsp/fallback/[slug]', '/gsp/fallback/always/'],
['/gssp', '/gssp/'],
['/gssp/[slug]', '/gssp/first/'],
]) {
const res = await fetchViaHTTP(
curCtx.appPort,
`${locale === 'en-US' ? '' : `/${locale}`}${asPath}`,
undefined,
{
redirect: 'manual',
}
)
expect(res.status).toBe(200)
it('should serve pages correctly with locale prefix', async () => {
for (const locale of nonDomainLocales) {
for (const [pathname, asPath] of [
['/', '/'],
['/links', '/links/'],
['/auto-export', '/auto-export/'],
['/gsp', '/gsp/'],
['/gsp/fallback/[slug]', '/gsp/fallback/always/'],
['/gssp', '/gssp/'],
['/gssp/[slug]', '/gssp/first/'],
]) {
const res = await fetchViaHTTP(
curCtx.appPort,
`${locale === 'en-US' ? '' : `/${locale}`}${asPath}`,
undefined,
{
redirect: 'manual',
}
)
expect(res.status).toBe(200)

const $ = cheerio.load(await res.text())

expect($('#router-pathname').text()).toBe(pathname)
expect($('#router-as-path').text()).toBe(asPath)
expect($('#router-locale').text()).toBe(locale)
expect(JSON.parse($('#router-locales').text())).toEqual(locales)
expect($('#router-default-locale').text()).toBe('en-US')
}
}
})

const $ = cheerio.load(await res.text())
it('should navigate between pages correctly', async () => {
for (const locale of nonDomainLocales) {
const localePath = `/${locale !== 'en-US' ? `${locale}/` : ''}`
const browser = await webdriver(curCtx.appPort, localePath)

expect($('#router-pathname').text()).toBe(pathname)
expect($('#router-as-path').text()).toBe(asPath)
expect($('#router-locale').text()).toBe(locale)
expect(JSON.parse($('#router-locales').text())).toEqual(locales)
expect($('#router-default-locale').text()).toBe('en-US')
}
}
})
await browser.eval('window.beforeNav = 1')
await browser.elementByCss('#to-gsp').click()
await browser.waitForElementByCss('#gsp')

it('should navigate between pages correctly', async () => {
for (const locale of nonDomainLocales) {
const localePath = `/${locale !== 'en-US' ? `${locale}/` : ''}`
const browser = await webdriver(curCtx.appPort, localePath)
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.eval('window.beforeNav = 1')
await browser.elementByCss('#to-gsp').click()
await browser.waitForElementByCss('#gsp')
await browser.back().waitForElementByCss('#index')

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/`
)
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.back().waitForElementByCss('#index')
await browser.elementByCss('#to-gssp-slug').click()
await browser.waitForElementByCss('#gssp')

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}`
)
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/`
)
}
})
}

await browser.elementByCss('#to-gssp-slug').click()
await browser.waitForElementByCss('#gssp')
describe('dev mode', () => {
const curCtx = {
...ctx,
isDev: true,
}
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
nextConfig.replace('// trailingSlash', 'trailingSlash')

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/`
)
curCtx.appPort = await findPort()
curCtx.app = await launchApp(appDir, curCtx.appPort)
})
afterAll(async () => {
nextConfig.restore()
await killApp(curCtx.app)
})

runSlashTests(curCtx)
})

describe('production mode', () => {
const curCtx = {
...ctx,
}
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
nextConfig.replace('// trailingSlash', 'trailingSlash')

await nextBuild(appDir)
curCtx.appPort = await findPort()
curCtx.app = await nextStart(appDir, curCtx.appPort)
})
afterAll(async () => {
nextConfig.restore()
await killApp(curCtx.app)
})

runSlashTests(curCtx)
})
})
})
2 changes: 1 addition & 1 deletion test/integration/i18n-support/test/shared.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ export function runTests(ctx) {
.replace(ctx.basePath, '')
.replace(/^\/_next\/data\/[^/]+/, '')
),
['/en-US/gsp.json', '/fr/gsp.json', '/nl-NL/gsp.json']
['/en-US/gsp.json', '/fr.json', '/fr/gsp.json', '/nl-NL/gsp.json']
)
return 'yes'
}, 'yes')
Expand Down

0 comments on commit 9afcdfc

Please sign in to comment.