feat(gatsby-script): On load callback, cache (#35464)

* feat(gatsy-script): Initial on load callback implementation * test(gatsby-script): Refactor e2e test structure * refactor(gatsby-script): Fix eslint, ts errors * feat(gatsby-script): Handle edge cases from Gatsby link navigation * test(gatsby-script): Adjust e2e tests so they run successfully again * feat(gatsby-script): On load callback e2e tests * test(gatsby-script): Scripts with sources navigation e2e tests * test(gatsby-script): Inline scripts navigation e2e tests * test(gatsby-script): Adjust e2e tests, add comment on edge case * test(gatsby-script): Fix unit tests
gatsbyjs · May 9, 2022 · baa1b6e · baa1b6e
1 parent fef9d3b
commit baa1b6e
Show file tree

Hide file tree

Showing 13 changed files with 723 additions and 300 deletions.
diff --git a/e2e-tests/gatsby-script/cypress/integration/inline-scripts.ts b/e2e-tests/gatsby-script/cypress/integration/inline-scripts.ts
diff --git a/e2e-tests/gatsby-script/cypress/integration/scripts-with-sources.ts b/e2e-tests/gatsby-script/cypress/integration/scripts-with-sources.ts
@@ -4,6 +4,9 @@ import { ResourceRecord } from "../../records"
 // TODO - Import from gatsby core after gatsby-script is in general availability
 import { ScriptStrategy } from "gatsby-script"
 
+// The page that we will assert against
+const page = `/scripts-with-sources`
+
 beforeEach(() => {
   // @ts-ignore Object.values does exist, Cypress wants ES5 in tsconfig
   for (const script of [...Object.values(scripts), new RegExp(`framework`)]) {
@@ -18,12 +21,12 @@ beforeEach(() => {
 describe(`scripts with sources`, () => {
   describe(`using the ${ScriptStrategy.preHydrate} strategy`, () => {
     it(`should load successfully`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
       cy.getRecord(Script.dayjs, `success`, true).should(`equal`, `true`)
     })
 
     it(`should load before other strategies`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
 
       cy.getRecord(Script.dayjs, ResourceRecord.fetchStart).then(
         dayjsFetchStart => {
@@ -39,16 +42,24 @@ describe(`scripts with sources`, () => {
         }
       )
     })
+
+    it(`should call an on load callback once the script has loaded`, () => {
+      cy.visit(page)
+
+      cy.getRecord(Script.dayjs, ResourceRecord.responseEnd).then(() => {
+        cy.get(`[data-on-load-result=${ScriptStrategy.preHydrate}]`)
+      })
+    })
   })
 
   describe(`using the ${ScriptStrategy.postHydrate} strategy`, () => {
     it(`should load successfully`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
       cy.getRecord(Script.three, `success`, true).should(`equal`, `true`)
     })
 
     it(`should load after the framework bundle has loaded`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
 
       // Assert framework is loaded before three starts loading
       cy.getRecord(Script.three, ResourceRecord.fetchStart).then(
@@ -60,16 +71,23 @@ describe(`scripts with sources`, () => {
         }
       )
     })
+
+    it(`should call an on load callback once the script has loaded`, () => {
+      cy.visit(page)
+      cy.getRecord(Script.three, ResourceRecord.responseEnd).then(() => {
+        cy.get(`[data-on-load-result=${ScriptStrategy.postHydrate}]`)
+      })
+    })
   })
 
   describe(`using the ${ScriptStrategy.idle} strategy`, () => {
     it(`should load successfully`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
       cy.getRecord(Script.marked, `success`, true).should(`equal`, `true`)
     })
 
     it(`should load after other strategies`, () => {
-      cy.visit(`/`)
+      cy.visit(page)
 
       cy.getRecord(Script.marked, ResourceRecord.fetchStart).then(
         markedFetchStart => {
@@ -85,5 +103,146 @@ describe(`scripts with sources`, () => {
         }
       )
     })
+
+    it(`should call an on load callback once the script has loaded`, () => {
+      cy.visit(page)
+      cy.getRecord(Script.marked, ResourceRecord.responseEnd).then(() => {
+        cy.get(`[data-on-load-result=${ScriptStrategy.idle}]`)
+      })
+    })
+  })
+
+  describe(`when navigation occurs`, () => {
+    it(`should load only once on initial page load`, () => {
+      cy.visit(page)
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
+
+    it(`should load only once after the page is refreshed`, () => {
+      cy.visit(page)
+      cy.reload()
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
+
+    it(`should load only once after anchor link navigation`, () => {
+      cy.visit(page)
+      cy.get(`a[id=anchor-link-back-to-index]`).click()
+      cy.get(`a[href="${page}"][id=anchor-link]`).click()
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
+
+    it(`should load only once if the page is revisited via browser back/forward buttons after anchor link navigation`, () => {
+      cy.visit(`/`)
+      cy.get(`a[href="${page}"][id=anchor-link]`).click()
+      cy.go(`back`)
+      cy.go(`forward`)
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
+
+    it(`should load only once after Gatsby link navigation`, () => {
+      cy.visit(page)
+      cy.get(`a[id=gatsby-link-back-to-index]`).click()
+      cy.get(`a[href="${page}"][id=gatsby-link]`).click()
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
+
+    // TODO - Fix
+    it.skip(`should load only once if the page is revisited via browser back/forward buttons after Gatsby link navigation`, () => {
+      cy.visit(`/`)
+      cy.get(`a[href="${page}"][id=gatsby-link]`).click()
+      cy.go(`back`)
+      cy.go(`forward`)
+
+      cy.get(`table[id=script-resource-records] tbody`)
+        .children()
+        .should(`have.length`, 4)
+      cy.getRecord(Script.dayjs, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.preHydrate
+      )
+      cy.getRecord(Script.three, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.postHydrate
+      )
+      cy.getRecord(Script.marked, `strategy`, true).should(
+        `equal`,
+        ScriptStrategy.idle
+      )
+    })
   })
 })
diff --git a/e2e-tests/gatsby-script/cypress/support/index.ts b/e2e-tests/gatsby-script/cypress/support/index.ts
@@ -13,7 +13,6 @@ declare global {
         metric: string,
         raw?: boolean
       ): Chainable<number | string>
-      waitForRouteChange(): unknown
     }
   }
 }

diff --git a/e2e-tests/gatsby-script/scripts.ts b/e2e-tests/gatsby-script/scripts.ts
@@ -76,13 +76,12 @@ export const inlineScripts = {
 }
 
 function constructInlineScript(type: string, strategy: ScriptStrategy): string {
-  return `(function() {
+  return `
     performance.mark(\`inline-script\`, { detail: {
       strategy: \`${strategy}\`,
       type: \`${type}\`,
       executeStart: performance.now()
     }})
     window[\`${strategy}-${type}\`] = true;
-  })();
   `
 }
diff --git a/e2e-tests/gatsby-script/src/components/script-mark-records.tsx b/e2e-tests/gatsby-script/src/components/script-mark-records.tsx
@@ -12,57 +12,51 @@ export function ScriptMarkRecords(): JSX.Element {
    * Poll for the mark records we care about.
    * We'll use this approach instead of listening for the load event to be consistent.
    */
-  function getMarkRecords(retries: number = 0): void {
-    const markRecords = performance.getEntriesByType(
-      `mark`
-    ) as Array<PerformanceMarkWithDetails>
-
-    const scriptRecords = markRecords.filter(
-      record => record.name === `inline-script`
-    )
-
-    if (scriptRecords.length !== 6 && retries < 10) {
-      setTimeout(() => {
-        getMarkRecords(retries + 1)
-      }, 100)
-    }
+  useEffect(() => {
+    const interval = setInterval(() => {
+      const markRecords = performance.getEntriesByType(
+        `mark`
+      ) as Array<PerformanceMarkWithDetails>
 
-    setRecords(scriptRecords)
-  }
+      const scriptRecords = markRecords.filter(
+        markRecord => markRecord.name === `inline-script`
+      )
 
-  useEffect(() => {
-    getMarkRecords()
+      if (scriptRecords.length === 6 || performance.now() > 10000) {
+        setRecords(scriptRecords)
+        clearInterval(interval)
+      }
+    }, 100)
   }, [])
 
   return (
-    <>
-      <table>
-        <thead>
-          <tr>
-            <th>Type</th>
-            <th>Strategy</th>
-            <th>Success</th>
-            <th>Execute start (ms)</th>
-          </tr>
-        </thead>
-        <tbody>
-          {records.map(record => {
+    <table id="script-mark-records">
+      <thead>
+        <tr>
+          <th>Type</th>
+          <th>Strategy</th>
+          <th>Success</th>
+          <th>Execute start (ms)</th>
+        </tr>
+      </thead>
+      <tbody>
+        {records
+          .sort((a, b) => a.detail.executeStart - b.detail.executeStart)
+          .map(record => {
             const { strategy, type, executeStart } = record.detail
             const key = `${strategy}-${type}`
-            // @ts-ignore
+            // @ts-ignore Do not complain about key not being a number
             const success = `${typeof window[key] === `boolean`}`
             return (
               <tr id={key} key={key}>
                 <td id="type">{type}</td>
                 <td id="strategy">{strategy}</td>
-                {/* @ts-ignore */}
                 <td id="success">{success}</td>
                 <td id={MarkRecord.executeStart}>{trim(executeStart)}</td>
               </tr>
             )
           })}
-        </tbody>
-      </table>
-    </>
+      </tbody>
+    </table>
   )
 }