From 75f33e4207886212d452da5aa083a3d6d9d00b31 Mon Sep 17 00:00:00 2001 From: Yosuke Furukawa Date: Wed, 26 Feb 2020 22:53:23 +0900 Subject: [PATCH 1/3] Add custom amp optimizer and skip validation mode --- packages/next/export/index.ts | 2 + packages/next/export/worker.js | 62 ++++++++++--------- .../next/next-server/server/next-server.ts | 2 + .../next/next-server/server/optimize-amp.ts | 9 ++- packages/next/next-server/server/render.tsx | 6 +- packages/next/server/next-dev-server.ts | 2 + .../amphtml-custom-optimizer/next.config.js | 12 ++++ .../amphtml-custom-optimizer/pages/dynamic.js | 20 ++++++ .../amphtml-custom-optimizer/pages/index.js | 12 ++++ .../test/index.test.js | 52 ++++++++++++++++ 10 files changed, 144 insertions(+), 35 deletions(-) create mode 100644 test/integration/amphtml-custom-optimizer/next.config.js create mode 100644 test/integration/amphtml-custom-optimizer/pages/dynamic.js create mode 100644 test/integration/amphtml-custom-optimizer/pages/index.js create mode 100644 test/integration/amphtml-custom-optimizer/test/index.test.js diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index e43f65e243417f7..6b5054a4435c23e 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -234,6 +234,8 @@ export default async function( canonicalBase: nextConfig.amp?.canonicalBase || '', isModern: nextConfig.experimental.modern, ampValidatorPath: nextConfig.experimental.amp?.validator || undefined, + ampSkipValidation: nextConfig.experimental.amp?.skipValidation || false, + ampOptimizerConfig: nextConfig.experimental.amp?.optimizer || undefined, } const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index 3997d954e3cf3fa..35bfd956ab215ad 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -214,38 +214,40 @@ export default async function({ } } - if (curRenderOpts.inAmpMode) { - await validateAmp(html, path, curRenderOpts.ampValidatorPath) - } else if (curRenderOpts.hybridAmp) { - // we need to render the AMP version - let ampHtmlFilename = `${ampPath}${sep}index.html` - if (!subFolders) { - ampHtmlFilename = `${ampPath}.html` - } - const ampBaseDir = join(outDir, dirname(ampHtmlFilename)) - const ampHtmlFilepath = join(outDir, ampHtmlFilename) - - try { - await accessP(ampHtmlFilepath) - } catch (_) { - // make sure it doesn't exist from manual mapping - let ampHtml - if (serverless) { - req.url += (req.url.includes('?') ? '&' : '?') + 'amp=1' - ampHtml = (await renderMethod(req, res, true)).html - } else { - ampHtml = await renderMethod( - req, - res, - page, - { ...query, amp: 1 }, - curRenderOpts - ) + if (!curRenderOpts.ampSkipValidation) { + if (curRenderOpts.inAmpMode) { + await validateAmp(html, path, curRenderOpts.ampValidatorPath) + } else if (curRenderOpts.hybridAmp) { + // we need to render the AMP version + let ampHtmlFilename = `${ampPath}${sep}index.html` + if (!subFolders) { + ampHtmlFilename = `${ampPath}.html` } + const ampBaseDir = join(outDir, dirname(ampHtmlFilename)) + const ampHtmlFilepath = join(outDir, ampHtmlFilename) - await validateAmp(ampHtml, page + '?amp=1') - await mkdirp(ampBaseDir) - await writeFileP(ampHtmlFilepath, ampHtml, 'utf8') + try { + await accessP(ampHtmlFilepath) + } catch (_) { + // make sure it doesn't exist from manual mapping + let ampHtml + if (serverless) { + req.url += (req.url.includes('?') ? '&' : '?') + 'amp=1' + ampHtml = (await renderMethod(req, res, true)).html + } else { + ampHtml = await renderMethod( + req, + res, + page, + { ...query, amp: 1 }, + curRenderOpts + ) + } + + await validateAmp(ampHtml, page + '?amp=1') + await mkdirp(ampBaseDir) + await writeFileP(ampHtmlFilepath, ampHtml, 'utf8') + } } } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index c4166c6b72538ba..bba5aba8efeffec 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -114,6 +114,7 @@ export default class Server { documentMiddlewareEnabled: boolean hasCssMode: boolean dev?: boolean + ampOptimizerConfig?: { [key: string]: any } } private compression?: Middleware private onErrorMiddleware?: ({ err }: { err: Error }) => Promise @@ -161,6 +162,7 @@ export default class Server { staticMarkup, buildId: this.buildId, generateEtags, + ampOptimizerConfig: this.nextConfig.experimental.amp?.optimizer, } // Only the `publicRuntimeConfig` key is exposed to the client side diff --git a/packages/next/next-server/server/optimize-amp.ts b/packages/next/next-server/server/optimize-amp.ts index c8c7e291c368282..6e7d3345c9fce69 100644 --- a/packages/next/next-server/server/optimize-amp.ts +++ b/packages/next/next-server/server/optimize-amp.ts @@ -1,10 +1,13 @@ -export default async function optimize(html: string): Promise { +export default async function optimize( + html: string, + config: any +): Promise { let AmpOptimizer try { AmpOptimizer = require('@ampproject/toolbox-optimizer') } catch (_) { return html } - const optimizer = AmpOptimizer.create() - return optimizer.transformHtml(html) + const optimizer = AmpOptimizer.create(config) + return optimizer.transformHtml(html, config) } diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index f0e7a8b7c56883a..694e73db0464af4 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -142,6 +142,8 @@ type RenderOpts = LoadComponentsReturnType & { hybridAmp?: boolean ErrorDebug?: React.ComponentType<{ error: Error }> ampValidator?: (html: string, pathname: string) => Promise + ampSkipValidation: boolean + ampOptimizerConfig?: { [key: string]: any } documentMiddlewareEnabled?: boolean isDataReq?: boolean params?: ParsedUrlQuery @@ -673,9 +675,9 @@ export async function renderToHTML( html.substring(0, ampRenderIndex) + `${docProps.html}` + html.substring(ampRenderIndex + AMP_RENDER_TARGET.length) - html = await optimizeAmp(html) + html = await optimizeAmp(html, renderOpts.ampOptimizerConfig) - if (renderOpts.ampValidator) { + if (!renderOpts.ampSkipValidation && renderOpts.ampValidator) { await renderOpts.ampValidator(html, pathname) } } diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 22d8f76273feda5..65b5f71cc55e0a9 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -53,6 +53,8 @@ export default class DevServer extends Server { this.devReady = new Promise(resolve => { this.setDevReady = resolve }) + ;(this.renderOpts as any).ampSkipValidation = + this.nextConfig.experimental?.amp?.skipValidation ?? false ;(this.renderOpts as any).ampValidator = ( html: string, pathname: string diff --git a/test/integration/amphtml-custom-optimizer/next.config.js b/test/integration/amphtml-custom-optimizer/next.config.js new file mode 100644 index 000000000000000..44c063c99fe03b6 --- /dev/null +++ b/test/integration/amphtml-custom-optimizer/next.config.js @@ -0,0 +1,12 @@ +module.exports = { + experimental: { + amp: { + optimizer: { + ampRuntimeVersion: '001515617716922', + ampUrlPrefix: '/amp', + verbose: true, + }, + skipValidation: true, + }, + }, +} diff --git a/test/integration/amphtml-custom-optimizer/pages/dynamic.js b/test/integration/amphtml-custom-optimizer/pages/dynamic.js new file mode 100644 index 000000000000000..ed59c6f760b9411 --- /dev/null +++ b/test/integration/amphtml-custom-optimizer/pages/dynamic.js @@ -0,0 +1,20 @@ +export const config = { + amp: true, +} + +const Dynamic = props => ( + +) + +Dynamic.getInitialProps = () => { + return { + src: 'https://amp.dev/static/samples/img/story_dog2_portrait.jpg', + } +} + +export default Dynamic diff --git a/test/integration/amphtml-custom-optimizer/pages/index.js b/test/integration/amphtml-custom-optimizer/pages/index.js new file mode 100644 index 000000000000000..cca9d141c938108 --- /dev/null +++ b/test/integration/amphtml-custom-optimizer/pages/index.js @@ -0,0 +1,12 @@ +export const config = { + amp: true, +} + +export default () => ( + +) diff --git a/test/integration/amphtml-custom-optimizer/test/index.test.js b/test/integration/amphtml-custom-optimizer/test/index.test.js new file mode 100644 index 000000000000000..dd7992ae1d34e60 --- /dev/null +++ b/test/integration/amphtml-custom-optimizer/test/index.test.js @@ -0,0 +1,52 @@ +/* eslint-env jest */ +/* global jasmine */ +import { join } from 'path' +import { + nextBuild, + findPort, + nextStart, + killApp, + renderViaHTTP, +} from 'next-test-utils' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1 + +let app +let appPort +const appDir = join(__dirname, '../') + +describe('AMP Custom Optimizer', () => { + it('should build and start for static page', async () => { + const { code } = await nextBuild(appDir, undefined) + expect(code).toBe(0) + + appPort = await findPort() + app = await nextStart(appDir, appPort) + + const html = await renderViaHTTP(appPort, '/') + await killApp(app) + + expect(html).toContain( + 'amp-twitter width="500" height="500" layout="responsive" data-tweetid="1159145442896166912"' + ) + expect(html).toContain('i-amphtml-version="001515617716922"') + expect(html).toContain('script async src="/amp/rtv/001515617716922/v0.js"') + }) + + it('should build and start for dynamic page', async () => { + const { code } = await nextBuild(appDir, undefined) + expect(code).toBe(0) + + appPort = await findPort() + app = await nextStart(appDir, appPort) + + const html = await renderViaHTTP(appPort, '/dynamic') + await killApp(app) + + expect(html).toContain( + 'amp-img width="500" height="500" layout="responsive" src="https://amp.dev/static/samples/img/story_dog2_portrait.jpg"' + ) + expect(html).toContain('i-amphtml-version="001515617716922"') + expect(html).toContain('script async src="/amp/rtv/001515617716922/v0.js"') + }) +}) From e35acba14f880f4543c1b32ce686b994022d2714 Mon Sep 17 00:00:00 2001 From: Yosuke Furukawa Date: Tue, 3 Mar 2020 13:49:05 +0900 Subject: [PATCH 2/3] fix: type --- packages/next/next-server/server/render.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 8f010cc50d81635..c6cc2f6ba468abc 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -141,7 +141,7 @@ export type RenderOptsPartial = { hybridAmp?: boolean ErrorDebug?: React.ComponentType<{ error: Error }> ampValidator?: (html: string, pathname: string) => Promise - ampSkipValidation: boolean + ampSkipValidation?: boolean ampOptimizerConfig?: { [key: string]: any } documentMiddlewareEnabled?: boolean isDataReq?: boolean From 99b29ac74656ffc03a02d7cd55defe62967e48c5 Mon Sep 17 00:00:00 2001 From: Yosuke Furukawa Date: Tue, 10 Mar 2020 16:47:49 +0900 Subject: [PATCH 3/3] fix worker lint errors --- packages/next/export/worker.js | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index 577fb0fe9689326..6cf1815cf8891ba 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -243,33 +243,12 @@ export default async function({ curRenderOpts ) } - const ampBaseDir = join(outDir, dirname(ampHtmlFilename)) - const ampHtmlFilepath = join(outDir, ampHtmlFilename) - try { - await accessP(ampHtmlFilepath) - } catch (_) { - // make sure it doesn't exist from manual mapping - let ampHtml - if (serverless) { - req.url += (req.url.includes('?') ? '&' : '?') + 'amp=1' - ampHtml = (await renderMethod(req, res, true)).html - } else { - ampHtml = await renderMethod( - req, - res, - page, - { ...query, amp: 1 }, - curRenderOpts - ) - } - - if (!curRenderOpts.ampSkipValidation) { - await validateAmp(ampHtml, page + '?amp=1') - } - await mkdirp(ampBaseDir) - await writeFileP(ampHtmlFilepath, ampHtml, 'utf8') + if (!curRenderOpts.ampSkipValidation) { + await validateAmp(ampHtml, page + '?amp=1') } + await mkdirp(ampBaseDir) + await writeFileP(ampHtmlFilepath, ampHtml, 'utf8') } }