From 11016221cdbcef94b60d636fcb289b7a36a84515 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 12 Apr 2019 19:04:52 -0500 Subject: [PATCH] Make sure AmpContext is available in _document (#7021) * Make sure AmpContext is available during _document render and update filtering of script tags in AMP mode * Update amphtml test to make sure AmpContext is set for _document render * Fix stray comma in render.tsx --- packages/next-server/server/render.tsx | 56 ++++++++++++--------- packages/next/pages/_document.js | 20 +++++--- test/integration/amphtml/pages/_document.js | 29 +++++++++++ test/integration/amphtml/test/index.test.js | 2 + 4 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 test/integration/amphtml/pages/_document.js diff --git a/packages/next-server/server/render.tsx b/packages/next-server/server/render.tsx index 017aecdda7..93e50838fe 100644 --- a/packages/next-server/server/render.tsx +++ b/packages/next-server/server/render.tsx @@ -126,6 +126,7 @@ type RenderOpts = { ampPath?: string amphtml?: boolean hasAmp?: boolean, + ampMode?: any, dataOnly?: boolean, buildManifest: BuildManifest reactLoadableManifest: ReactLoadableManifest @@ -155,6 +156,7 @@ function renderDocument( ampPath, amphtml, hasAmp, + ampMode, staticMarkup, devFiles, files, @@ -168,6 +170,7 @@ function renderDocument( ampPath: string, amphtml: boolean hasAmp: boolean, + ampMode: any, dynamicImportsIds: string[] dynamicImports: ManifestItem[] files: string[] @@ -177,31 +180,33 @@ function renderDocument( return ( '' + renderToStaticMarkup( - , + + + , ) ) } @@ -420,6 +425,7 @@ export async function renderToHTML( let html = renderDocument(Document, { ...renderOpts, dataManagerData, + ampMode, props, docProps, pathname, diff --git a/packages/next/pages/_document.js b/packages/next/pages/_document.js index 0b51e84995..9205cbf7fe 100644 --- a/packages/next/pages/_document.js +++ b/packages/next/pages/_document.js @@ -186,12 +186,20 @@ export class Head extends Component { badProp = 'name="viewport"' } else if (type === 'link' && props.rel === 'canonical') { badProp = 'rel="canonical"' - } else if (type === 'script' && !(props.src && props.src.indexOf('ampproject') > -1)) { - badProp = ' { - badProp += ` ${prop}="${props[prop]}"` - }) - badProp += '/>' + } else if (type === 'script') { + // only block if + // 1. it has a src and isn't pointing to ampproject's CDN + // 2. it is using dangerouslySetInnerHTML without a type or + // a type of text/javascript + if ((props.src && props.src.indexOf('ampproject') < -1) || + (props.dangerouslySetInnerHTML && (!props.type || props.type === 'text/javascript')) + ) { + badProp = ' { + badProp += ` ${prop}="${props[prop]}"` + }) + badProp += '/>' + } } if (badProp) { diff --git a/test/integration/amphtml/pages/_document.js b/test/integration/amphtml/pages/_document.js new file mode 100644 index 0000000000..be53f8093f --- /dev/null +++ b/test/integration/amphtml/pages/_document.js @@ -0,0 +1,29 @@ +import { useAmp } from 'next/amp' +import Document, { Html, Head, Main, NextScript } from 'next/document' + +const AmpTst = () => { + const isAmp = useAmp() + return

{isAmp ? 'AMP Power!!!' : 'no AMP for you...'}

+} + +class MyDocument extends Document { + static async getInitialProps (ctx) { + const initialProps = await Document.getInitialProps(ctx) + return { ...initialProps } + } + + render () { + return ( + + + +
+ + + + + ) + } +} + +export default MyDocument diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js index 4f246b9b40..d16f9f64d7 100644 --- a/test/integration/amphtml/test/index.test.js +++ b/test/integration/amphtml/test/index.test.js @@ -119,12 +119,14 @@ describe('AMP Usage', () => { it('should render the normal page that uses the AMP hook', async () => { const html = await renderViaHTTP(appPort, '/use-amp-hook') expect(html).toMatch(/Hello others/) + expect(html).toMatch(/no AMP for you\.\.\./) }) it('should render the AMP page that uses the AMP hook', async () => { const html = await renderViaHTTP(appPort, '/use-amp-hook?amp=1') await validateAMP(html) expect(html).toMatch(/Hello AMP/) + expect(html).toMatch(/AMP Power!!!/) }) it('should render nested normal page with AMP hook', async () => { -- GitLab