fix(#39993): avoid race condition for next/script onReady (#40002)

Fixes #39993.

Before the PR:

- `next/script` component mount, `useEffect` for `onReady` executes
- The script's cacheKey is not added to `LoadCache`, skip `onReady`
- The second `useEffect` for `loadScript` executes
- The script's cacheKey is added to `LoadCache` even if it might not fully load yet
- Because of React's strict mode, `useEffect` for `onReady` executes again
- Since the script's cacheKey is in `LoadCache`, `onReady` is called (even when the script is not loaded yet)
- After the script is actually loaded, inside the `script.onload` event handler the `onReady` is called again

After the PR:

- `next/script` component mount, `useEffect` for `onReady` executes
- The script's cacheKey is not added to `LoadCache`, `useEffect` skips `onReady`
- The second `useEffect` for `loadScript` executes
- The script's cacheKey is added to `LoadCache` only if it is an inline script
- Because of React's strict mode, `useEffect` for `onReady` executes again
- The script is not yet loaded, its cacheKey is not in `LoadCache`, `useEffect` skips `onReady` again
- After the script is actually loaded, inside the `script.onload` event handler the `onReady` is finally called

In short, the PR resolves a race condition that only occurs under React strict mode (and makes the `next/script` component more concurrent rendering resilient).

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

packages/next/client/script.tsx

@@ -61,6 +61,9 @@ const loadScript = (props: ScriptProps): void => {
 
   const loadPromise = new Promise<void>((resolve, reject) => {
     el.addEventListener('load', function (e) {
+      // add cacheKey to LoadCache when load successfully
+      LoadCache.add(cacheKey)
+
       resolve()
       if (onLoad) {
         onLoad.call(this, e)
@@ -79,22 +82,27 @@ const loadScript = (props: ScriptProps): void => {
     }
   })
 
-  if (src) {
-    ScriptCache.set(src, loadPromise)
-  }
-  LoadCache.add(cacheKey)
-
   if (dangerouslySetInnerHTML) {
     el.innerHTML = dangerouslySetInnerHTML.__html || ''
+
+    // add cacheKey to LoadCache for inline script
+    LoadCache.add(cacheKey)
   } else if (children) {
     el.textContent =
       typeof children === 'string'
         ? children
         : Array.isArray(children)
         ? children.join('')
         : ''
+
+    // add cacheKey to LoadCache for inline script
+    LoadCache.add(cacheKey)
   } else if (src) {
     el.src = src
+    // do not add cacheKey into LoadCache for remote script here
+    // cacheKey will be added to LoadCache when it is actually loaded (see loadPromise above)
+
+    ScriptCache.set(src, loadPromise)
   }
 
   for (const [k, value] of Object.entries(props)) {

test/integration/script-loader/base/pages/page10.js

@@ -0,0 +1,31 @@
+import Script from 'next/script'
+import Link from 'next/link'
+
+if (typeof window !== 'undefined') {
+  window.remoteScriptsOnReadyCalls ??= 0
+  window.inlineScriptsOnReadyCalls ??= 0
+}
+
+const Page = () => {
+  return (
+    <div className="container">
+      <Link href="/page9">Page 9</Link>
+      <div id="text"></div>
+      <Script
+        src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.1/jquery.min.js"
+        onReady={() => {
+          window.remoteScriptsOnReadyCalls++
+        }}
+      />
+      <Script
+        id="i-am-an-inline-script-that-has-on-ready"
+        dangerouslySetInnerHTML={{ __html: 'console.log("inline script!")' }}
+        onReady={() => {
+          window.inlineScriptsOnReadyCalls++
+        }}
+      />
+    </div>
+  )
+}
+
+export default Page

test/integration/script-loader/strictmode/next.config.js

@@ -0,0 +1,3 @@
+module.exports = {
+  reactStrictMode: true,
+}

test/integration/script-loader/strictmode/pages/onready.js

@@ -0,0 +1,31 @@
+import Script from 'next/script'
+import Link from 'next/link'
+
+if (typeof window !== 'undefined') {
+  window.remoteScriptsOnReadyCalls ??= 0
+  window.inlineScriptsOnReadyCalls ??= 0
+}
+
+const Page = () => {
+  return (
+    <div className="container">
+      <Link href="/page9">Page 9</Link>
+      <div id="text"></div>
+      <Script
+        src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.1/jquery.min.js"
+        onReady={() => {
+          window.remoteScriptsOnReadyCalls++
+        }}
+      />
+      <Script
+        id="i-am-an-inline-script-that-has-on-ready"
+        dangerouslySetInnerHTML={{ __html: 'console.log("inline script!")' }}
+        onReady={() => {
+          window.inlineScriptsOnReadyCalls++
+        }}
+      />
+    </div>
+  )
+}
+
+export default Page

test/integration/script-loader/test/index.test.js

@@ -8,12 +8,16 @@ import {
   stopApp,
   nextBuild,
   waitFor,
+  findPort,
+  launchApp,
+  killApp,
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 import cheerio from 'cheerio'
 
 let appDir = join(__dirname, '../base')
 let appWithPartytownMissingDir = join(__dirname, '../partytown-missing')
+let appWithStrictModeDir = join(__dirname, '../strictmode')
 let server
 let appPort
 
@@ -231,7 +235,6 @@ describe('Next.js Script - Primary Strategies', () => {
     let browser
     try {
       browser = await webdriver(appPort, '/page7')
-
       await waitFor(1000)
 
       const logs = await browser.log()
@@ -281,3 +284,25 @@ describe('Next.js Script - Primary Strategies', () => {
     )
   })
 })
+
+describe('Next.js Script - Strict Mode', () => {
+  let devAppPort
+  let devApp
+  // https://github.com/vercel/next.js/issues/39993
+  it('onReady should only fires once after load event in dev mode (issue #39993)', async () => {
+    let browser
+    try {
+      devAppPort = await findPort()
+      devApp = await launchApp(appWithStrictModeDir, devAppPort)
+      browser = await webdriver(devAppPort, '/onready')
+
+      // wait for jQuery to be loaded
+      await waitFor(1000)
+      expect(await browser.eval(`window.remoteScriptsOnReadyCalls`)).toBe(1)
+      expect(await browser.eval(`window.inlineScriptsOnReadyCalls`)).toBe(1)
+    } finally {
+      if (browser) await browser.close()
+      if (devApp) await killApp(devApp)
+    }
+  })
+})