Skip to content

Commit ae95d66

Browse files
gatsbybottyhopp
andauthored
fix(gatsby-script): Make load callback work when both load and error callbacks defined (#35760) (#35787)
Co-authored-by: Ty Hopp <[email protected]>
1 parent 9f5c107 commit ae95d66

File tree

7 files changed

+92
-6
lines changed

7 files changed

+92
-6
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
describe(`both onLoad and onError callbacks are declared`, () => {
2+
beforeEach(() => {
3+
cy.visit(`/gatsby-script-both-callbacks/`).waitForRouteChange()
4+
})
5+
6+
it(`should execute the onLoad callback`, () => {
7+
cy.get(`[data-on-load-result=both-callbacks]`).should(`have.length`, 1)
8+
})
9+
10+
it(`should execute the onError callback`, () => {
11+
cy.get(`[data-on-error-result=both-callbacks]`).should(`have.length`, 1)
12+
})
13+
})
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import React from "react"
2+
import { Script } from "gatsby"
3+
import { scripts } from "../../gatsby-script-scripts"
4+
import { onLoad, onError } from "../utils/gatsby-script-callbacks"
5+
6+
const BothCallbacksPage = () => {
7+
return (
8+
<main>
9+
<h1>Script component e2e test</h1>
10+
11+
<Script
12+
src={scripts.marked}
13+
onLoad={() => {
14+
onLoad(`both-callbacks`)
15+
}}
16+
onError={() => {}}
17+
/>
18+
19+
<Script
20+
src="/non-existent-script.js"
21+
onLoad={() => {}}
22+
onError={() => {
23+
onError(`both-callbacks`)
24+
}}
25+
/>
26+
</main>
27+
)
28+
}
29+
30+
export default BothCallbacksPage

e2e-tests/development-runtime/src/pages/gatsby-script-duplicate-scripts.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Script } from "gatsby"
33
import { scripts } from "../../gatsby-script-scripts"
44
import { onLoad, onError } from "../utils/gatsby-script-callbacks"
55

6-
const DuplicateScripts = () => {
6+
const DuplicateScriptsPage = () => {
77
const [onLoadScriptLoaded, setOnLoadScriptLoaded] = useState(false)
88
const [onErrorScriptLoaded, setOnErrorScriptLoaded] = useState(false)
99
const [secondOnLoadScriptLoaded, setSecondOnLoadScriptLoaded] = useState(
@@ -98,4 +98,4 @@ const DuplicateScripts = () => {
9898
)
9999
}
100100

101-
export default DuplicateScripts
101+
export default DuplicateScriptsPage
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
describe(`both onLoad and onError callbacks are declared`, () => {
2+
beforeEach(() => {
3+
cy.visit(`/gatsby-script-both-callbacks/`).waitForRouteChange()
4+
})
5+
6+
it(`should execute the onLoad callback`, () => {
7+
cy.get(`[data-on-load-result=both-callbacks]`).should(`have.length`, 1)
8+
})
9+
10+
it(`should execute the onError callback`, () => {
11+
cy.get(`[data-on-error-result=both-callbacks]`).should(`have.length`, 1)
12+
})
13+
})
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import React from "react"
2+
import { Script } from "gatsby"
3+
import { scripts } from "../../gatsby-script-scripts"
4+
import { onLoad, onError } from "../utils/gatsby-script-callbacks"
5+
6+
const BothCallbacksPage = () => {
7+
return (
8+
<main>
9+
<h1>Script component e2e test</h1>
10+
11+
<Script
12+
src={scripts.marked}
13+
onLoad={() => {
14+
onLoad(`both-callbacks`)
15+
}}
16+
onError={() => {}}
17+
/>
18+
19+
<Script
20+
src="/non-existent-script.js"
21+
onLoad={() => {}}
22+
onError={() => {
23+
onError(`both-callbacks`)
24+
}}
25+
/>
26+
</main>
27+
)
28+
}
29+
30+
export default BothCallbacksPage

e2e-tests/production-runtime/src/pages/gatsby-script-duplicate-scripts.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Script } from "gatsby"
33
import { scripts } from "../../gatsby-script-scripts"
44
import { onLoad, onError } from "../utils/gatsby-script-callbacks"
55

6-
const DuplicateScripts = () => {
6+
const DuplicateScriptsPage = () => {
77
const [onLoadScriptLoaded, setOnLoadScriptLoaded] = useState(false)
88
const [onErrorScriptLoaded, setOnErrorScriptLoaded] = useState(false)
99

@@ -58,4 +58,4 @@ const DuplicateScripts = () => {
5858
)
5959
}
6060

61-
export default DuplicateScripts
61+
export default DuplicateScriptsPage

packages/gatsby-script/src/gatsby-script.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,17 +153,17 @@ function injectScript(props: ScriptProps): IInjectedScriptDetails | null {
153153
* If a duplicate script is already loaded/errored, we replay load/error callbacks with the original event.
154154
* If it's not yet loaded/errored, keep track of callbacks so we can call load/error callbacks for each when the event occurs.
155155
*/
156-
const cachedCallbacks = scriptCallbackCache.get(scriptKey) || {}
157-
158156
for (const name of callbackNames) {
159157
if (currentCallbacks?.[name]) {
158+
const cachedCallbacks = scriptCallbackCache.get(scriptKey) || {}
160159
const { callbacks = [] } = cachedCallbacks?.[name] || {}
161160
callbacks.push(currentCallbacks?.[name])
162161

163162
if (cachedCallbacks?.[name]?.event) {
164163
currentCallbacks?.[name]?.(cachedCallbacks?.[name]?.event)
165164
} else {
166165
scriptCallbackCache.set(scriptKey, {
166+
...cachedCallbacks,
167167
[name]: {
168168
callbacks,
169169
},

0 commit comments

Comments
 (0)