From b4b5c5d283799873bb424d06f03f332db5279923 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 3 Mar 2021 17:06:16 -0600 Subject: [PATCH 1/3] Ensure i18n index prefetch is correct with trailingSlash --- packages/next/build/index.ts | 1 - packages/next/client/page-loader.ts | 6 +- packages/next/export/index.ts | 3 +- packages/next/export/worker.ts | 5 + test/integration/i18n-support/pages/mixed.js | 5 + .../i18n-support/test/index.test.js | 266 +++++++++++------- test/integration/i18n-support/test/shared.js | 2 +- 7 files changed, 185 insertions(+), 103 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 2ec1573b9c531..f6135d2ac6c7c 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -1036,7 +1036,6 @@ export default async function build( } return defaultMap }, - trailingSlash: false, } await exportApp(dir, exportOptions, exportConfig) diff --git a/packages/next/client/page-loader.ts b/packages/next/client/page-loader.ts index b95c0e7e331c2..6f6615a060883 100644 --- a/packages/next/client/page-loader.ts +++ b/packages/next/client/page-loader.ts @@ -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, @@ -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}` ) diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index 055118cfcf396..92099932e5d34 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -167,7 +167,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) { @@ -360,6 +360,7 @@ export default async function exportApp( locale: i18n?.defaultLocale, defaultLocale: i18n?.defaultLocale, domainLocales: i18n?.domains, + trailingSlash: nextConfig.trailingSlash, } const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index ca2eaa5734db9..378051f24fd1a 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -77,6 +77,7 @@ interface RenderOpts { locales?: string[] locale?: string defaultLocale?: string + trailingSlash?: boolean } type ComponentModule = ComponentType<{}> & { @@ -193,6 +194,10 @@ export default async function exportPage({ res.statusCode = 500 } + if (renderOpts.trailingSlash && !req.url?.endsWith('/')) { + req.url += '/' + } + envConfig.setConfig({ serverRuntimeConfig, publicRuntimeConfig: renderOpts.runtimeConfig, diff --git a/test/integration/i18n-support/pages/mixed.js b/test/integration/i18n-support/pages/mixed.js index 5b830abe6b937..f1939e78f852f 100644 --- a/test/integration/i18n-support/pages/mixed.js +++ b/test/integration/i18n-support/pages/mixed.js @@ -28,6 +28,11 @@ export default function Page(props) { to /gsp +
+ + + to / + ) } diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js index f78a55786b574..480c28d707130 100644 --- a/test/integration/i18n-support/test/index.test.js +++ b/test/integration/i18n-support/test/index.test.js @@ -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')) @@ -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) }) }) }) diff --git a/test/integration/i18n-support/test/shared.js b/test/integration/i18n-support/test/shared.js index 9a554b7437125..b6d43b0bfecb9 100644 --- a/test/integration/i18n-support/test/shared.js +++ b/test/integration/i18n-support/test/shared.js @@ -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') From b8b87c16a6e216fac6db4dd1de2ab5971f652d07 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 3 Mar 2021 17:15:07 -0600 Subject: [PATCH 2/3] Update mixed page for i18n + basePath --- test/integration/i18n-support-base-path/pages/mixed.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/integration/i18n-support-base-path/pages/mixed.js b/test/integration/i18n-support-base-path/pages/mixed.js index 5b830abe6b937..f1939e78f852f 100644 --- a/test/integration/i18n-support-base-path/pages/mixed.js +++ b/test/integration/i18n-support-base-path/pages/mixed.js @@ -28,6 +28,11 @@ export default function Page(props) { to /gsp +
+ + + to / + ) } From 33a812671d12c5cf63347d7b72ccab2e717609dc Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 3 Mar 2021 17:34:14 -0600 Subject: [PATCH 3/3] Update build-output test --- test/integration/build-output/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 5c592ebee4487..433a9af5811d8 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -104,7 +104,7 @@ describe('Build Output', () => { expect(parseFloat(err404FirstLoad)).toBeCloseTo(67.1, 0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll)).toBeCloseTo(63.8, 1) + expect(parseFloat(sharedByAll)).toBeCloseTo(63.9, 1) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) {