next/script fix duplicate scripts (#28428)

* Fix #27747 * Fix lint error * Add data attribute to script component * Fix #28036 * Fix tests * Fix tests Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Aug 24, 2021 · fd1c56e · fd1c56e
1 parent ee46244
commit fd1c56e
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 8 deletions.
diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx
@@ -156,13 +156,17 @@ window.__NEXT_P = []
 const headManager: {
   mountedInstances: Set<unknown>
   updateHead: (head: JSX.Element[]) => void
+  getIsSsr?: () => boolean
 } = initHeadManager()
 const appElement: HTMLElement | null = document.getElementById('__next')
 
 let lastRenderReject: (() => void) | null
 let webpackHMR: any
 export let router: Router
 let CachedApp: AppComponent, onPerfEntry: (metric: any) => void
+headManager.getIsSsr = () => {
+  return router.isSsr
+}
 
 class Container extends React.Component<{
   fn: (err: Error, info?: any) => void

diff --git a/packages/next/client/script.tsx b/packages/next/client/script.tsx
@@ -140,7 +140,7 @@ function Script(props: ScriptProps): JSX.Element | null {
   } = props
 
   // Context is available only during SSR
-  const { updateScripts, scripts } = useContext(HeadManagerContext)
+  const { updateScripts, scripts, getIsSsr } = useContext(HeadManagerContext)
 
   useEffect(() => {
     if (strategy === 'afterInteractive') {
@@ -161,7 +161,10 @@ function Script(props: ScriptProps): JSX.Element | null {
         },
       ])
       updateScripts(scripts)
-    } else {
+    } else if (getIsSsr && getIsSsr()) {
+      // Script has already loaded during SSR
+      LoadCache.add(restProps.id || src)
+    } else if (getIsSsr && !getIsSsr()) {
       loadScript(props)
     }
   }

diff --git a/packages/next/shared/lib/head-manager-context.ts b/packages/next/shared/lib/head-manager-context.ts
@@ -5,6 +5,7 @@ export const HeadManagerContext: React.Context<{
   mountedInstances?: any
   updateScripts?: (state: any) => void
   scripts?: any
+  getIsSsr?: () => boolean
 }> = React.createContext({})
 
 if (process.env.NODE_ENV !== 'production') {

diff --git a/test/integration/script-loader/pages/page3.js b/test/integration/script-loader/pages/page3.js
@@ -4,11 +4,10 @@ const Page = () => {
   return (
     <div class="container">
       <Script id="inline-script">
-        {`(window.onload = function () {
-            const newDiv = document.createElement('div')
-            newDiv.id = 'onload-div'
-            document.querySelector('body').appendChild(newDiv)
-          })`}
+        {`const newDiv = document.createElement('div')
+          newDiv.id = 'onload-div'
+          document.querySelector('body').appendChild(newDiv)
+        `}
       </Script>
       <Script
         id="scriptLazyOnload"

diff --git a/test/integration/script-loader/test/index.test.js b/test/integration/script-loader/test/index.test.js
@@ -128,14 +128,25 @@ describe('Script Loader', () => {
     try {
       browser = await webdriver(appPort, '/')
 
+      // beforeInteractive scripts should load once
+      let documentBIScripts = await browser.elementsByCss(
+        '[src$="documentBeforeInteractive"]'
+      )
+      expect(documentBIScripts.length).toBe(1)
+
       await browser.waitForElementByCss('[href="/page1"]')
       await browser.click('[href="/page1"]')
 
       await browser.waitForElementByCss('.container')
-      await waitFor(1000)
 
       const script = await browser.elementById('scriptBeforeInteractive')
 
+      // Ensure beforeInteractive script isn't duplicated on navigation
+      documentBIScripts = await browser.elementsByCss(
+        '[src$="documentBeforeInteractive"]'
+      )
+      expect(documentBIScripts.length).toBe(1)
+
       // Renders script tag
       expect(script).toBeDefined()
     } finally {