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

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
11016221 · JJ Kasper · GitHub · 5ed51c00 · 11016221 · 11016221
4 changed file
--- 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 (
    '<!DOCTYPE html>' +
    renderToStaticMarkup(
-      <Document
-        __NEXT_DATA__={{
-          dataManager: dataManagerData,
-          props, // The result of getInitialProps
-          page: pathname, // The rendered page
-          query, // querystring parsed / passed by the user
-          buildId, // buildId is used to facilitate caching of page bundles, we send it to the client so that pageloader knows where to load bundles
-          dynamicBuildId, // Specifies if the buildId should by dynamically fetched
-          assetPrefix: assetPrefix === '' ? undefined : assetPrefix, // send assetPrefix to the client side when configured, otherwise don't sent in the resulting HTML
-          runtimeConfig, // runtimeConfig if provided, otherwise don't sent in the resulting HTML
-          nextExport, // If this is a page exported by `next export`
-          dynamicIds:
-          dynamicImportsIds.length === 0 ? undefined : dynamicImportsIds,
-          err: err ? serializeError(dev, err) : undefined, // Error if one happened, otherwise don't sent in the resulting HTML
-        }}
-        ampPath={ampPath}
-        amphtml={amphtml}
-        hasAmp={hasAmp}
-        staticMarkup={staticMarkup}
-        devFiles={devFiles}
-        files={files}
-        dynamicImports={dynamicImports}
-        assetPrefix={assetPrefix}
-        {...docProps}
-      />,
+      <AmpModeContext.Provider value={ampMode}>
+        <Document
+          __NEXT_DATA__={{
+            dataManager: dataManagerData,
+            props, // The result of getInitialProps
+            page: pathname, // The rendered page
+            query, // querystring parsed / passed by the user
+            buildId, // buildId is used to facilitate caching of page bundles, we send it to the client so that pageloader knows where to load bundles
+            dynamicBuildId, // Specifies if the buildId should by dynamically fetched
+            assetPrefix: assetPrefix === '' ? undefined : assetPrefix, // send assetPrefix to the client side when configured, otherwise don't sent in the resulting HTML
+            runtimeConfig, // runtimeConfig if provided, otherwise don't sent in the resulting HTML
+            nextExport, // If this is a page exported by `next export`
+            dynamicIds:
+            dynamicImportsIds.length === 0 ? undefined : dynamicImportsIds,
+            err: err ? serializeError(dev, err) : undefined, // Error if one happened, otherwise don't sent in the resulting HTML
+          }}
+          ampPath={ampPath}
+          amphtml={amphtml}
+          hasAmp={hasAmp}
+          staticMarkup={staticMarkup}
+          devFiles={devFiles}
+          files={files}
+          dynamicImports={dynamicImports}
+          assetPrefix={assetPrefix}
+          {...docProps}
+        />
+      </AmpModeContext.Provider>,
    )
  )
 }
@@ -420,6 +425,7 @@ export async function renderToHTML(
  let html = renderDocument(Document, {
    ...renderOpts,
    dataManagerData,
+    ampMode,
    props,
    docProps,
    pathname,

--- 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 = '<script'
-        Object.keys(props).forEach(prop => {
-          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 = '<script'
+          Object.keys(props).forEach(prop => {
+            badProp += ` ${prop}="${props[prop]}"`
+          })
+          badProp += '/>'
+        }
      }

      if (badProp) {

--- a/test/integration/amphtml/pages/_document.js
+++ b/test/integration/amphtml/pages/_document.js
+import { useAmp } from 'next/amp'
+import Document, { Html, Head, Main, NextScript } from 'next/document'
+
+const AmpTst = () => {
+  const isAmp = useAmp()
+  return <p>{isAmp ? 'AMP Power!!!' : 'no AMP for you...'}</p>
+}
+
+class MyDocument extends Document {
+  static async getInitialProps (ctx) {
+    const initialProps = await Document.getInitialProps(ctx)
+    return { ...initialProps }
+  }
+
+  render () {
+    return (
+      <Html>
+        <Head />
+        <body>
+          <Main />
+          <AmpTst />
+          <NextScript />
+        </body>
+      </Html>
+    )
+  }
+}
+
+export default MyDocument
--- 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 () => {