From 5770973196024de72101feab5e20860df8d94f74 Mon Sep 17 00:00:00 2001 From: pieh Date: Thu, 14 Jul 2022 14:32:35 +0200 Subject: [PATCH 1/9] feat: deduplicate head elements on id attrbibute in browser --- .../head/head-export-handler-for-browser.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 index 758e6011e1ada..d4113e15e7ff2 100644 --- a/packages/gatsby/cache-dir/head/head-export-handler-for-browser.js +++ b/packages/gatsby/cache-dir/head/head-export-handler-for-browser.js @@ -22,15 +22,28 @@ const onHeadRendered = () => { removePrevHeadElements() + const seenIds = new Map() for (const node of hiddenRoot.childNodes) { const nodeName = node.nodeName.toLowerCase() + const id = node.attributes.id?.value if (!VALID_NODE_NAMES.includes(nodeName)) { warnForInvalidTags(nodeName) } else { const clonedNode = node.cloneNode(true) clonedNode.setAttribute(`data-gatsby-head`, true) - validHeadNodes.push(clonedNode) + if (id) { + if (!seenIds.has(id)) { + validHeadNodes.push(clonedNode) + seenIds.set(id, validHeadNodes.length - 1) + } else { + const indexOfPreviouslyInsertedNode = seenIds.get(id) + validHeadNodes[indexOfPreviouslyInsertedNode].remove() + validHeadNodes[indexOfPreviouslyInsertedNode] = clonedNode + } + } else { + validHeadNodes.push(clonedNode) + } } } From 1a327d51436aeb823ba3e9a8a63679ba83ac7eac Mon Sep 17 00:00:00 2001 From: pieh Date: Thu, 14 Jul 2022 14:37:34 +0200 Subject: [PATCH 2/9] feat: deduplicate head elements on id attrbibute in html gen --- .../cache-dir/head/head-export-handler-for-ssr.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/gatsby/cache-dir/head/head-export-handler-for-ssr.js b/packages/gatsby/cache-dir/head/head-export-handler-for-ssr.js index 0aa7d53e5dac7..2003321cf71cb 100644 --- a/packages/gatsby/cache-dir/head/head-export-handler-for-ssr.js +++ b/packages/gatsby/cache-dir/head/head-export-handler-for-ssr.js @@ -54,8 +54,10 @@ export function headHandlerForSSR({ const validHeadNodes = [] + const seenIds = new Map() for (const node of headNodes) { const { rawTagName, attributes } = node + const id = attributes.id if (!VALID_NODE_NAMES.includes(rawTagName)) { warnForInvalidTags(rawTagName) @@ -68,8 +70,17 @@ export function headHandlerForSSR({ }, node.childNodes[0]?.textContent ) - - validHeadNodes.push(element) + if (id) { + if (!seenIds.has(id)) { + validHeadNodes.push(element) + seenIds.set(id, validHeadNodes.length - 1) + } else { + const indexOfPreviouslyInsertedNode = seenIds.get(id) + validHeadNodes[indexOfPreviouslyInsertedNode] = element + } + } else { + validHeadNodes.push(element) + } } } From a8c015aa1599112d3dc6978e54f77dca2c33410c Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 15 Jul 2022 14:52:29 +0200 Subject: [PATCH 3/9] page with head deduplication --- .../head-function-export/deduplication.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js diff --git a/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js new file mode 100644 index 0000000000000..6fe27bfa9bff5 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js @@ -0,0 +1,43 @@ +import * as React from "react" + +export default function HeadFunctionExportBasic() { + return ( + <> +

+ I deduplicated Head elements by their id +

+ + ) +} + +function SEO({ children }) { + return ( + <> + + + {children} + + ) +} + +export function Head() { + return ( + + + + + ) +} From 2963da8c52d34bac6d27d3941641a421a45de364 Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 15 Jul 2022 15:43:31 +0200 Subject: [PATCH 4/9] add test --- .../head-function-export/deduplication.js | 19 +++++++++++++++++++ .../shared-data/head-function-export.js | 1 + .../head-function-export/deduplication.js | 12 ++---------- 3 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js diff --git a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js new file mode 100644 index 0000000000000..0bb468e7ca623 --- /dev/null +++ b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js @@ -0,0 +1,19 @@ +import headFunctionExportSharedData from "../../../shared-data/head-function-export.js" + +it(`Head function export receive correct props`, () => { + cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange() + + // icon has id and should be deduplicated + cy.get(`link[rel=deduplication]`).should("have.length", 1) + // last icon should win + cy.get(`link[rel=deduplication]`).should("have.attr", "href", "/bar") + // we should preserve id + cy.get(`link[rel=deduplication]`).should( + "have.attr", + "id", + "deduplication-test" + ) + + // alternate links are not using id, so should have multiple instances + cy.get(`link[rel=alternate]`).should("have.length", 2) +}) diff --git a/e2e-tests/development-runtime/shared-data/head-function-export.js b/e2e-tests/development-runtime/shared-data/head-function-export.js index 05ea7b7bf8f8b..2acbefa3aced6 100644 --- a/e2e-tests/development-runtime/shared-data/head-function-export.js +++ b/e2e-tests/development-runtime/shared-data/head-function-export.js @@ -11,6 +11,7 @@ const page = { ssr: `${path}/ssr/`, invalidElements: `${path}/invalid-elements/`, fsRouteApi: `${path}/fs-route-api/`, + deduplication: `${path}/deduplication/`, } const data = { diff --git a/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js index 6fe27bfa9bff5..1f52c35ce06d5 100644 --- a/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js +++ b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js @@ -13,11 +13,7 @@ export default function HeadFunctionExportBasic() { function SEO({ children }) { return ( <> - + - + ) From 3157d104ca1d3b9796f6dd652ab4c4bfef60e40f Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 15 Jul 2022 15:53:57 +0200 Subject: [PATCH 5/9] update comments to match current code --- .../cypress/integration/head-function-export/deduplication.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js index 0bb468e7ca623..23db5a0f0147f 100644 --- a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js +++ b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js @@ -3,9 +3,9 @@ import headFunctionExportSharedData from "../../../shared-data/head-function-exp it(`Head function export receive correct props`, () => { cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange() - // icon has id and should be deduplicated + // deduplication link has id and should be deduplicated cy.get(`link[rel=deduplication]`).should("have.length", 1) - // last icon should win + // last deduplication link should win cy.get(`link[rel=deduplication]`).should("have.attr", "href", "/bar") // we should preserve id cy.get(`link[rel=deduplication]`).should( From 1a6c5c5850879e951400a7e99853066c51bca393 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Fri, 15 Jul 2022 16:22:03 +0200 Subject: [PATCH 6/9] Update e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js Co-authored-by: Jude Agboola --- .../cypress/integration/head-function-export/deduplication.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js index 23db5a0f0147f..588e688fc6c9a 100644 --- a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js +++ b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js @@ -1,6 +1,6 @@ import headFunctionExportSharedData from "../../../shared-data/head-function-export.js" -it(`Head function export receive correct props`, () => { +it(`Deduplicates tags multiple tags with same id`, () => { cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange() // deduplication link has id and should be deduplicated From 83ec61dd487edf3d9bc409131975937c5dcf096a Mon Sep 17 00:00:00 2001 From: Jude Agboola Date: Fri, 15 Jul 2022 15:23:27 +0100 Subject: [PATCH 7/9] Update e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js --- .../cypress/integration/head-function-export/deduplication.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js index 588e688fc6c9a..1c6efdae12c39 100644 --- a/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js +++ b/e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js @@ -1,6 +1,6 @@ import headFunctionExportSharedData from "../../../shared-data/head-function-export.js" -it(`Deduplicates tags multiple tags with same id`, () => { +it(`Deduplicates multiple tags with same id`, () => { cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange() // deduplication link has id and should be deduplicated From a6a78207064c9476b5b10e1baaf730160540b501 Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 15 Jul 2022 17:10:29 +0200 Subject: [PATCH 8/9] add test case to e2e-production --- .../head-function-export/deduplication.js | 2 +- .../head-function-export/deduplication.js | 19 ++++++++++ .../shared-data/head-function-export.js | 1 + .../head-function-export/deduplication.js | 35 +++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 e2e-tests/production-runtime/cypress/integration/head-function-export/deduplication.js create mode 100644 e2e-tests/production-runtime/src/pages/head-function-export/deduplication.js diff --git a/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js index 1f52c35ce06d5..53302923a3462 100644 --- a/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js +++ b/e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js @@ -1,6 +1,6 @@ import * as React from "react" -export default function HeadFunctionExportBasic() { +export default function HeadFunctionDeduplication() { return ( <>

diff --git a/e2e-tests/production-runtime/cypress/integration/head-function-export/deduplication.js b/e2e-tests/production-runtime/cypress/integration/head-function-export/deduplication.js new file mode 100644 index 0000000000000..1c6efdae12c39 --- /dev/null +++ b/e2e-tests/production-runtime/cypress/integration/head-function-export/deduplication.js @@ -0,0 +1,19 @@ +import headFunctionExportSharedData from "../../../shared-data/head-function-export.js" + +it(`Deduplicates multiple tags with same id`, () => { + cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange() + + // deduplication link has id and should be deduplicated + cy.get(`link[rel=deduplication]`).should("have.length", 1) + // last deduplication link should win + cy.get(`link[rel=deduplication]`).should("have.attr", "href", "/bar") + // we should preserve id + cy.get(`link[rel=deduplication]`).should( + "have.attr", + "id", + "deduplication-test" + ) + + // alternate links are not using id, so should have multiple instances + cy.get(`link[rel=alternate]`).should("have.length", 2) +}) diff --git a/e2e-tests/production-runtime/shared-data/head-function-export.js b/e2e-tests/production-runtime/shared-data/head-function-export.js index c20460ae662ff..7f0d5cd8c98b8 100644 --- a/e2e-tests/production-runtime/shared-data/head-function-export.js +++ b/e2e-tests/production-runtime/shared-data/head-function-export.js @@ -11,6 +11,7 @@ const page = { ssr: `${path}/ssr/`, invalidElements: `${path}/invalid-elements/`, fsRouteApi: `${path}/fs-route-api/`, + deduplication: `${path}/deduplication/`, } const data = { diff --git a/e2e-tests/production-runtime/src/pages/head-function-export/deduplication.js b/e2e-tests/production-runtime/src/pages/head-function-export/deduplication.js new file mode 100644 index 0000000000000..53302923a3462 --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/head-function-export/deduplication.js @@ -0,0 +1,35 @@ +import * as React from "react" + +export default function HeadFunctionDeduplication() { + return ( + <> +

+ I deduplicated Head elements by their id +

+ + ) +} + +function SEO({ children }) { + return ( + <> + + + {children} + + ) +} + +export function Head() { + return ( + + + + + ) +} From 35f358ded6962749e9b4327372fb9c8feb4bb8ac Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 15 Jul 2022 17:25:02 +0200 Subject: [PATCH 9/9] add test case to head integration tests --- .../__tests__/ssr-html-output.js | 19 ++++++++++ .../head-function-export/package.json | 1 + .../shared-data/head-function-export.js | 1 + .../head-function-export/deduplication.js | 35 +++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 integration-tests/head-function-export/src/pages/head-function-export/deduplication.js diff --git a/integration-tests/head-function-export/__tests__/ssr-html-output.js b/integration-tests/head-function-export/__tests__/ssr-html-output.js index f60c7cd47a1ad..1a355073a9c90 100644 --- a/integration-tests/head-function-export/__tests__/ssr-html-output.js +++ b/integration-tests/head-function-export/__tests__/ssr-html-output.js @@ -73,4 +73,23 @@ describe(`Head function export SSR'ed HTML output`, () => { expect(style.text).toContain(data.queried.style) expect(link.attributes.href).toEqual(data.queried.link) }) + + it(`deduplicates multiple tags with same id`, () => { + const html = readFileSync(`${publicDir}${page.deduplication}/index.html`) + const dom = parse(html) + + // deduplication link has id and should be deduplicated + expect(dom.querySelectorAll(`link[rel=deduplication]`)?.length).toEqual(1) + // last deduplication link should win + expect( + dom.querySelector(`link[rel=deduplication]`)?.attributes?.href + ).toEqual("/bar") + // we should preserve id + expect( + dom.querySelector(`link[rel=deduplication]`)?.attributes?.id + ).toEqual("deduplication-test") + + // alternate links are not using id, so should have multiple instances + expect(dom.querySelectorAll(`link[rel=alternate]`)?.length).toEqual(2) + }) }) diff --git a/integration-tests/head-function-export/package.json b/integration-tests/head-function-export/package.json index 112e6bb1e8b45..3005b8f69b4ff 100644 --- a/integration-tests/head-function-export/package.json +++ b/integration-tests/head-function-export/package.json @@ -19,6 +19,7 @@ "babel-preset-gatsby-package": "^2.4.0", "fs-extra": "^10.0.0", "jest": "^27.2.1", + "node-html-parser": "^5.3.3", "npm-run-all": "4.1.5" }, "dependencies": { diff --git a/integration-tests/head-function-export/shared-data/head-function-export.js b/integration-tests/head-function-export/shared-data/head-function-export.js index 056f993b590c3..90febb55cbdbc 100644 --- a/integration-tests/head-function-export/shared-data/head-function-export.js +++ b/integration-tests/head-function-export/shared-data/head-function-export.js @@ -7,6 +7,7 @@ const page = { staticQuery: `${path}/static-query-component/`, warnings: `${path}/warnings/`, allProps: `${path}/all-props/`, + deduplication: `${path}/deduplication/`, } const data = { diff --git a/integration-tests/head-function-export/src/pages/head-function-export/deduplication.js b/integration-tests/head-function-export/src/pages/head-function-export/deduplication.js new file mode 100644 index 0000000000000..53302923a3462 --- /dev/null +++ b/integration-tests/head-function-export/src/pages/head-function-export/deduplication.js @@ -0,0 +1,35 @@ +import * as React from "react" + +export default function HeadFunctionDeduplication() { + return ( + <> +

+ I deduplicated Head elements by their id +

+ + ) +} + +function SEO({ children }) { + return ( + <> + + + {children} + + ) +} + +export function Head() { + return ( + + + + + ) +}