From 552fc6d863179d9554c37582dd3a200be6cd7cb4 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 7 Oct 2020 19:54:11 -0500 Subject: [PATCH] Make sure default locale isn't prefixed on the client --- .../webpack/loaders/next-serverless-loader.ts | 1 + packages/next/client/index.tsx | 2 + packages/next/client/link.tsx | 4 +- packages/next/client/page-loader.ts | 29 +++- packages/next/client/router.ts | 1 + packages/next/export/index.ts | 7 +- .../next/next-server/lib/router/router.ts | 32 +++- packages/next/next-server/lib/utils.ts | 1 + .../next/next-server/server/next-server.ts | 2 + packages/next/next-server/server/render.tsx | 11 +- test/integration/i18n-support/next.config.js | 2 +- .../i18n-support/test/index.test.js | 140 ++++++++++++++---- test/lib/next-webdriver.js | 1 + 13 files changed, 190 insertions(+), 43 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 4683e74cc57d6..2621f41005285 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -468,6 +468,7 @@ const nextServerlessLoader: loader.Loader = function () { isDataReq: _nextData, locale: detectedLocale, locales: i18n.locales, + defaultLocale: i18n.defaultLocale, }, options, ) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index bd14886471449..e511d8abb2949 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -65,6 +65,7 @@ const { isFallback, head: initialHeadData, locales, + defaultLocale, } = data let { locale } = data @@ -317,6 +318,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { render({ App, Component, styleSheets, props, err }), locale, locales, + defaultLocale, }) // call init-client middleware diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 76b59444dde70..56da5f68c4c3c 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -332,7 +332,9 @@ function Link(props: React.PropsWithChildren) { // If child is an tag and doesn't have a href attribute, or if the 'passHref' property is // defined, we specify the current 'href', so that repetition is not needed by the user if (props.passHref || (child.type === 'a' && !('href' in child.props))) { - childProps.href = addBasePath(addLocale(as, router && router.locale)) + childProps.href = addBasePath( + addLocale(as, router && router.locale, router && router.defaultLocale) + ) } return React.cloneElement(child, childProps) diff --git a/packages/next/client/page-loader.ts b/packages/next/client/page-loader.ts index 57e8c3374bfb7..f53dc9a1a6171 100644 --- a/packages/next/client/page-loader.ts +++ b/packages/next/client/page-loader.ts @@ -203,13 +203,23 @@ export default class PageLoader { * @param {string} href the route href (file-system path) * @param {string} asPath the URL as shown in browser (virtual path); used for dynamic routes */ - getDataHref(href: string, asPath: string, ssg: boolean, locale?: string) { + getDataHref( + href: string, + asPath: string, + ssg: boolean, + locale?: string, + defaultLocale?: string + ) { const { pathname: hrefPathname, query, search } = parseRelativeUrl(href) const { pathname: asPathname } = parseRelativeUrl(asPath) const route = normalizeRoute(hrefPathname) const getHrefForSlug = (path: string) => { - const dataRoute = addLocale(getAssetPathFromRoute(path, '.json'), locale) + const dataRoute = addLocale( + getAssetPathFromRoute(path, '.json'), + locale, + defaultLocale + ) return addBasePath( `/_next/data/${this.buildId}${dataRoute}${ssg ? '' : search}` ) @@ -229,7 +239,12 @@ export default class PageLoader { * @param {string} href the route href (file-system path) * @param {string} asPath the URL as shown in browser (virtual path); used for dynamic routes */ - prefetchData(href: string, asPath: string) { + prefetchData( + href: string, + asPath: string, + locale?: string, + defaultLocale?: string + ) { const { pathname: hrefPathname } = parseRelativeUrl(href) const route = normalizeRoute(hrefPathname) return this.promisedSsgManifest!.then( @@ -237,7 +252,13 @@ export default class PageLoader { // Check if the route requires a data file s.has(route) && // Try to generate data href, noop when falsy - (_dataHref = this.getDataHref(href, asPath, true)) && + (_dataHref = this.getDataHref( + href, + asPath, + true, + locale, + defaultLocale + )) && // noop when data has already been prefetched (dedupe) !document.querySelector( `link[rel="${relPrefetch}"][href^="${_dataHref}"]` diff --git a/packages/next/client/router.ts b/packages/next/client/router.ts index 54a3f65b378f7..cff79525bd50f 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -39,6 +39,7 @@ const urlPropertyFields = [ 'basePath', 'locale', 'locales', + 'defaultLocale', ] const routerEvents = [ 'routeChangeStart', diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index 056f86c5fb8ee..cc5f14a7a5ffc 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -283,6 +283,8 @@ export default async function exportApp( } } + const { i18n } = nextConfig.experimental + // Start the rendering process const renderOpts = { dir, @@ -298,8 +300,9 @@ export default async function exportApp( ampValidatorPath: nextConfig.experimental.amp?.validator || undefined, ampSkipValidation: nextConfig.experimental.amp?.skipValidation || false, ampOptimizerConfig: nextConfig.experimental.amp?.optimizer || undefined, - locales: nextConfig.experimental.i18n?.locales, - locale: nextConfig.experimental.i18n?.defaultLocale, + locales: i18n?.locales, + locale: i18n.defaultLocale, + defaultLocale: i18n.defaultLocale, } const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index f2614c4545ce3..c6d0916aedd2f 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -55,9 +55,13 @@ function addPathPrefix(path: string, prefix?: string) { : path } -export function addLocale(path: string, locale?: string) { +export function addLocale( + path: string, + locale?: string, + defaultLocale?: string +) { if (process.env.__NEXT_i18n_SUPPORT) { - return locale && !path.startsWith('/' + locale) + return locale && locale !== defaultLocale && !path.startsWith('/' + locale) ? addPathPrefix(path, '/' + locale) : path } @@ -246,6 +250,7 @@ export type BaseRouter = { basePath: string locale?: string locales?: string[] + defaultLocale?: string } export type NextRouter = BaseRouter & @@ -356,6 +361,7 @@ export default class Router implements BaseRouter { _shallow?: boolean locale?: string locales?: string[] + defaultLocale?: string static events: MittEmitter = mitt() @@ -375,6 +381,7 @@ export default class Router implements BaseRouter { isFallback, locale, locales, + defaultLocale, }: { subscription: Subscription initialProps: any @@ -387,6 +394,7 @@ export default class Router implements BaseRouter { isFallback: boolean locale?: string locales?: string[] + defaultLocale?: string } ) { // represents the current component key @@ -440,6 +448,7 @@ export default class Router implements BaseRouter { if (process.env.__NEXT_i18n_SUPPORT) { this.locale = locale this.locales = locales + this.defaultLocale = defaultLocale } if (typeof window !== 'undefined') { @@ -596,7 +605,7 @@ export default class Router implements BaseRouter { this.abortComponentLoad(this._inFlightRoute) } - as = addLocale(as, this.locale) + as = addLocale(as, this.locale, this.defaultLocale) const cleanedAs = delLocale( hasBasePath(as) ? delBasePath(as) : as, this.locale @@ -790,7 +799,12 @@ export default class Router implements BaseRouter { } Router.events.emit('beforeHistoryChange', as) - this.changeState(method, url, addLocale(as, this.locale), options) + this.changeState( + method, + url, + addLocale(as, this.locale, this.defaultLocale), + options + ) if (process.env.NODE_ENV !== 'production') { const appComp: any = this.components['/_app'].Component @@ -960,7 +974,8 @@ export default class Router implements BaseRouter { formatWithValidation({ pathname, query }), delBasePath(as), __N_SSG, - this.locale + this.locale, + this.defaultLocale ) } @@ -1117,7 +1132,12 @@ export default class Router implements BaseRouter { const route = removePathTrailingSlash(pathname) await Promise.all([ - this.pageLoader.prefetchData(url, asPath), + this.pageLoader.prefetchData( + url, + asPath, + this.locale, + this.defaultLocale + ), this.pageLoader[options.priority ? 'loadPage' : 'prefetch'](route), ]) } diff --git a/packages/next/next-server/lib/utils.ts b/packages/next/next-server/lib/utils.ts index a65c74eabd1f4..057188def881c 100644 --- a/packages/next/next-server/lib/utils.ts +++ b/packages/next/next-server/lib/utils.ts @@ -103,6 +103,7 @@ export type NEXT_DATA = { head: HeadEntry[] locale?: string locales?: string[] + defaultLocale?: string } /** diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index df23dbc4a9048..03e5c8f3bd76f 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -140,6 +140,7 @@ export default class Server { optimizeImages: boolean locale?: string locales?: string[] + defaultLocale?: string } private compression?: Middleware private onErrorMiddleware?: ({ err }: { err: Error }) => Promise @@ -193,6 +194,7 @@ export default class Server { : null, optimizeImages: this.nextConfig.experimental.optimizeImages, locales: this.nextConfig.experimental.i18n?.locales, + defaultLocale: this.nextConfig.experimental.i18n?.defaultLocale, } // Only the `publicRuntimeConfig` key is exposed to the client side diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 22d45c812b26e..b809297909318 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -69,6 +69,7 @@ class ServerRouter implements NextRouter { isFallback: boolean locale?: string locales?: string[] + defaultLocale?: string // TODO: Remove in the next major version, as this would mean the user is adding event listeners in server-side `render` method static events: MittEmitter = mitt() @@ -79,7 +80,8 @@ class ServerRouter implements NextRouter { { isFallback }: { isFallback: boolean }, basePath: string, locale?: string, - locales?: string[] + locales?: string[], + defaultLocale?: string ) { this.route = pathname.replace(/\/$/, '') || '/' this.pathname = pathname @@ -89,6 +91,7 @@ class ServerRouter implements NextRouter { this.basePath = basePath this.locale = locale this.locales = locales + this.defaultLocale = defaultLocale } push(): any { noRouter() @@ -164,6 +167,7 @@ export type RenderOptsPartial = { resolvedAsPath?: string locale?: string locales?: string[] + defaultLocale?: string } export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial @@ -203,6 +207,7 @@ function renderDocument( devOnlyCacheBusterQueryString, locale, locales, + defaultLocale, }: RenderOpts & { props: any docComponentsRendered: DocumentProps['docComponentsRendered'] @@ -251,6 +256,7 @@ function renderDocument( appGip, // whether the _app has getInitialProps locale, locales, + defaultLocale, head: React.Children.toArray(docProps.head || []) .map((elem) => { const { children } = elem?.props @@ -517,7 +523,8 @@ export async function renderToHTML( }, basePath, renderOpts.locale, - renderOpts.locales + renderOpts.locales, + renderOpts.defaultLocale ) const ctx = { err, diff --git a/test/integration/i18n-support/next.config.js b/test/integration/i18n-support/next.config.js index 1fb0f8120d470..d759a91263b9f 100644 --- a/test/integration/i18n-support/next.config.js +++ b/test/integration/i18n-support/next.config.js @@ -3,7 +3,7 @@ module.exports = { experimental: { i18n: { locales: ['nl-NL', 'nl-BE', 'nl', 'en-US', 'en'], - defaultLocale: 'en', + defaultLocale: 'en-US', }, }, } diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js index 42b249413e9c7..dff7a80ad0ddf 100644 --- a/test/integration/i18n-support/test/index.test.js +++ b/test/integration/i18n-support/test/index.test.js @@ -24,14 +24,14 @@ let app let appPort // let buildId -const locales = ['en', 'nl-NL', 'nl-BE', 'nl', 'en-US'] +const locales = ['en-US', 'nl-NL', 'nl-BE', 'nl', 'en'] function runTests() { it('should remove un-necessary locale prefix for default locale', async () => { - const res = await fetchViaHTTP(appPort, '/en', undefined, { + const res = await fetchViaHTTP(appPort, '/en-US', undefined, { redirect: 'manual', headers: { - 'Accept-Language': 'en;q=0.9', + 'Accept-Language': 'en-US;q=0.9', }, }) @@ -48,12 +48,12 @@ function runTests() { const $ = cheerio.load(html) expect(JSON.parse($('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, }) - expect($('#router-locale').text()).toBe('en') + expect($('#router-locale').text()).toBe('en-US') expect(JSON.parse($('#router-locales').text())).toEqual(locales) - expect($('html').attr('lang')).toBe('en') + expect($('html').attr('lang')).toBe('en-US') }) it('should load getStaticProps fallback prerender page correctly SSR (default locale no prefix)', async () => { @@ -61,7 +61,7 @@ function runTests() { const $ = cheerio.load(html) expect(JSON.parse($('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, params: { slug: 'first', @@ -70,9 +70,9 @@ function runTests() { expect(JSON.parse($('#router-query').text())).toEqual({ slug: 'first', }) - expect($('#router-locale').text()).toBe('en') + expect($('#router-locale').text()).toBe('en-US') expect(JSON.parse($('#router-locales').text())).toEqual(locales) - expect($('html').attr('lang')).toBe('en') + expect($('html').attr('lang')).toBe('en-US') }) it('should load getStaticProps fallback non-prerender page correctly (default locale no prefix', async () => { @@ -81,7 +81,7 @@ function runTests() { await browser.waitForElementByCss('#props') expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, params: { slug: 'another', @@ -120,14 +120,14 @@ function runTests() { { redirect: 'manual', headers: { - 'Accept-Language': 'en-US,en;q=0.9', + 'Accept-Language': 'en;q=0.9', }, } ) expect(res2.status).toBe(307) const parsedUrl2 = url.parse(res2.headers.get('location'), true) - expect(parsedUrl2.pathname).toBe('/en-US') + expect(parsedUrl2.pathname).toBe('/en') expect(parsedUrl2.query).toEqual({ hello: 'world' }) }) @@ -138,7 +138,7 @@ function runTests() { expect(res.status).toBe(307) const parsedUrl = url.parse(res.headers.get('location'), true) - expect(parsedUrl.pathname).toBe('/en') + expect(parsedUrl.pathname).toBe('/en-US') expect(parsedUrl.query).toEqual({}) const res2 = await fetchViaHTTP( @@ -152,7 +152,7 @@ function runTests() { expect(res2.status).toBe(307) const parsedUrl2 = url.parse(res2.headers.get('location'), true) - expect(parsedUrl2.pathname).toBe('/en') + expect(parsedUrl2.pathname).toBe('/en-US') expect(parsedUrl2.query).toEqual({ hello: 'world' }) }) @@ -170,11 +170,11 @@ function runTests() { }) it('should load getStaticProps fallback prerender page correctly SSR', async () => { - const html = await renderViaHTTP(appPort, '/en/gsp/fallback/first') + const html = await renderViaHTTP(appPort, '/en-US/gsp/fallback/first') const $ = cheerio.load(html) expect(JSON.parse($('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, params: { slug: 'first', @@ -183,9 +183,9 @@ function runTests() { expect(JSON.parse($('#router-query').text())).toEqual({ slug: 'first', }) - expect($('#router-locale').text()).toBe('en') + expect($('#router-locale').text()).toBe('en-US') expect(JSON.parse($('#router-locales').text())).toEqual(locales) - expect($('html').attr('lang')).toBe('en') + expect($('html').attr('lang')).toBe('en-US') }) it('should load getStaticProps fallback non-prerender page correctly', async () => { @@ -210,7 +210,8 @@ function runTests() { JSON.parse(await browser.elementByCss('#router-locales').text()) ).toEqual(locales) - // TODO: handle updating locale for fallback pages? + // TODO: this will be fixed after fallback pages are generated + // for all locales // expect( // await browser.elementByCss('html').getAttribute('lang') // ).toBe('en-US') @@ -221,16 +222,99 @@ function runTests() { const $ = cheerio.load(html) expect(JSON.parse($('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, }) - expect($('#router-locale').text()).toBe('en') + expect($('#router-locale').text()).toBe('en-US') expect(JSON.parse($('#router-locales').text())).toEqual(locales) expect(JSON.parse($('#router-query').text())).toEqual({}) - expect($('html').attr('lang')).toBe('en') + expect($('html').attr('lang')).toBe('en-US') }) - // TODO: client navigation tests for default locale with no prefix + it('should navigate client side for default locale with no prefix', async () => { + const browser = await webdriver(appPort, '/') + // make sure default locale is used in case browser isn't set to + // favor en-US by default + await browser.manage().addCookie({ name: 'NEXT_LOCALE', value: 'en-US' }) + await browser.get(browser.initUrl) + + const checkIndexValues = async () => { + expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ + locale: 'en-US', + locales, + }) + expect(await browser.elementByCss('#router-locale').text()).toBe('en-US') + expect( + JSON.parse(await browser.elementByCss('#router-locales').text()) + ).toEqual(locales) + expect( + JSON.parse(await browser.elementByCss('#router-query').text()) + ).toEqual({}) + expect(await browser.elementByCss('#router-pathname').text()).toBe('/') + expect(await browser.elementByCss('#router-as-path').text()).toBe('/') + expect( + url.parse(await browser.eval(() => window.location.href)).pathname + ).toBe('/') + } + + await checkIndexValues() + + await browser.elementByCss('#to-another').click() + await browser.waitForElementByCss('#another') + + expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ + locale: 'en-US', + locales, + }) + expect(await browser.elementByCss('#router-locale').text()).toBe('en-US') + expect( + JSON.parse(await browser.elementByCss('#router-locales').text()) + ).toEqual(locales) + expect( + JSON.parse(await browser.elementByCss('#router-query').text()) + ).toEqual({}) + expect(await browser.elementByCss('#router-pathname').text()).toBe( + '/another' + ) + expect(await browser.elementByCss('#router-as-path').text()).toBe( + '/another' + ) + expect( + url.parse(await browser.eval(() => window.location.href)).pathname + ).toBe('/another') + + await browser.elementByCss('#to-index').click() + await browser.waitForElementByCss('#index') + + await checkIndexValues() + + await browser.elementByCss('#to-gsp').click() + await browser.waitForElementByCss('#gsp') + + expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ + locale: 'en-US', + locales, + }) + expect(await browser.elementByCss('#router-locale').text()).toBe('en-US') + expect( + JSON.parse(await browser.elementByCss('#router-locales').text()) + ).toEqual(locales) + expect( + JSON.parse(await browser.elementByCss('#router-query').text()) + ).toEqual({}) + expect(await browser.elementByCss('#router-pathname').text()).toBe('/gsp') + expect(await browser.elementByCss('#router-as-path').text()).toBe('/gsp') + expect( + url.parse(await browser.eval(() => window.location.href)).pathname + ).toBe('/gsp') + + await browser.elementByCss('#to-index').click() + await browser.waitForElementByCss('#index') + + await checkIndexValues() + + await browser.manage().deleteCookie('NEXT_LOCALE') + }) it('should load getStaticProps fallback non-prerender page another locale correctly', async () => { const browser = await webdriver(appPort, '/nl-NL/gsp/fallback/another') @@ -256,12 +340,12 @@ function runTests() { }) it('should load getStaticProps non-fallback correctly', async () => { - const browser = await webdriver(appPort, '/en/gsp/no-fallback/first') + const browser = await webdriver(appPort, '/en-US/gsp/no-fallback/first') await browser.waitForElementByCss('#props') expect(JSON.parse(await browser.elementByCss('#props').text())).toEqual({ - locale: 'en', + locale: 'en-US', locales, params: { slug: 'first', @@ -272,11 +356,13 @@ function runTests() { ).toEqual({ slug: 'first', }) - expect(await browser.elementByCss('#router-locale').text()).toBe('en') + expect(await browser.elementByCss('#router-locale').text()).toBe('en-US') expect( JSON.parse(await browser.elementByCss('#router-locales').text()) ).toEqual(locales) - expect(await browser.elementByCss('html').getAttribute('lang')).toBe('en') + expect(await browser.elementByCss('html').getAttribute('lang')).toBe( + 'en-US' + ) }) it('should load getStaticProps non-fallback correctly another locale', async () => { diff --git a/test/lib/next-webdriver.js b/test/lib/next-webdriver.js index e3b8e6ed6c2f2..9ad87b8b22375 100644 --- a/test/lib/next-webdriver.js +++ b/test/lib/next-webdriver.js @@ -167,6 +167,7 @@ export default async (appPort, path, waitHydration = true) => { } const url = `http://${deviceIP}:${appPort}${path}` + browser.initUrl = url console.log(`\n> Loading browser with ${url}\n`) await browser.get(url)