Skip to content

Commit 6f0d46b

Browse files
gatsbybotpieh
gatsbybot
and
pieh
authored
feat(gatsby): allow deduplicating head elements on id (#36138) (#36159)
* feat: deduplicate head elements on id attrbibute in browser * feat: deduplicate head elements on id attrbibute in html gen * page with head deduplication * add test * update comments to match current code * Update e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js Co-authored-by: Jude Agboola <[email protected]> * Update e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js * add test case to e2e-production * add test case to head integration tests Co-authored-by: Jude Agboola <[email protected]> (cherry picked from commit b08ef18) Co-authored-by: Michal Piechowiak <[email protected]>
1 parent 62d2a49 commit 6f0d46b

File tree

12 files changed

+193
-3
lines changed

12 files changed

+193
-3
lines changed

‎e2e-tests/development-runtime/cypress/integration/head-function-export/deduplication.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import headFunctionExportSharedData from "../../../shared-data/head-function-export.js"
2+
3+
it(`Deduplicates multiple tags with same id`, () => {
4+
cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange()
5+
6+
// deduplication link has id and should be deduplicated
7+
cy.get(`link[rel=deduplication]`).should("have.length", 1)
8+
// last deduplication link should win
9+
cy.get(`link[rel=deduplication]`).should("have.attr", "href", "/bar")
10+
// we should preserve id
11+
cy.get(`link[rel=deduplication]`).should(
12+
"have.attr",
13+
"id",
14+
"deduplication-test"
15+
)
16+
17+
// alternate links are not using id, so should have multiple instances
18+
cy.get(`link[rel=alternate]`).should("have.length", 2)
19+
})

‎e2e-tests/development-runtime/shared-data/head-function-export.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const page = {
1111
ssr: `${path}/ssr/`,
1212
invalidElements: `${path}/invalid-elements/`,
1313
fsRouteApi: `${path}/fs-route-api/`,
14+
deduplication: `${path}/deduplication/`,
1415
}
1516

1617
const data = {

‎e2e-tests/development-runtime/src/pages/head-function-export/deduplication.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as React from "react"
2+
3+
export default function HeadFunctionDeduplication() {
4+
return (
5+
<>
6+
<h1>
7+
I deduplicated Head elements by their <code>id</code>
8+
</h1>
9+
</>
10+
)
11+
}
12+
13+
function SEO({ children }) {
14+
return (
15+
<>
16+
<link rel="deduplication" id="deduplication-test" href="/foo" />
17+
<link
18+
rel="alternate"
19+
type="application/atom+xml"
20+
title="RSS Feed"
21+
href="/blog/news/atom"
22+
/>
23+
{children}
24+
</>
25+
)
26+
}
27+
28+
export function Head() {
29+
return (
30+
<SEO>
31+
<link rel="deduplication" id="deduplication-test" href="/bar" />
32+
<link rel="alternate" hrefLang="de-DE" href="/de/" />
33+
</SEO>
34+
)
35+
}

‎e2e-tests/production-runtime/cypress/integration/head-function-export/deduplication.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import headFunctionExportSharedData from "../../../shared-data/head-function-export.js"
2+
3+
it(`Deduplicates multiple tags with same id`, () => {
4+
cy.visit(headFunctionExportSharedData.page.deduplication).waitForRouteChange()
5+
6+
// deduplication link has id and should be deduplicated
7+
cy.get(`link[rel=deduplication]`).should("have.length", 1)
8+
// last deduplication link should win
9+
cy.get(`link[rel=deduplication]`).should("have.attr", "href", "/bar")
10+
// we should preserve id
11+
cy.get(`link[rel=deduplication]`).should(
12+
"have.attr",
13+
"id",
14+
"deduplication-test"
15+
)
16+
17+
// alternate links are not using id, so should have multiple instances
18+
cy.get(`link[rel=alternate]`).should("have.length", 2)
19+
})

‎e2e-tests/production-runtime/shared-data/head-function-export.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const page = {
1111
ssr: `${path}/ssr/`,
1212
invalidElements: `${path}/invalid-elements/`,
1313
fsRouteApi: `${path}/fs-route-api/`,
14+
deduplication: `${path}/deduplication/`,
1415
}
1516

1617
const data = {

‎e2e-tests/production-runtime/src/pages/head-function-export/deduplication.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as React from "react"
2+
3+
export default function HeadFunctionDeduplication() {
4+
return (
5+
<>
6+
<h1>
7+
I deduplicated Head elements by their <code>id</code>
8+
</h1>
9+
</>
10+
)
11+
}
12+
13+
function SEO({ children }) {
14+
return (
15+
<>
16+
<link rel="deduplication" id="deduplication-test" href="/foo" />
17+
<link
18+
rel="alternate"
19+
type="application/atom+xml"
20+
title="RSS Feed"
21+
href="/blog/news/atom"
22+
/>
23+
{children}
24+
</>
25+
)
26+
}
27+
28+
export function Head() {
29+
return (
30+
<SEO>
31+
<link rel="deduplication" id="deduplication-test" href="/bar" />
32+
<link rel="alternate" hrefLang="de-DE" href="/de/" />
33+
</SEO>
34+
)
35+
}

‎integration-tests/head-function-export/__tests__/ssr-html-output.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,23 @@ describe(`Head function export SSR'ed HTML output`, () => {
7676
expect(style.text).toContain(data.queried.style)
7777
expect(link.attributes.href).toEqual(data.queried.link)
7878
})
79+
80+
it(`deduplicates multiple tags with same id`, () => {
81+
const html = readFileSync(`${publicDir}${page.deduplication}/index.html`)
82+
const dom = parse(html)
83+
84+
// deduplication link has id and should be deduplicated
85+
expect(dom.querySelectorAll(`link[rel=deduplication]`)?.length).toEqual(1)
86+
// last deduplication link should win
87+
expect(
88+
dom.querySelector(`link[rel=deduplication]`)?.attributes?.href
89+
).toEqual("/bar")
90+
// we should preserve id
91+
expect(
92+
dom.querySelector(`link[rel=deduplication]`)?.attributes?.id
93+
).toEqual("deduplication-test")
94+
95+
// alternate links are not using id, so should have multiple instances
96+
expect(dom.querySelectorAll(`link[rel=alternate]`)?.length).toEqual(2)
97+
})
7998
})

‎integration-tests/head-function-export/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"babel-preset-gatsby-package": "^2.4.0",
2020
"fs-extra": "^10.0.0",
2121
"jest": "^27.2.1",
22+
"node-html-parser": "^5.3.3",
2223
"npm-run-all": "4.1.5"
2324
},
2425
"dependencies": {

‎integration-tests/head-function-export/shared-data/head-function-export.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const page = {
77
staticQuery: `${path}/static-query-component/`,
88
warnings: `${path}/warnings/`,
99
allProps: `${path}/all-props/`,
10+
deduplication: `${path}/deduplication/`,
1011
}
1112

1213
const data = {

‎integration-tests/head-function-export/src/pages/head-function-export/deduplication.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as React from "react"
2+
3+
export default function HeadFunctionDeduplication() {
4+
return (
5+
<>
6+
<h1>
7+
I deduplicated Head elements by their <code>id</code>
8+
</h1>
9+
</>
10+
)
11+
}
12+
13+
function SEO({ children }) {
14+
return (
15+
<>
16+
<link rel="deduplication" id="deduplication-test" href="/foo" />
17+
<link
18+
rel="alternate"
19+
type="application/atom+xml"
20+
title="RSS Feed"
21+
href="/blog/news/atom"
22+
/>
23+
{children}
24+
</>
25+
)
26+
}
27+
28+
export function Head() {
29+
return (
30+
<SEO>
31+
<link rel="deduplication" id="deduplication-test" href="/bar" />
32+
<link rel="alternate" hrefLang="de-DE" href="/de/" />
33+
</SEO>
34+
)
35+
}

‎packages/gatsby/cache-dir/head/head-export-handler-for-browser.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,28 @@ const onHeadRendered = () => {
2222

2323
removePrevHeadElements()
2424

25+
const seenIds = new Map()
2526
for (const node of hiddenRoot.childNodes) {
2627
const nodeName = node.nodeName.toLowerCase()
28+
const id = node.attributes.id?.value
2729

2830
if (!VALID_NODE_NAMES.includes(nodeName)) {
2931
warnForInvalidTags(nodeName)
3032
} else {
3133
const clonedNode = node.cloneNode(true)
3234
clonedNode.setAttribute(`data-gatsby-head`, true)
33-
validHeadNodes.push(clonedNode)
35+
if (id) {
36+
if (!seenIds.has(id)) {
37+
validHeadNodes.push(clonedNode)
38+
seenIds.set(id, validHeadNodes.length - 1)
39+
} else {
40+
const indexOfPreviouslyInsertedNode = seenIds.get(id)
41+
validHeadNodes[indexOfPreviouslyInsertedNode].remove()
42+
validHeadNodes[indexOfPreviouslyInsertedNode] = clonedNode
43+
}
44+
} else {
45+
validHeadNodes.push(clonedNode)
46+
}
3447
}
3548
}
3649

‎packages/gatsby/cache-dir/head/head-export-handler-for-ssr.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ export function headHandlerForSSR({
5454

5555
const validHeadNodes = []
5656

57+
const seenIds = new Map()
5758
for (const node of headNodes) {
5859
const { rawTagName, attributes } = node
60+
const id = attributes.id
5961

6062
if (!VALID_NODE_NAMES.includes(rawTagName)) {
6163
warnForInvalidTags(rawTagName)
@@ -68,8 +70,17 @@ export function headHandlerForSSR({
6870
},
6971
node.childNodes[0]?.textContent
7072
)
71-
72-
validHeadNodes.push(element)
73+
if (id) {
74+
if (!seenIds.has(id)) {
75+
validHeadNodes.push(element)
76+
seenIds.set(id, validHeadNodes.length - 1)
77+
} else {
78+
const indexOfPreviouslyInsertedNode = seenIds.get(id)
79+
validHeadNodes[indexOfPreviouslyInsertedNode] = element
80+
}
81+
} else {
82+
validHeadNodes.push(element)
83+
}
7384
}
7485
}
7586

0 commit comments

Comments
 (0)