Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gatsby-script): Duplicate script callbacks if no injected script callbacks #35717

Merged
merged 5 commits into from
May 23, 2022
Merged

feat(gatsby-script): Duplicate script callbacks if no injected script callbacks #35717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a71e155
refactor(gatsby-script): Only dedupe if id or src exists
tyhopp May 23, 2022
cfb9d1c
test(gatsby-script): Add waitForRouteChange on visits
tyhopp May 23, 2022
4473745
feat(gatsby-script): Execute duplicate script callbacks even if first…
tyhopp May 23, 2022
7571200
chore(gatsby-script): Add comment about listener behavior
tyhopp May 23, 2022
a6c5120
feat(gatsby-script): Adjust remove event listener logic
tyhopp May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion .../development-runtime/cypress/integration/gatsby-script/gatsby-script-duplicate-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe(`duplicate scripts`, () => {
beforeEach(() => {
cy.visit(`/gatsby-script-duplicate-scripts/`)
cy.visit(`/gatsby-script-duplicate-scripts/`).waitForRouteChange()
})

it(`should execute load callbacks of duplicate scripts`, () => {
Expand All @@ -14,4 +14,25 @@ describe(`duplicate scripts`, () => {
cy.get(`[data-on-error-result=duplicate-2]`).should(`have.length`, 1)
cy.get(`[data-on-error-result=duplicate-3]`).should(`have.length`, 1)
})

it(`should execute load callbacks even if the first script has no declared load callback`, () => {
cy.get(`[data-on-load-result=duplicate-first-script-no-callback]`).should(
`have.length`,
1
)
cy.get(`[data-on-load-result=duplicate-first-script-no-callback-2]`).should(
`have.length`,
1
)
})

it(`should execute error callbacks even if the first script has no declared error callback`, () => {
cy.get(`[data-on-error-result=duplicate-first-script-no-callback]`).should(
`have.length`,
1
)
cy.get(
`[data-on-error-result=duplicate-first-script-no-callback-2]`
).should(`have.length`, 1)
})
})
16 changes: 8 additions & 8 deletions ...sts/development-runtime/cypress/integration/gatsby-script/gatsby-script-inline-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
describe(`inline scripts set via ${descriptor}`, () => {
describe(`using the post-hydrate strategy`, () => {
it(`should execute successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(
`post-hydrate-${inlineScriptType}`,
Expand All @@ -36,7 +36,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load after the framework bundle has loaded`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

// Assert framework is loaded before inline script is executed
cy.getRecord(
Expand All @@ -53,7 +53,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {

describe(`using the idle strategy`, () => {
it(`should execute successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(`idle-${inlineScriptType}`, `success`, true).should(
`equal`,
Expand All @@ -62,7 +62,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load after other strategies`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(`idle-${inlineScriptType}`, markRecord.executeStart).then(
dangerouslySetExecuteStart => {
Expand All @@ -77,7 +77,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {

describe(`when navigation occurs`, () => {
it(`should load only once on initial page load`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.get(`table[id=script-mark-records] tbody`)
.children()
Expand Down Expand Up @@ -112,7 +112,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once after anchor link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=anchor-link-back-to-index]`).click()
cy.url().should(`contain`, page.navigation)
cy.get(`a[href="${page.target}"][id=anchor-link]`).click()
Expand Down Expand Up @@ -153,7 +153,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once after Gatsby link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=gatsby-link-back-to-index]`).click()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()

Expand All @@ -172,7 +172,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once if the page is revisited via browser back/forward buttons after Gatsby link navigation`, () => {
cy.visit(page.navigation)
cy.visit(page.navigation).waitForRouteChange()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()
cy.go(`back`)
cy.go(`forward`)
Expand Down
28 changes: 14 additions & 14 deletions ...velopment-runtime/cypress/integration/gatsby-script/gatsby-script-scripts-with-sources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ const page = {
describe(`scripts with sources`, () => {
describe(`using the post-hydrate strategy`, () => {
it(`should load successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.getRecord(script.three, `success`, true).should(`equal`, `true`)
})

it(`should load after the framework bundle has loaded`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

// Assert framework is loaded before three starts loading
cy.getRecord(script.three, resourceRecord.fetchStart).then(
Expand All @@ -28,26 +28,26 @@ describe(`scripts with sources`, () => {
})

it(`should call an on load callback once the script has loaded`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.getRecord(script.three, resourceRecord.responseEnd).then(() => {
cy.get(`[data-on-load-result=post-hydrate]`)
})
})

it(`should call an on error callback if an error occurred`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`[data-on-error-result=post-hydrate]`)
})
})

describe(`using the idle strategy`, () => {
it(`should load successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.getRecord(script.marked, `success`, true).should(`equal`, `true`)
})

it(`should load after other strategies`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(script.marked, resourceRecord.fetchStart).then(
markedFetchStart => {
Expand All @@ -60,21 +60,21 @@ describe(`scripts with sources`, () => {
})

it(`should call an on load callback once the script has loaded`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.getRecord(script.marked, resourceRecord.responseEnd).then(() => {
cy.get(`[data-on-load-result=idle]`)
})
})

it(`should call an on error callback if an error occurred`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`[data-on-error-result=idle]`)
})
})

describe(`when navigation occurs`, () => {
it(`should load only once on initial page load`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.get(`table[id=script-resource-records] tbody`)
.children()
Expand All @@ -87,7 +87,7 @@ describe(`scripts with sources`, () => {
})

it(`should load only once after the page is refreshed`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.reload()

cy.get(`table[id=script-resource-records] tbody`)
Expand All @@ -101,7 +101,7 @@ describe(`scripts with sources`, () => {
})

it(`should load only once after anchor link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=anchor-link-back-to-index]`).click()
cy.url().should(`contain`, page.navigation)
cy.get(`a[href="${page.target}"][id=anchor-link]`).click()
Expand All @@ -117,7 +117,7 @@ describe(`scripts with sources`, () => {
})

it(`should load only once if the page is revisited via browser back/forward buttons after anchor link navigation`, () => {
cy.visit(page.navigation)
cy.visit(page.navigation).waitForRouteChange()
cy.get(`a[href="${page.target}"][id=anchor-link]`).click()
cy.go(`back`)
cy.go(`forward`)
Expand All @@ -133,7 +133,7 @@ describe(`scripts with sources`, () => {
})

it(`should load only once after Gatsby link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=gatsby-link-back-to-index]`).click()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()

Expand All @@ -148,7 +148,7 @@ describe(`scripts with sources`, () => {
})

it(`should load only once if the page is revisited via browser back/forward buttons after Gatsby link navigation`, () => {
cy.visit(page.navigation)
cy.visit(page.navigation).waitForRouteChange()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()
cy.go(`back`)
cy.go(`forward`)
Expand Down
4 changes: 2 additions & 2 deletions ...s/development-runtime/cypress/integration/gatsby-script/gatsby-script-ssr-browser-apis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import { script } from "../../../gatsby-script-scripts"
const page = `/gatsby-script-ssr-browser-apis/`

it(`scripts load successfully when used via wrapPageElement`, () => {
cy.visit(page)
cy.visit(page).waitForRouteChange()
cy.getRecord(script.three, `success`, true).should(`equal`, `true`)
cy.getRecord(script.marked, `success`, true).should(`equal`, `true`)
})

it(`scripts load successfully when used via wrapRootElement`, () => {
cy.visit(page)
cy.visit(page).waitForRouteChange()
cy.getRecord(script.jQuery, `success`, true).should(`equal`, `true`)
cy.getRecord(script.popper, `success`, true).should(`equal`, `true`)
})
40 changes: 40 additions & 0 deletions e2e-tests/development-runtime/src/pages/gatsby-script-duplicate-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import { onLoad, onError } from "../utils/gatsby-script-callbacks"
const DuplicateScripts = () => {
const [onLoadScriptLoaded, setOnLoadScriptLoaded] = useState(false)
const [onErrorScriptLoaded, setOnErrorScriptLoaded] = useState(false)
const [secondOnLoadScriptLoaded, setSecondOnLoadScriptLoaded] = useState(
false
)
const [secondOnErrorScriptLoaded, setSecondOnErrorScriptLoaded] = useState(
false
)

return (
<main>
Expand Down Expand Up @@ -54,6 +60,40 @@ const DuplicateScripts = () => {
}}
/>
)}

<Script src={scripts.three} />
<Script
src={scripts.three}
onLoad={() => {
onLoad(`duplicate-first-script-no-callback`)
setSecondOnLoadScriptLoaded(true)
}}
/>
{secondOnLoadScriptLoaded && (
<Script
src={scripts.three}
onLoad={() => {
onLoad(`duplicate-first-script-no-callback-2`)
}}
/>
)}

<Script src="/other-non-existent-script.js" />
<Script
src="/other-non-existent-script.js"
onError={() => {
onError(`duplicate-first-script-no-callback`)
setSecondOnErrorScriptLoaded(true)
}}
/>
{secondOnErrorScriptLoaded && (
<Script
src="/other-non-existent-script.js"
onError={() => {
onError(`duplicate-first-script-no-callback-2`)
}}
/>
)}
</main>
)
}
Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/production-runtime/cypress/integration/gatsby-script-duplicate-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Cypress.config(`defaultCommandTimeout`, 30000) // Since we're asserting network

describe(`duplicate scripts`, () => {
beforeEach(() => {
cy.visit(`/gatsby-script-duplicate-scripts/`)
cy.visit(`/gatsby-script-duplicate-scripts/`).waitForRouteChange()
})

it(`should execute load callbacks of duplicate scripts`, () => {
Expand Down
16 changes: 8 additions & 8 deletions e2e-tests/production-runtime/cypress/integration/gatsby-script-inline-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
describe(`inline scripts set via ${descriptor}`, () => {
describe(`using the post-hydrate strategy`, () => {
it(`should execute successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(
`post-hydrate-${inlineScriptType}`,
Expand All @@ -36,7 +36,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load after the framework bundle has loaded`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

// Assert framework is loaded before inline script is executed
cy.getRecord(
Expand All @@ -53,7 +53,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {

describe(`using the idle strategy`, () => {
it(`should execute successfully`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(`idle-${inlineScriptType}`, `success`, true).should(
`equal`,
Expand All @@ -62,7 +62,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load after other strategies`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.getRecord(`idle-${inlineScriptType}`, markRecord.executeStart).then(
dangerouslySetExecuteStart => {
Expand All @@ -77,7 +77,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {

describe(`when navigation occurs`, () => {
it(`should load only once on initial page load`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()

cy.get(`table[id=script-mark-records] tbody`)
.children()
Expand Down Expand Up @@ -112,7 +112,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once after anchor link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=anchor-link-back-to-index]`).click()
cy.url().should(`contain`, page.navigation)
cy.get(`a[href="${page.target}"][id=anchor-link]`).click()
Expand Down Expand Up @@ -153,7 +153,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once after Gatsby link navigation`, () => {
cy.visit(page.target)
cy.visit(page.target).waitForRouteChange()
cy.get(`a[id=gatsby-link-back-to-index]`).click()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()

Expand All @@ -172,7 +172,7 @@ for (const { descriptor, inlineScriptType } of typesOfInlineScripts) {
})

it(`should load only once if the page is revisited via browser back/forward buttons after Gatsby link navigation`, () => {
cy.visit(page.navigation)
cy.visit(page.navigation).waitForRouteChange()
cy.get(`a[href="${page.target}"][id=gatsby-link]`).click()
cy.go(`back`)
cy.go(`forward`)
Expand Down
Loading