From 34c6fdceb51fdc2498d66d2e660a8c5d1690ac1f Mon Sep 17 00:00:00 2001 From: Daniel Emod Kovacs Date: Wed, 13 Mar 2019 21:03:59 +0100 Subject: [PATCH 1/2] Closes #12553 head components check for empty values --- .../__snapshots__/static-entry.js.snap | 8 ++ .../cache-dir/__tests__/static-entry.js | 78 +++++++++++++++++++ packages/gatsby/cache-dir/static-entry.js | 14 +++- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap index 603e8dfc70d86..f5431bda10976 100644 --- a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap +++ b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap @@ -11,3 +11,11 @@ exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"
div3
div2
div1
"`; exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"
div3
div2
div1
"`; + +exports[`static-entry onPreRenderHTML can filter out empty array 1`] = `"
"`; + +exports[`static-entry onPreRenderHTML can filter out empty arrays 1`] = `"
"`; + +exports[`static-entry onPreRenderHTML can filter out null value 1`] = `"
"`; + +exports[`static-entry onPreRenderHTML can filter out null values 1`] = `"
"`; diff --git a/packages/gatsby/cache-dir/__tests__/static-entry.js b/packages/gatsby/cache-dir/__tests__/static-entry.js index 03d857d15a1de..40dd4f3513280 100644 --- a/packages/gatsby/cache-dir/__tests__/static-entry.js +++ b/packages/gatsby/cache-dir/__tests__/static-entry.js @@ -61,6 +61,48 @@ const reverseHeadersPlugin = { }, } +const setNullHeaderPlugin = { + plugin: { + onPreRenderHTML: ({ replaceHeadComponents }) => { + replaceHeadComponents(null) + }, + }, +} + +const setNullHeadersPlugin = { + plugin: { + onPreRenderHTML: ({ replaceHeadComponents }) => { + replaceHeadComponents([null, null]) + }, + }, +} + +const setEmptyArrayHeaderPlugin = { + plugin: { + onPreRenderHTML: ({ replaceHeadComponents }) => { + replaceHeadComponents([]) + }, + }, +} + +const setEmptyArrayHeadersPlugin = { + plugin: { + onPreRenderHTML: ({ replaceHeadComponents }) => { + replaceHeadComponents([[], []]) + }, + }, +} + +const checkNonEmptyHeadersPlugin = { + plugin: { + onPreRenderHTML: ({ getHeadComponents }) => { + const headComponents = getHeadComponents() + expect(headComponents.includes(null)).toBeFalsy() + expect(headComponents.find(val => Array.isArray(val) && val.length === 0)) + }, + }, +} + const fakeStylesPlugin = { plugin: { onRenderBody: ({ setHeadComponents }) => @@ -147,6 +189,42 @@ describe(`static-entry`, () => { }) }) + test(`onPreRenderHTML can filter out null value`, done => { + global.plugins = [setNullHeaderPlugin, checkNonEmptyHeadersPlugin] + + StaticEntry(`/about/`, (_, html) => { + expect(html).toMatchSnapshot() + done() + }) + }) + + test(`onPreRenderHTML can filter out null values`, done => { + global.plugins = [setNullHeadersPlugin, checkNonEmptyHeadersPlugin] + + StaticEntry(`/about/`, (_, html) => { + expect(html).toMatchSnapshot() + done() + }) + }) + + test(`onPreRenderHTML can filter out empty array`, done => { + global.plugins = [setEmptyArrayHeaderPlugin, checkNonEmptyHeadersPlugin] + + StaticEntry(`/about/`, (_, html) => { + expect(html).toMatchSnapshot() + done() + }) + }) + + test(`onPreRenderHTML can filter out empty arrays`, done => { + global.plugins = [setEmptyArrayHeadersPlugin, checkNonEmptyHeadersPlugin] + + StaticEntry(`/about/`, (_, html) => { + expect(html).toMatchSnapshot() + done() + }) + }) + test(`onPreRenderHTML can be used to replace postBodyComponents`, done => { global.plugins = [ fakeComponentsPluginFactory(`Post`), diff --git a/packages/gatsby/cache-dir/static-entry.js b/packages/gatsby/cache-dir/static-entry.js index 3e0cb708ba184..f339b6037e494 100644 --- a/packages/gatsby/cache-dir/static-entry.js +++ b/packages/gatsby/cache-dir/static-entry.js @@ -49,6 +49,16 @@ const getPage = path => pagesObjectMap.get(path) const createElement = React.createElement +const sanitizeHeadComponents = components => { + if (Array.isArray(components)) { + // remove falsy items + return components.filter(val => (Array.isArray(val) ? val.length > 0 : val)) + } else { + // we also accept single components, so we need to handle this case as well + return components ? [components] : [] + } +} + export default (pagePath, callback) => { let bodyHtml = `` let headComponents = [ @@ -69,7 +79,7 @@ export default (pagePath, callback) => { } const setHeadComponents = components => { - headComponents = headComponents.concat(components) + headComponents = headComponents.concat(sanitizeHeadComponents(components)) } const setHtmlAttributes = attributes => { @@ -95,7 +105,7 @@ export default (pagePath, callback) => { const getHeadComponents = () => headComponents const replaceHeadComponents = components => { - headComponents = components + headComponents = sanitizeHeadComponents(components) } const getPreBodyComponents = () => preBodyComponents From fe7fc842ffd2a71ab575d05f8b52b9743f08180c Mon Sep 17 00:00:00 2001 From: Daniel Emod Kovacs Date: Thu, 14 Mar 2019 14:52:08 +0100 Subject: [PATCH 2/2] Added sanity checks to pre and post body component setters --- .../__snapshots__/static-entry.js.snap | 20 +-- .../cache-dir/__tests__/static-entry.js | 142 ++++++++++-------- packages/gatsby/cache-dir/static-entry.js | 16 +- 3 files changed, 93 insertions(+), 85 deletions(-) diff --git a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap index f5431bda10976..2137dd0c59a58 100644 --- a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap +++ b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap @@ -1,21 +1,13 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`develop-static-entry onPreRenderHTML can be used to replace headComponents 1`] = `"
"`; +exports[`develop-static-entry onPreRenderHTML can be used to replace headComponents 1`] = `"
"`; -exports[`develop-static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"
div3
div2
div1
"`; +exports[`develop-static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"
div3
div2
div1
"`; -exports[`develop-static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"
div3
div2
div1
"`; +exports[`develop-static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"
div3
div2
div1
"`; -exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] = `"
"`; +exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] = `"
"`; -exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"
div3
div2
div1
"`; +exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"
div3
div2
div1
"`; -exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"
div3
div2
div1
"`; - -exports[`static-entry onPreRenderHTML can filter out empty array 1`] = `"
"`; - -exports[`static-entry onPreRenderHTML can filter out empty arrays 1`] = `"
"`; - -exports[`static-entry onPreRenderHTML can filter out null value 1`] = `"
"`; - -exports[`static-entry onPreRenderHTML can filter out null values 1`] = `"
"`; +exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"
div3
div2
div1
"`; diff --git a/packages/gatsby/cache-dir/__tests__/static-entry.js b/packages/gatsby/cache-dir/__tests__/static-entry.js index 40dd4f3513280..41edef94ab482 100644 --- a/packages/gatsby/cache-dir/__tests__/static-entry.js +++ b/packages/gatsby/cache-dir/__tests__/static-entry.js @@ -17,7 +17,9 @@ jest.mock( }, } }, - { virtual: true } + { + virtual: true, + } ) jest.mock( @@ -38,7 +40,9 @@ jest.mock( ], } }, - { virtual: true } + { + virtual: true, + } ) const MOCK_FILE_INFO = { @@ -61,44 +65,37 @@ const reverseHeadersPlugin = { }, } -const setNullHeaderPlugin = { - plugin: { - onPreRenderHTML: ({ replaceHeadComponents }) => { - replaceHeadComponents(null) - }, - }, -} - -const setNullHeadersPlugin = { - plugin: { - onPreRenderHTML: ({ replaceHeadComponents }) => { - replaceHeadComponents([null, null]) - }, - }, -} - -const setEmptyArrayHeaderPlugin = { - plugin: { - onPreRenderHTML: ({ replaceHeadComponents }) => { - replaceHeadComponents([]) +const injectValuePlugin = (hookName, methodName, value) => { + return { + plugin: { + [hookName]: staticEntry => { + const method = staticEntry[methodName] + method(value) + }, }, - }, + } } -const setEmptyArrayHeadersPlugin = { - plugin: { - onPreRenderHTML: ({ replaceHeadComponents }) => { - replaceHeadComponents([[], []]) - }, - }, +const checkSanitized = components => { + expect(components.includes(null)).toBeFalsy() + expect( + components.find(val => Array.isArray(val) && val.length === 0) + ).toBeFalsy() } const checkNonEmptyHeadersPlugin = { plugin: { - onPreRenderHTML: ({ getHeadComponents }) => { + onPreRenderHTML: ({ + getHeadComponents, + getPreBodyComponents, + getPostBodyComponents, + }) => { const headComponents = getHeadComponents() - expect(headComponents.includes(null)).toBeFalsy() - expect(headComponents.find(val => Array.isArray(val) && val.length === 0)) + const preBodyComponents = getPreBodyComponents() + const postBodyComponents = getPostBodyComponents() + checkSanitized(headComponents) + checkSanitized(preBodyComponents) + checkSanitized(postBodyComponents) }, }, } @@ -107,9 +104,9 @@ const fakeStylesPlugin = { plugin: { onRenderBody: ({ setHeadComponents }) => setHeadComponents([ - , - , - , + , + , + , ]), }, } @@ -131,9 +128,9 @@ const fakeComponentsPluginFactory = type => { plugin: { onRenderBody: props => { props[`set${type}BodyComponents`]([ -
div1
, -
div2
, -
div3
, +
div1
, +
div2
, +
div3
, ]) }, }, @@ -175,49 +172,66 @@ describe(`develop-static-entry`, () => { }) }) -describe(`static-entry`, () => { +describe(`static-entry sanity checks`, () => { beforeEach(() => { global.__PATH_PREFIX__ = `` }) - test(`onPreRenderHTML can be used to replace headComponents`, done => { - global.plugins = [fakeStylesPlugin, reverseHeadersPlugin] + const methodsToCheck = [ + `replaceHeadComponents`, + `replacePreBodyComponents`, + `replacePostBodyComponents`, + ] - StaticEntry(`/about/`, (_, html) => { - expect(html).toMatchSnapshot() - done() + methodsToCheck.forEach(methodName => { + test(`${methodName} can filter out null value`, done => { + const plugin = injectValuePlugin(`onPreRenderHTML`, methodName, null) + global.plugins = [plugin, checkNonEmptyHeadersPlugin] + + StaticEntry(`/about/`, (_, html) => { + done() + }) }) - }) - test(`onPreRenderHTML can filter out null value`, done => { - global.plugins = [setNullHeaderPlugin, checkNonEmptyHeadersPlugin] + test(`${methodName} can filter out null values`, done => { + const plugin = injectValuePlugin(`onPreRenderHTML`, methodName, [ + null, + null, + ]) + global.plugins = [plugin, checkNonEmptyHeadersPlugin] - StaticEntry(`/about/`, (_, html) => { - expect(html).toMatchSnapshot() - done() + StaticEntry(`/about/`, (_, html) => { + done() + }) }) - }) - test(`onPreRenderHTML can filter out null values`, done => { - global.plugins = [setNullHeadersPlugin, checkNonEmptyHeadersPlugin] + test(`${methodName} can filter out empty array`, done => { + const plugin = injectValuePlugin(`onPreRenderHTML`, methodName, []) + global.plugins = [plugin, checkNonEmptyHeadersPlugin] - StaticEntry(`/about/`, (_, html) => { - expect(html).toMatchSnapshot() - done() + StaticEntry(`/about/`, (_, html) => { + done() + }) }) - }) - test(`onPreRenderHTML can filter out empty array`, done => { - global.plugins = [setEmptyArrayHeaderPlugin, checkNonEmptyHeadersPlugin] + test(`${methodName} can filter out empty arrays`, done => { + const plugin = injectValuePlugin(`onPreRenderHTML`, methodName, [[], []]) + global.plugins = [plugin, checkNonEmptyHeadersPlugin] - StaticEntry(`/about/`, (_, html) => { - expect(html).toMatchSnapshot() - done() + StaticEntry(`/about/`, (_, html) => { + done() + }) }) }) +}) + +describe(`static-entry`, () => { + beforeEach(() => { + global.__PATH_PREFIX__ = `` + }) - test(`onPreRenderHTML can filter out empty arrays`, done => { - global.plugins = [setEmptyArrayHeadersPlugin, checkNonEmptyHeadersPlugin] + test(`onPreRenderHTML can be used to replace headComponents`, done => { + global.plugins = [fakeStylesPlugin, reverseHeadersPlugin] StaticEntry(`/about/`, (_, html) => { expect(html).toMatchSnapshot() diff --git a/packages/gatsby/cache-dir/static-entry.js b/packages/gatsby/cache-dir/static-entry.js index f339b6037e494..d3fbebd1a523c 100644 --- a/packages/gatsby/cache-dir/static-entry.js +++ b/packages/gatsby/cache-dir/static-entry.js @@ -49,7 +49,7 @@ const getPage = path => pagesObjectMap.get(path) const createElement = React.createElement -const sanitizeHeadComponents = components => { +const sanitizeComponents = components => { if (Array.isArray(components)) { // remove falsy items return components.filter(val => (Array.isArray(val) ? val.length > 0 : val)) @@ -79,7 +79,7 @@ export default (pagePath, callback) => { } const setHeadComponents = components => { - headComponents = headComponents.concat(sanitizeHeadComponents(components)) + headComponents = headComponents.concat(sanitizeComponents(components)) } const setHtmlAttributes = attributes => { @@ -91,11 +91,13 @@ export default (pagePath, callback) => { } const setPreBodyComponents = components => { - preBodyComponents = preBodyComponents.concat(components) + preBodyComponents = preBodyComponents.concat(sanitizeComponents(components)) } const setPostBodyComponents = components => { - postBodyComponents = postBodyComponents.concat(components) + postBodyComponents = postBodyComponents.concat( + sanitizeComponents(components) + ) } const setBodyProps = props => { @@ -105,19 +107,19 @@ export default (pagePath, callback) => { const getHeadComponents = () => headComponents const replaceHeadComponents = components => { - headComponents = sanitizeHeadComponents(components) + headComponents = sanitizeComponents(components) } const getPreBodyComponents = () => preBodyComponents const replacePreBodyComponents = components => { - preBodyComponents = components + preBodyComponents = sanitizeComponents(components) } const getPostBodyComponents = () => postBodyComponents const replacePostBodyComponents = components => { - postBodyComponents = components + postBodyComponents = sanitizeComponents(components) } const page = getPage(pagePath)