Skip to content

Commit

Permalink
Ensure i18n support with AMP (#17923)
Browse files Browse the repository at this point in the history
* Ensure i18n support with AMP

* Fix type error

* Update size-limit
  • Loading branch information
ijjk committed Oct 15, 2020
1 parent 2a94ae0 commit 4b126cc
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/next/build/index.ts
Expand Up @@ -798,7 +798,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 },
}

Expand Down
12 changes: 12 additions & 0 deletions packages/next/build/webpack/loaders/next-serverless-loader.ts
Expand Up @@ -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,
Expand Down
27 changes: 20 additions & 7 deletions packages/next/export/worker.ts
Expand Up @@ -71,6 +71,7 @@ interface RenderOpts {
fontManifest?: FontManifest
locales?: string[]
locale?: string
defaultLocale?: string
}

type ComponentModule = ComponentType<{}> & {
Expand Down Expand Up @@ -100,8 +101,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

Expand All @@ -115,6 +117,10 @@ export default async function exportPage({
if (localePathResult.detectedLocale) {
updatedPath = localePathResult.pathname
locale = localePathResult.detectedLocale

if (locale === renderOpts.defaultLocale) {
renderAmpPath = `${normalizePagePath(updatedPath)}.amp`
}
}
}

Expand Down Expand Up @@ -239,7 +245,7 @@ export default async function exportPage({
res,
'export',
{
ampPath,
ampPath: renderAmpPath,
/// @ts-ignore
optimizeFonts,
/// @ts-ignore
Expand Down Expand Up @@ -299,7 +305,7 @@ export default async function exportPage({
curRenderOpts = {
...components,
...renderOpts,
ampPath,
ampPath: renderAmpPath,
params,
optimizeFonts,
optimizeImages,
Expand Down Expand Up @@ -356,16 +362,23 @@ 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 as any)(
req,
res,
'export',
curRenderOpts,
params
)
).html
} else {
ampHtml = await renderMethod(
req,
res,
page,
// @ts-ignore
{ ...query, amp: 1 },
curRenderOpts
{ ...query, amp: '1' },
curRenderOpts as any
)
}

Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -909,6 +909,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)
Expand Down
32 changes: 32 additions & 0 deletions 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 (
<>
<p id="another">another page</p>
<p id="is-amp">{useAmp() ? 'yes' : 'no'}</p>
<p id="props">{JSON.stringify(props)}</p>
<p id="router-locale">{router.locale}</p>
<p id="router-locales">{JSON.stringify(router.locales)}</p>
<p id="router-query">{JSON.stringify(router.query)}</p>
<p id="router-pathname">{router.pathname}</p>
<p id="router-as-path">{router.asPath}</p>
</>
)
}

export const getServerSideProps = ({ locale, locales }) => {
return {
props: {
locale,
locales,
},
}
}
23 changes: 23 additions & 0 deletions 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 (
<>
<p id="another">another page</p>
<p id="is-amp">{useAmp() ? 'yes' : 'no'}</p>
<p id="props">{JSON.stringify(props)}</p>
<p id="router-locale">{router.locale}</p>
<p id="router-locales">{JSON.stringify(router.locales)}</p>
<p id="router-query">{JSON.stringify(router.query)}</p>
<p id="router-pathname">{router.pathname}</p>
<p id="router-as-path">{router.asPath}</p>
</>
)
}
47 changes: 47 additions & 0 deletions test/integration/i18n-support/test/index.test.js
Expand Up @@ -418,6 +418,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(
Expand Down
2 changes: 1 addition & 1 deletion test/integration/size-limit/test/index.test.js
Expand Up @@ -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
})
Expand Down

0 comments on commit 4b126cc

Please sign in to comment.