fix(gatsby): Make <script> in Head behave correctly (#36212)

* use innerHTML since since text is unesacaped * make scripts run * eliminate multiple append calls * dev inline script test * fix problematic multiple appending * test that script run and get called once * diff nodes before reappending * add new elements and remove stale ones * remove element in prev loop * move node diffind to separate function, add unit test * slightly more performant diffing * don't init array if we will bail early anyway Co-authored-by: pieh <[email protected]> (cherry picked from commit 6612f69)
gatsbyjs · Aug 2, 2022 · 9c6988d · 9c6988d
1 parent a6ff9e9
commit 9c6988d
Show file tree

Hide file tree

Showing 8 changed files with 207 additions and 7 deletions.
diff --git a/e2e-tests/development-runtime/cypress/integration/head-function-export/scripts.js b/e2e-tests/development-runtime/cypress/integration/head-function-export/scripts.js
@@ -0,0 +1,20 @@
+import { page } from "../../../shared-data/head-function-export.js"
+
+describe("Scripts", () => {
+  beforeEach(() => {
+    cy.visit(page.basic).waitForRouteChange()
+  })
+
+  // This tests that we don't append elements to the document head more than once
+  // A script will get called more than once it that happens
+  it(`Inline script work and get called only once`, () => {
+
+    // Head export seem to be appending the tags after waitForRouteChange()
+    // We need to find a way to make waitForRouteChange() catch Head export too
+    cy.wait(3000)
+
+    cy.window().then(win => {
+      expect(win.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__).to.equal(1)
+    })
+  })
+})
diff --git a/e2e-tests/development-runtime/src/pages/head-function-export/basic.js b/e2e-tests/development-runtime/src/pages/head-function-export/basic.js
@@ -12,7 +12,10 @@ export default function HeadFunctionExportBasic() {
       <Link data-testid="gatsby-link" to="/head-function-export/page-query">
         Navigate to page-query via Gatsby Link
       </Link>
-      <Link data-testid="navigate-to-page-without-head-export" to="/without-head">
+      <Link
+        data-testid="navigate-to-page-without-head-export"
+        to="/without-head"
+      >
         Navigate to without head export
       </Link>
     </>
@@ -28,7 +31,7 @@ export function Head() {
     style,
     link,
     extraMeta,
-    jsonLD
+    jsonLD,
   } = data.static
 
   return (
@@ -54,6 +57,9 @@ export function Head() {
       <script data-testid="jsonLD" type="application/ld+json">
         {jsonLD}
       </script>
+      <script type="text/javascript">
+        {`window.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__ = (window.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__ || 0 ) + 1`}
+      </script>
     </>
   )
 }
diff --git a/e2e-tests/production-runtime/cypress/integration/head-function-export/scripts.js b/e2e-tests/production-runtime/cypress/integration/head-function-export/scripts.js
@@ -0,0 +1,20 @@
+import { page } from "../../../shared-data/head-function-export.js"
+
+describe("Scripts", () => {
+  beforeEach(() => {
+    cy.visit(page.basic).waitForRouteChange()
+  })
+
+  // This tests that we don't append elements to the document head more than once
+  // A script will get called more than once it that happens
+  it(`Inline script work and get called only once`, () => {
+
+    // Head export seem to be appending the tags after waitForRouteChange()
+    // We need to find a way to make waitForRouteChange() catch Head export too
+    cy.wait(3000)
+
+    cy.window().then(win => {
+      expect(win.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__).to.equal(1)
+    })
+  })
+})
diff --git a/e2e-tests/production-runtime/src/pages/head-function-export/basic.js b/e2e-tests/production-runtime/src/pages/head-function-export/basic.js
@@ -38,6 +38,9 @@ export function Head() {
       <script data-testid="jsonLD" type="application/ld+json">
         {jsonLD}
       </script>
+      <script type="text/javascript">
+        {`window.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__ = (window.__SOME_GLOBAL_TO_CHECK_CALL_COUNT__ || 0 ) + 1`}
+      </script>
     </>
   )
 }
diff --git a/integration-tests/head-function-export/__tests__/ssr-html-output.js b/integration-tests/head-function-export/__tests__/ssr-html-output.js
@@ -34,7 +34,7 @@ describe(`Head function export SSR'ed HTML output`, () => {
     expect(noscript.text).toEqual(data.static.noscript)
     expect(style.text).toContain(data.static.style)
     expect(link.attributes.href).toEqual(data.static.link)
-    expect(jsonLD.text).toEqual(data.static.jsonLD)
+    expect(jsonLD.innerHTML).toEqual(data.static.jsonLD)
   })
 
   it(`should work with data from a page query`, () => {

diff --git a/packages/gatsby/cache-dir/head/__tests__/utils.ts b/packages/gatsby/cache-dir/head/__tests__/utils.ts
@@ -0,0 +1,74 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { diffNodes } from "../utils"
+
+function createElement(
+  type: string,
+  attributes: Record<string, string> | undefined = undefined,
+  innerHTML: string | undefined = undefined
+): Element {
+  const element: Element = document.createElement(type)
+  if (attributes) {
+    for (const [key, value] of Object.entries(attributes)) {
+      if (value === `string`) {
+        element.setAttribute(key, value)
+      }
+    }
+  }
+  if (innerHTML) {
+    element.innerHTML = innerHTML
+  }
+  return element
+}
+
+describe(`diffNodes`, () => {
+  it(`should keep same nodes, remove nodes that were not re-created, and add new nodes`, () => {
+    const oldNodes = [
+      createElement(`title`, {}, `to remove`),
+      createElement(`script`, {}, `stable`),
+      createElement(`script`, {}, `to remove`),
+    ]
+
+    const newNodes = [
+      createElement(`title`, {}, `to add`),
+      createElement(`script`, {}, `stable`),
+      createElement(`script`, {}, `to add`),
+    ]
+
+    const onStale = jest.fn()
+    const onNew = jest.fn()
+
+    diffNodes({ oldNodes, newNodes, onStale, onNew })
+
+    expect(onStale.mock.calls).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          <title>
+            to remove
+          </title>,
+        ],
+        Array [
+          <script>
+            to remove
+          </script>,
+        ],
+      ]
+    `)
+    expect(onNew.mock.calls).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          <title>
+            to add
+          </title>,
+        ],
+        Array [
+          <script>
+            to add
+          </script>,
+        ],
+      ]
+    `)
+  })
+})
diff --git a/packages/gatsby/cache-dir/head/head-export-handler-for-browser.js b/packages/gatsby/cache-dir/head/head-export-handler-for-browser.js
@@ -9,6 +9,7 @@ import {
   headExportValidator,
   filterHeadProps,
   warnForInvalidTags,
+  diffNodes,
 } from "./utils"
 
 const hiddenRoot = document.createElement(`div`)
@@ -21,8 +22,6 @@ const removePrevHeadElements = () => {
 const onHeadRendered = () => {
   const validHeadNodes = []
 
-  removePrevHeadElements()
-
   const seenIds = new Map()
   for (const node of hiddenRoot.childNodes) {
     const nodeName = node.nodeName.toLowerCase()
@@ -31,8 +30,19 @@ const onHeadRendered = () => {
     if (!VALID_NODE_NAMES.includes(nodeName)) {
       warnForInvalidTags(nodeName)
     } else {
-      const clonedNode = node.cloneNode(true)
+      let clonedNode = node.cloneNode(true)
       clonedNode.setAttribute(`data-gatsby-head`, true)
+
+      // Create an element for scripts to make script work
+      if (clonedNode.nodeName.toLowerCase() === `script`) {
+        const script = document.createElement(`script`)
+        for (const attr of clonedNode.attributes) {
+          script.setAttribute(attr.name, attr.value)
+        }
+        script.innerHTML = clonedNode.innerHTML
+        clonedNode = script
+      }
+
       if (id) {
         if (!seenIds.has(id)) {
           validHeadNodes.push(clonedNode)
@@ -48,7 +58,24 @@ const onHeadRendered = () => {
     }
   }
 
-  document.head.append(...validHeadNodes)
+  const existingHeadElements = [
+    ...document.querySelectorAll(`[data-gatsby-head]`),
+  ]
+
+  if (existingHeadElements.length === 0) {
+    document.head.append(...validHeadNodes)
+    return
+  }
+
+  const newHeadNodes = []
+  diffNodes({
+    oldNodes: existingHeadElements,
+    newNodes: validHeadNodes,
+    onStale: node => node.remove(),
+    onNew: node => newHeadNodes.push(node),
+  })
+
+  document.head.append(...newHeadNodes)
 }
 
 if (process.env.BUILD_STAGE === `develop`) {

diff --git a/packages/gatsby/cache-dir/head/utils.js b/packages/gatsby/cache-dir/head/utils.js
@@ -52,3 +52,53 @@ export function warnForInvalidTags(tagName) {
     warnOnce(warning)
   }
 }
+
+/**
+ * When a `nonce` is present on an element, browsers such as Chrome and Firefox strip it out of the
+ * actual HTML attributes for security reasons *when the element is added to the document*. Thus,
+ * given two equivalent elements that have nonces, `Element,isEqualNode()` will return false if one
+ * of those elements gets added to the document. Although the `element.nonce` property will be the
+ * same for both elements, the one that was added to the document will return an empty string for
+ * its nonce HTML attribute value.
+ *
+ * This custom `isEqualNode()` function therefore removes the nonce value from the `newTag` before
+ * comparing it to `oldTag`, restoring it afterwards.
+ *
+ * For more information, see:
+ * https://bugs.chromium.org/p/chromium/issues/detail?id=1211471#c12
+ */
+export function isEqualNode(oldTag, newTag) {
+  if (oldTag instanceof HTMLElement && newTag instanceof HTMLElement) {
+    const nonce = newTag.getAttribute(`nonce`)
+    // Only strip the nonce if `oldTag` has had it stripped. An element's nonce attribute will not
+    // be stripped if there is no content security policy response header that includes a nonce.
+    if (nonce && !oldTag.getAttribute(`nonce`)) {
+      const cloneTag = newTag.cloneNode(true)
+      cloneTag.setAttribute(`nonce`, ``)
+      cloneTag.nonce = nonce
+      return nonce === oldTag.nonce && oldTag.isEqualNode(cloneTag)
+    }
+  }
+
+  return oldTag.isEqualNode(newTag)
+}
+
+export function diffNodes({ oldNodes, newNodes, onStale, onNew }) {
+  for (const existingHeadElement of oldNodes) {
+    const indexInNewNodes = newNodes.findIndex(e =>
+      isEqualNode(e, existingHeadElement)
+    )
+
+    if (indexInNewNodes === -1) {
+      onStale(existingHeadElement)
+    } else {
+      // this node is re-created as-is, so we keep old node, and remove it from list of new nodes (as we handled it already here)
+      newNodes.splice(indexInNewNodes, 1)
+    }
+  }
+
+  // remaing new nodes didn't have matching old node, so need to be added
+  for (const newNode of newNodes) {
+    onNew(newNode)
+  }
+}