Skip to content

Commit

Permalink
Ensure hybrid AMP works correctly with SSG (#11205)
Browse files Browse the repository at this point in the history
* Ensure hybrid AMP works correctly with SSG

* Strip AMP from query when not needed
  • Loading branch information
ijjk committed Mar 20, 2020
1 parent a63ac3e commit c14d983
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 3 deletions.
Expand Up @@ -327,7 +327,7 @@ const nextServerlessLoader: loader.Loader = function() {
const previewData = tryGetPreviewData(req, res, options.previewProps)
const isPreviewMode = previewData !== false
let result = await renderToHTML(req, res, "${page}", Object.assign({}, getStaticProps ? {} : parsedUrl.query, nowParams ? nowParams : params, _params, isFallback ? { __nextFallback: 'true' } : {}), renderOpts)
let result = await renderToHTML(req, res, "${page}", Object.assign({}, getStaticProps ? { ...(parsedUrl.query.amp ? { amp: '1' } : {}) } : parsedUrl.query, nowParams ? nowParams : params, _params, isFallback ? { __nextFallback: 'true' } : {}), renderOpts)
if (!renderMode) {
if (_nextData || getStaticProps || getServerSideProps) {
Expand Down
14 changes: 12 additions & 2 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -818,7 +818,7 @@ export default class Server {
components,
query: {
...(components.getStaticProps
? { _nextDataReq: query._nextDataReq }
? { _nextDataReq: query._nextDataReq, amp: query.amp }
: query),
...(params || {}),
},
Expand Down Expand Up @@ -896,6 +896,14 @@ export default class Server {
const isServerProps = !!components.getServerSideProps
const hasStaticPaths = !!components.getStaticPaths

if (isSSG && query.amp) {
pathname += `.amp`
}

if (!query.amp) {
delete query.amp
}

// Toggle whether or not this is a Data request
const isDataReq = !!query._nextDataReq
delete query._nextDataReq
Expand Down Expand Up @@ -984,7 +992,9 @@ export default class Server {
}

// Compute the SPR cache key
const urlPathname = parseUrl(req.url || '').pathname!
const urlPathname = `${parseUrl(req.url || '').pathname!}${
query.amp ? '.amp' : ''
}`
const ssgCacheKey = isPreviewMode
? `__` + nanoid() // Preview mode uses a throw away key to not coalesce preview invokes
: urlPathname
Expand Down
22 changes: 22 additions & 0 deletions test/integration/amphtml-ssg/pages/amp.js
@@ -0,0 +1,22 @@
import { useAmp } from 'next/amp'

export const config = {
amp: true,
}

export const getStaticProps = () => {
return {
props: {
hello: 'hello',
random: Math.random(),
},
}
}

export default ({ hello, random }) => (
<>
<p id="use-amp">useAmp: {useAmp() ? 'yes' : 'no'}</p>
<p id="hello">{hello}</p>
<p id="random">{random}</p>
</>
)
22 changes: 22 additions & 0 deletions test/integration/amphtml-ssg/pages/hybrid.js
@@ -0,0 +1,22 @@
import { useAmp } from 'next/amp'

export const config = {
amp: 'hybrid',
}

export const getStaticProps = () => {
return {
props: {
hello: 'hello',
random: Math.random(),
},
}
}

export default ({ hello, random }) => (
<>
<p id="use-amp">useAmp: {useAmp() ? 'yes' : 'no'}</p>
<p id="hello">{hello}</p>
<p id="random">{random}</p>
</>
)
1 change: 1 addition & 0 deletions test/integration/amphtml-ssg/pages/index.js
@@ -0,0 +1 @@
export default () => <p>normal old page</p>
124 changes: 124 additions & 0 deletions test/integration/amphtml-ssg/test/index.test.js
@@ -0,0 +1,124 @@
/* eslint-env jest */
/* global jasmine */
import fs from 'fs-extra'
import { join } from 'path'
import cheerio from 'cheerio'
import { validateAMP } from 'amp-test-utils'
import {
nextBuild,
renderViaHTTP,
findPort,
launchApp,
killApp,
nextStart,
} from 'next-test-utils'

const appDir = join(__dirname, '../')
const nextConfig = join(appDir, 'next.config.js')
let builtServerPagesDir
let appPort
let app

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2

const runTests = (isDev = false) => {
it('should load an amp first page correctly', async () => {
const html = await renderViaHTTP(appPort, '/amp')

if (!isDev) {
await validateAMP(html)
}
const $ = cheerio.load(html)
expect($('#use-amp').text()).toContain('yes')
})

it('should load a hybrid amp page without query correctly', async () => {
const html = await renderViaHTTP(appPort, '/hybrid')
const $ = cheerio.load(html)
expect($('#use-amp').text()).toContain('no')
expect($('#hello').text()).toContain('hello')
})

it('should load a hybrid amp page with query correctly', async () => {
const html = await renderViaHTTP(appPort, '/hybrid?amp=1')

if (!isDev) {
await validateAMP(html)
}
const $ = cheerio.load(html)
expect($('#use-amp').text()).toContain('yes')
expect($('#hello').text()).toContain('hello')
})

if (!isDev) {
const fsExists = file =>
fs
.access(file)
.then(() => true)
.catch(() => false)

const builtPage = file => join(builtServerPagesDir, file)

it('should output prerendered files correctly during build', async () => {
expect(await fsExists(builtPage('amp.js'))).toBe(true)
expect(await fsExists(builtPage('amp.html'))).toBe(true)
expect(await fsExists(builtPage('amp.json'))).toBe(true)

expect(await fsExists(builtPage('hybrid.js'))).toBe(true)
expect(await fsExists(builtPage('hybrid.html'))).toBe(true)
expect(await fsExists(builtPage('hybrid.json'))).toBe(true)

expect(await fsExists(builtPage('hybrid.amp.js'))).toBe(false)
expect(await fsExists(builtPage('hybrid.amp.html'))).toBe(true)
expect(await fsExists(builtPage('hybrid.amp.json'))).toBe(true)
})
}
}

describe('AMP SSG Support', () => {
describe('serverless mode', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`
module.exports = {
target: 'experimental-serverless-trace'
}
`
)
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
builtServerPagesDir = join(appDir, '.next/serverless/pages')
})
afterAll(async () => {
await fs.remove(nextConfig)
await killApp(app)
})
runTests()
})
describe('server mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
const buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
builtServerPagesDir = join(
appDir,
'.next/server/static',
buildId,
'pages'
)
})
afterAll(() => killApp(app))
runTests()
})
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))
runTests(true)
})
})

0 comments on commit c14d983

Please sign in to comment.