Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure i18n support with AMP #17923

Merged
merged 5 commits into from Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next/build/index.ts
Expand Up @@ -795,7 +795,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 @@ -70,6 +70,7 @@ interface RenderOpts {
fontManifest?: FontManifest
locales?: string[]
locale?: string
defaultLocale?: string
}

type ComponentModule = ComponentType<{}> & {
Expand Down Expand Up @@ -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

Expand All @@ -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`
}
}
}

Expand Down Expand Up @@ -238,7 +244,7 @@ export default async function exportPage({
res,
'export',
{
ampPath,
ampPath: renderAmpPath,
/// @ts-ignore
optimizeFonts,
/// @ts-ignore
Expand Down Expand Up @@ -298,7 +304,7 @@ export default async function exportPage({
curRenderOpts = {
...components,
...renderOpts,
ampPath,
ampPath: renderAmpPath,
params,
optimizeFonts,
optimizeImages,
Expand Down Expand Up @@ -352,16 +358,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 @@ -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)
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 @@ -416,6 +416,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