Skip to content

Commit

Permalink
fix(gatsby): Make <script> in Head behave correctly (#36212) (#36299)
Browse files Browse the repository at this point in the history
Co-authored-by: pieh <misiek.piechowiak@gmail.com>
Co-authored-by: Jude Agboola <marvinjudehk@gmail.com>
  • Loading branch information
3 people authored Aug 2, 2022
1 parent a6ff9e9 commit 6c0316e
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
Expand All @@ -28,7 +31,7 @@ export function Head() {
style,
link,
extraMeta,
jsonLD
jsonLD,
} = data.static

return (
Expand All @@ -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>
</>
)
}
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand Down
74 changes: 74 additions & 0 deletions packages/gatsby/cache-dir/head/__tests__/utils.ts
Original file line number Diff line number Diff line change
@@ -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>,
],
]
`)
})
})
35 changes: 31 additions & 4 deletions packages/gatsby/cache-dir/head/head-export-handler-for-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
headExportValidator,
filterHeadProps,
warnForInvalidTags,
diffNodes,
} from "./utils"

const hiddenRoot = document.createElement(`div`)
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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`) {
Expand Down
50 changes: 50 additions & 0 deletions packages/gatsby/cache-dir/head/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 6c0316e

Please sign in to comment.