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

Add custom amp optimizer and skip validation mode #10705

Merged
Show file tree
Hide file tree
Changes from all 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: 2 additions & 0 deletions packages/next/export/index.ts
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions packages/next/export/worker.js
Expand Up @@ -223,7 +223,7 @@ export default async function({
}
}

if (curRenderOpts.inAmpMode) {
if (curRenderOpts.inAmpMode && !curRenderOpts.ampSkipValidation) {
await validateAmp(html, path, curRenderOpts.ampValidatorPath)
} else if (curRenderOpts.hybridAmp) {
// we need to render the AMP version
Expand Down Expand Up @@ -252,7 +252,9 @@ export default async function({
)
}

await validateAmp(ampHtml, page + '?amp=1')
if (!curRenderOpts.ampSkipValidation) {
await validateAmp(ampHtml, page + '?amp=1')
}
await mkdir(ampBaseDir, { recursive: true })
await writeFileP(ampHtmlFilepath, ampHtml, 'utf8')
}
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -119,6 +119,7 @@ export default class Server {
dev?: boolean
previewProps: __ApiPreviewProps
customServer?: boolean
ampOptimizerConfig?: { [key: string]: any }
}
private compression?: Middleware
private onErrorMiddleware?: ({ err }: { err: Error }) => Promise<void>
Expand Down Expand Up @@ -172,6 +173,7 @@ export default class Server {
generateEtags,
previewProps: this.getPreviewProps(),
customServer: customServer === true ? true : undefined,
ampOptimizerConfig: this.nextConfig.experimental.amp?.optimizer,
}

// Only the `publicRuntimeConfig` key is exposed to the client side
Expand Down
9 changes: 6 additions & 3 deletions packages/next/next-server/server/optimize-amp.ts
@@ -1,10 +1,13 @@
export default async function optimize(html: string): Promise<string> {
export default async function optimize(
html: string,
config: any
): Promise<string> {
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)
}
6 changes: 4 additions & 2 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -146,6 +146,8 @@ export type RenderOptsPartial = {
hybridAmp?: boolean
ErrorDebug?: React.ComponentType<{ error: Error }>
ampValidator?: (html: string, pathname: string) => Promise<void>
ampSkipValidation?: boolean
ampOptimizerConfig?: { [key: string]: any }
documentMiddlewareEnabled?: boolean
isDataReq?: boolean
params?: ParsedUrlQuery
Expand Down Expand Up @@ -743,9 +745,9 @@ export async function renderToHTML(
html.substring(0, ampRenderIndex) +
`<!-- __NEXT_DATA__ -->${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)
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/next/server/next-dev-server.ts
Expand Up @@ -54,6 +54,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
Expand Down
12 changes: 12 additions & 0 deletions 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,
},
},
}
20 changes: 20 additions & 0 deletions test/integration/amphtml-custom-optimizer/pages/dynamic.js
@@ -0,0 +1,20 @@
export const config = {
amp: true,
}

const Dynamic = props => (
<amp-img
width="500"
height="500"
layout="responsive"
src={props.src}
></amp-img>
)

Dynamic.getInitialProps = () => {
return {
src: 'https://amp.dev/static/samples/img/story_dog2_portrait.jpg',
}
}

export default Dynamic
12 changes: 12 additions & 0 deletions test/integration/amphtml-custom-optimizer/pages/index.js
@@ -0,0 +1,12 @@
export const config = {
amp: true,
}

export default () => (
<amp-twitter
width="500"
height="500"
layout="responsive"
data-tweetid="1159145442896166912"
></amp-twitter>
)
52 changes: 52 additions & 0 deletions 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"')
})
})