Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Commit

Permalink
fix(helmet): fixes race condition with react-helmet (#534)
Browse files Browse the repository at this point in the history
Co-authored-by: Jonny Adshead <JAdshead@users.noreply.github.com>
  • Loading branch information
e-l-i-s-e and JAdshead committed Aug 18, 2021
1 parent 5c31ee2 commit 36f65d9
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 19 deletions.
22 changes: 7 additions & 15 deletions __tests__/client/__snapshots__/prerender.spec.js.snap
Original file line number Diff line number Diff line change
@@ -1,29 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`moveHelmetScripts should only move scripts that were injected by react-helmet, ignoring others: helmet scripts 1`] = `
exports[`moveHelmetScripts should only move scripts that were injected by react-helmet, ignoring others: non-helmet scripts 1`] = `
NodeList [
<script
data-react-helmet="true"
src="hello.js"
/>,
<script
data-react-helmet="true"
src="world.js"
src="foo.js"
/>,
<script
data-react-helmet="true"
src="baz.js"
src="bar.js"
/>,
]
`;

exports[`moveHelmetScripts should only move scripts that were injected by react-helmet, ignoring others: non-helmet scripts 1`] = `
NodeList [
<script
src="foo.js"
/>,
exports[`moveHelmetScripts should remove helmet scripts from the body and only append helmet scripts to the head if react-helmet has not already injected them: head helmet scripts 1`] = `
Array [
<script
src="bar.js"
data-react-helmet="true"
src="hello.js"
/>,
]
`;
16 changes: 15 additions & 1 deletion __tests__/client/prerender.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,21 @@ describe('moveHelmetScripts', () => {
document.addEventListener.mock.calls[0][1]();

expect(document.querySelectorAll('body script')).toMatchSnapshot('non-helmet scripts');
expect(document.querySelectorAll('head script')).toMatchSnapshot('helmet scripts');
});

it('should remove helmet scripts from the body and only append helmet scripts to the head if react-helmet has not already injected them', () => {
const scriptForBody = createScript({ src: 'hello.js', helmet: true });
const scriptForHead = createScript({ src: 'hello.js', helmet: true });

document.body.appendChild(scriptForBody);
document.head.appendChild(scriptForHead);

moveHelmetScripts();
document.addEventListener.mock.calls[0][1]();
const scriptsFromHead = [...document.head.querySelectorAll('script')];
expect([...document.body.querySelectorAll('script')].length).toEqual(0);
expect(scriptsFromHead).toMatchSnapshot('head helmet scripts');
expect(scriptsFromHead.length).toEqual(1);
});

it('should not expect NodeList to have forEach due to IE', () => {
Expand Down
13 changes: 10 additions & 3 deletions src/client/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,16 @@ export function loadPrerenderScripts(initialState) {

export function moveHelmetScripts() {
document.addEventListener('DOMContentLoaded', () => {
const helmetScripts = [...document.querySelectorAll('script[data-react-helmet]')];
helmetScripts.forEach((script) => document.body.removeChild(script));
helmetScripts.forEach((script) => document.head.appendChild(script));
const headHelmetScripts = [...document.head.querySelectorAll('script[data-react-helmet]')];
const bodyHelmetScripts = [...document.body.querySelectorAll('script[data-react-helmet]')];

bodyHelmetScripts.forEach((script) => document.body.removeChild(script));

// If the helmet scripts exist in the head then we can assume that the body
// scripts were already added by react-helmet
if (headHelmetScripts.length === 0) {
bodyHelmetScripts.forEach((script) => document.head.appendChild(script));
}
});
}

Expand Down

0 comments on commit 36f65d9

Please sign in to comment.