From e58b5f155a19e32ac10dc90319591c8b521bf94d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 15 Oct 2020 14:12:18 -0500 Subject: [PATCH 1/3] Ensure i18n support with AMP --- packages/next/build/index.ts | 2 +- .../webpack/loaders/next-serverless-loader.ts | 12 +++++ packages/next/export/worker.ts | 19 +++++--- .../next/next-server/server/next-server.ts | 1 + packages/next/next-server/server/render.tsx | 4 ++ .../i18n-support/pages/amp/amp-first.js | 32 +++++++++++++ .../i18n-support/pages/amp/amp-hybrid.js | 23 +++++++++ .../i18n-support/test/index.test.js | 47 +++++++++++++++++++ 8 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 test/integration/i18n-support/pages/amp/amp-first.js create mode 100644 test/integration/i18n-support/pages/amp/amp-hybrid.js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 0633784c85e7103..4b2a41a1ee19c37 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -785,7 +785,7 @@ export default async function build( const outputPath = `/${locale}${page === '/' ? '' : page}` defaultMap[outputPath] = { - page: defaultMap[page].page, + page: defaultMap[page]?.page || page, query: { __nextLocale: locale }, } diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index f38f687558a600d..4d56c172e8080f9 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -533,6 +533,18 @@ const nextServerlessLoader: loader.Loader = function () { ${handleBasePath} + // remove ?amp=1 from request URL if rendering for export + if (fromExport && parsedUrl.query.amp) { + const queryNoAmp = Object.assign({}, origQuery) + delete queryNoAmp.amp + + req.url = formatUrl({ + ...parsedUrl, + search: undefined, + query: queryNoAmp + }) + } + if (parsedUrl.pathname.match(/_next\\/data/)) { const { default: getrouteNoAssetPath, diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index 379121835242f7e..f5bdcc02f7e8cd1 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -70,6 +70,7 @@ interface RenderOpts { fontManifest?: FontManifest locales?: string[] locale?: string + defaultLocale?: string } type ComponentModule = ComponentType<{}> & { @@ -99,8 +100,9 @@ export default async function exportPage({ const { query: originalQuery = {} } = pathMap const { page } = pathMap const filePath = normalizePagePath(path) - const ampPath = `${filePath}.amp` const isDynamic = isDynamicRoute(page) + const ampPath = `${filePath}.amp` + let renderAmpPath = ampPath let query = { ...originalQuery } let params: { [key: string]: string | string[] } | undefined @@ -114,6 +116,10 @@ export default async function exportPage({ if (localePathResult.detectedLocale) { updatedPath = localePathResult.pathname locale = localePathResult.detectedLocale + + if (locale === renderOpts.defaultLocale) { + renderAmpPath = `${normalizePagePath(updatedPath)}.amp` + } } } @@ -238,7 +244,7 @@ export default async function exportPage({ res, 'export', { - ampPath, + ampPath: renderAmpPath, /// @ts-ignore optimizeFonts, /// @ts-ignore @@ -298,7 +304,7 @@ export default async function exportPage({ curRenderOpts = { ...components, ...renderOpts, - ampPath, + ampPath: renderAmpPath, params, optimizeFonts, optimizeImages, @@ -352,15 +358,16 @@ export default async function exportPage({ if (serverless) { req.url += (req.url!.includes('?') ? '&' : '?') + 'amp=1' // @ts-ignore - ampHtml = (await renderMethod(req, res, 'export', undefined, params)) - .html + ampHtml = ( + await renderMethod(req, res, 'export', curRenderOpts, params) + ).html } else { ampHtml = await renderMethod( req, res, page, // @ts-ignore - { ...query, amp: 1 }, + { ...query, amp: '1' }, curRenderOpts ) } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 4c2d6244ba24bba..44a973da2d6c14f 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -335,6 +335,7 @@ export default class Server { ...parsed, pathname: localePathResult.pathname, }) + ;(req as any).__nextStrippedLocale = true parsedUrl.pathname = localePathResult.pathname // check if the locale prefix matches a domain's defaultLocale diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 332823e52612af0..ea4067285f072f7 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -899,6 +899,10 @@ export async function renderToHTML( let html = renderDocument(Document, { ...renderOpts, + canonicalBase: + !renderOpts.ampPath && (req as any).__nextStrippedLocale + ? `${renderOpts.canonicalBase || ''}/${renderOpts.locale}` + : renderOpts.canonicalBase, docComponentsRendered, buildManifest: filteredBuildManifest, // Only enabled in production as development mode has features relying on HMR (style injection for example) diff --git a/test/integration/i18n-support/pages/amp/amp-first.js b/test/integration/i18n-support/pages/amp/amp-first.js new file mode 100644 index 000000000000000..eeb68575a23d8dc --- /dev/null +++ b/test/integration/i18n-support/pages/amp/amp-first.js @@ -0,0 +1,32 @@ +import { useAmp } from 'next/amp' +import { useRouter } from 'next/router' + +export const config = { + amp: true, +} + +export default function Page(props) { + const router = useRouter() + + return ( + <> +

another page

+

{useAmp() ? 'yes' : 'no'}

+

{JSON.stringify(props)}

+

{router.locale}

+

{JSON.stringify(router.locales)}

+

{JSON.stringify(router.query)}

+

{router.pathname}

+

{router.asPath}

+ + ) +} + +export const getServerSideProps = ({ locale, locales }) => { + return { + props: { + locale, + locales, + }, + } +} diff --git a/test/integration/i18n-support/pages/amp/amp-hybrid.js b/test/integration/i18n-support/pages/amp/amp-hybrid.js new file mode 100644 index 000000000000000..c693898eb538912 --- /dev/null +++ b/test/integration/i18n-support/pages/amp/amp-hybrid.js @@ -0,0 +1,23 @@ +import { useAmp } from 'next/amp' +import { useRouter } from 'next/router' + +export const config = { + amp: 'hybrid', +} + +export default function Page(props) { + const router = useRouter() + + return ( + <> +

another page

+

{useAmp() ? 'yes' : 'no'}

+

{JSON.stringify(props)}

+

{router.locale}

+

{JSON.stringify(router.locales)}

+

{JSON.stringify(router.query)}

+

{router.pathname}

+

{router.asPath}

+ + ) +} diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js index 90f8f36835e2a81..e75637a49d871b9 100644 --- a/test/integration/i18n-support/test/index.test.js +++ b/test/integration/i18n-support/test/index.test.js @@ -391,6 +391,53 @@ function runTests(isDev) { await checkDomainLocales('fr', 'example.fr') }) + it('should generate AMP pages with all locales', async () => { + for (const locale of locales) { + const localePath = locale !== 'en-US' ? `/${locale}` : '' + const html = await renderViaHTTP(appPort, `${localePath}/amp/amp-hybrid`) + const $ = cheerio.load(html) + expect($('html').attr('lang')).toBe(locale) + expect($('#is-amp').text()).toBe('no') + expect($('#router-locale').text()).toBe(locale) + expect(JSON.parse($('#router-locales').text())).toEqual(locales) + expect($('#router-pathname').text()).toBe('/amp/amp-hybrid') + expect($('#router-as-path').text()).toBe('/amp/amp-hybrid') + expect(JSON.parse($('#router-query').text())).toEqual({}) + + const amphtmlPath = `${localePath}/amp/amp-hybrid${ + isDev ? '?amp=1' : '.amp' + }` + expect($('link[rel=amphtml]').attr('href')).toBe(amphtmlPath) + + const html2 = await renderViaHTTP(appPort, amphtmlPath) + const $2 = cheerio.load(html2) + expect($2('html').attr('lang')).toBe(locale) + expect($2('#is-amp').text()).toBe('yes') + expect($2('#router-locale').text()).toBe(locale) + expect(JSON.parse($2('#router-locales').text())).toEqual(locales) + expect($2('#router-pathname').text()).toBe('/amp/amp-hybrid') + expect($2('#router-as-path').text()).toBe('/amp/amp-hybrid') + expect(JSON.parse($2('#router-query').text())).toEqual({ amp: '1' }) + expect($2('link[rel=amphtml]').attr('href')).toBeFalsy() + } + }) + + it('should work with AMP first page with all locales', async () => { + for (const locale of locales) { + const localePath = locale !== 'en-US' ? `/${locale}` : '' + const html = await renderViaHTTP(appPort, `${localePath}/amp/amp-first`) + const $ = cheerio.load(html) + expect($('html').attr('lang')).toBe(locale) + expect($('#is-amp').text()).toBe('yes') + expect($('#router-locale').text()).toBe(locale) + expect(JSON.parse($('#router-locales').text())).toEqual(locales) + expect($('#router-pathname').text()).toBe('/amp/amp-first') + expect($('#router-as-path').text()).toBe('/amp/amp-first') + expect(JSON.parse($('#router-query').text())).toEqual({}) + expect($('link[rel=amphtml]').attr('href')).toBeFalsy() + } + }) + it('should generate fallbacks with all locales', async () => { for (const locale of locales) { const html = await renderViaHTTP( From fc0715d7952299a23b6322406876d89d6d7e1db3 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 15 Oct 2020 14:15:48 -0500 Subject: [PATCH 2/3] Fix type error --- packages/next/export/worker.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index f5bdcc02f7e8cd1..01c5fe2ca42bf67 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -359,7 +359,13 @@ export default async function exportPage({ req.url += (req.url!.includes('?') ? '&' : '?') + 'amp=1' // @ts-ignore ampHtml = ( - await renderMethod(req, res, 'export', curRenderOpts, params) + await (renderMethod as any)( + req, + res, + 'export', + curRenderOpts, + params + ) ).html } else { ampHtml = await renderMethod( @@ -368,7 +374,7 @@ export default async function exportPage({ page, // @ts-ignore { ...query, amp: '1' }, - curRenderOpts + curRenderOpts as any ) } From b1ec0467630a685294cdceed4257c58a399a28ec Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 15 Oct 2020 17:29:12 -0500 Subject: [PATCH 3/3] Update size-limit --- test/integration/size-limit/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 02b215d6ad01b1b..dc5a102f9ed7b40 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -80,7 +80,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 280 * 1024 + const delta = responseSizesBytes - 281 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target })