fix(script): dedupe concurrent same-src loads (#1263)

* fix(script): dedupe concurrent same-src loads

Multiple next/script components with the same src could append duplicate DOM scripts when they mounted before the first script load event fired. That double-executed third-party code and diverged from Next.js' in-flight script cache behavior.

The shim now tracks remote scripts that are currently loading separately from scripts that have completed loading, and fans out load/error callbacks without adding another script element.

Adds a Pages Router browser regression that verifies simultaneous same-src scripts produce one DOM script and one execution.

* fix: address review feedback for script load dedupe

- Defer loadedScripts.add(key) to after in-flight promise resolves
- Add onReady() callback for duplicate callers joining in-flight load
- Clean up loadingScripts entry on promise settlement to allow retry

Author: NathanDrake2406

packages/vinext/src/shims/script.tsx

@@ -49,8 +49,11 @@ export type ScriptProps = {
   [key: string]: unknown;
 };
 
-// Track scripts that have already been loaded to avoid duplicates
+// Track scripts that have already been loaded, plus remote scripts currently
+// loading, to avoid duplicate DOM insertion when same-src components mount
+// before the first load event fires.
 const loadedScripts = new Set<string>();
+const loadingScripts = new Map<string, Promise<Event>>();
 
 function getClientAutoNonce(): string | undefined {
   if (typeof document === "undefined") return undefined;
@@ -96,51 +99,118 @@ function buildBeforeInteractiveScriptProps(options: {
   return scriptProps;
 }
 
-/**
- * Load a script imperatively (outside of React).
- */
-export function handleClientScriptLoad(props: ScriptProps): void {
+function setScriptAttributes(el: HTMLScriptElement, rest: Record<string, unknown>): void {
+  for (const [attr, value] of Object.entries(rest)) {
+    if (attr === "dangerouslySetInnerHTML") continue;
+    if (attr === "className") {
+      el.setAttribute("class", String(value));
+    } else if (typeof value === "string") {
+      el.setAttribute(attr, value);
+    } else if (typeof value === "boolean" && value) {
+      el.setAttribute(attr, "");
+    }
+  }
+}
+
+function loadClientScript(
+  props: ScriptProps,
+  options: {
+    resolvedNonce?: string;
+    fireReadyWhenAlreadyLoaded: boolean;
+  },
+): void {
   const {
     src,
     id,
     onLoad,
+    onReady,
     onError,
-    strategy: _strategy,
-    onReady: _onReady,
+    strategy = "afterInteractive",
     children,
+    dangerouslySetInnerHTML,
     ...rest
   } = props;
   if (typeof window === "undefined") return;
 
   const key = id ?? src ?? "";
-  if (key && loadedScripts.has(key)) return;
+  if (key && loadedScripts.has(key)) {
+    if (options.fireReadyWhenAlreadyLoaded) {
+      onReady?.();
+    }
+    return;
+  }
+
+  if (src) {
+    const existingLoad = loadingScripts.get(src);
+    if (existingLoad) {
+      void existingLoad.then(
+        (event) => {
+          if (key) loadedScripts.add(key);
+          onLoad?.(event);
+          onReady?.();
+        },
+        (event) => onError?.(event),
+      );
+      return;
+    }
+  }
 
   const el = document.createElement("script");
   if (src) el.src = src;
   if (id) el.id = id;
-  const resolvedNonce = resolveScriptNonce(rest.nonce);
 
-  for (const [attr, value] of Object.entries(rest)) {
-    if (attr === "dangerouslySetInnerHTML" || attr === "className") continue;
-    if (typeof value === "string") {
-      el.setAttribute(attr, value);
-    } else if (typeof value === "boolean" && value) {
-      el.setAttribute(attr, "");
-    }
+  setScriptAttributes(el, rest);
+  if (options.resolvedNonce && !el.getAttribute("nonce")) {
+    el.setAttribute("nonce", options.resolvedNonce);
   }
-  if (resolvedNonce && !el.getAttribute("nonce")) {
-    el.setAttribute("nonce", resolvedNonce);
+
+  if (strategy === "worker") {
+    el.setAttribute("type", "text/partytown");
   }
 
-  if (children && typeof children === "string") {
+  const markLoaded = () => {
+    if (key) loadedScripts.add(key);
+    onReady?.();
+  };
+
+  if (dangerouslySetInnerHTML?.__html) {
+    // Intentional: mirrors the Next.js <Script> API where dangerouslySetInnerHTML
+    // is developer-supplied inline script content (not user input). The prop name
+    // itself signals developer awareness of the XSS risk, consistent with React's
+    // design. User-supplied data must never flow into this prop.
+    el.innerHTML = dangerouslySetInnerHTML.__html;
+    markLoaded();
+  } else if (children && typeof children === "string") {
     el.textContent = children;
+    markLoaded();
+  } else if (src) {
+    const loadPromise = new Promise<Event>((resolve, reject) => {
+      el.addEventListener("load", (event) => {
+        resolve(event);
+        if (key) loadedScripts.add(key);
+        onLoad?.(event);
+        onReady?.();
+      });
+      el.addEventListener("error", (event) => {
+        reject(event);
+        onError?.(event);
+      });
+    });
+    loadPromise.catch(() => undefined).finally(() => loadingScripts.delete(src));
+    loadingScripts.set(src, loadPromise);
   }
 
-  if (onLoad) el.addEventListener("load", onLoad);
-  if (onError) el.addEventListener("error", onError);
-
   document.body.appendChild(el);
-  if (key) loadedScripts.add(key);
+}
+
+/**
+ * Load a script imperatively (outside of React).
+ */
+export function handleClientScriptLoad(props: ScriptProps): void {
+  loadClientScript(props, {
+    resolvedNonce: resolveScriptNonce(props.nonce),
+    fireReadyWhenAlreadyLoaded: false,
+  });
 }
 
 /**
@@ -192,48 +262,20 @@ function Script(props: ScriptProps): React.ReactElement | null {
         return;
       }
 
-      const el = document.createElement("script");
-      if (src) el.src = src;
-      if (id) el.id = id;
-
-      for (const [attr, value] of Object.entries(rest)) {
-        if (attr === "className") {
-          el.setAttribute("class", String(value));
-        } else if (typeof value === "string") {
-          el.setAttribute(attr, value);
-        } else if (typeof value === "boolean" && value) {
-          el.setAttribute(attr, "");
-        }
-      }
-      if (resolvedNonce && !el.getAttribute("nonce")) {
-        el.setAttribute("nonce", resolvedNonce);
-      }
-
-      if (strategy === "worker") {
-        el.setAttribute("type", "text/partytown");
-      }
-
-      if (dangerouslySetInnerHTML?.__html) {
-        // Intentional: mirrors the Next.js <Script> API where dangerouslySetInnerHTML
-        // is developer-supplied inline script content (not user input). The prop name
-        // itself signals developer awareness of the XSS risk, consistent with React's
-        // design. User-supplied data must never flow into this prop.
-        el.innerHTML = dangerouslySetInnerHTML.__html as string;
-      } else if (children && typeof children === "string") {
-        el.textContent = children;
-      }
-
-      el.addEventListener("load", (e) => {
-        if (key) loadedScripts.add(key);
-        onLoad?.(e);
-        onReady?.();
-      });
-
-      if (onError) {
-        el.addEventListener("error", onError);
-      }
-
-      document.body.appendChild(el);
+      loadClientScript(
+        {
+          src,
+          id,
+          strategy,
+          onLoad,
+          onReady,
+          onError,
+          children,
+          dangerouslySetInnerHTML,
+          ...rest,
+        },
+        { resolvedNonce, fireReadyWhenAlreadyLoaded: true },
+      );
     };
 
     if (strategy === "lazyOnload") {

tests/e2e/pages-router/script.spec.ts

@@ -0,0 +1,19 @@
+import { test, expect } from "@playwright/test";
+
+const BASE = "http://localhost:4173";
+
+test.describe("next/script", () => {
+  // Ported from Next.js: packages/next/src/client/script.tsx
+  // https://github.com/vercel/next.js/blob/canary/packages/next/src/client/script.tsx
+  // Next.js keeps a ScriptCache for in-flight remote scripts so same-src
+  // components mounted together only append one DOM script.
+  test("deduplicates simultaneous same-src scripts before load completes", async ({ page }) => {
+    await page.goto(`${BASE}/script-dedupe`);
+    await expect(page.getByRole("heading", { name: "Script Dedupe" })).toBeVisible();
+
+    await expect.poll(() => page.locator('script[src="/dedupe-script.js"]').count()).toBe(1);
+    await expect
+      .poll(() => page.evaluate(() => Reflect.get(window, "__vinextScriptDedupeExecutions")))
+      .toBe(1);
+  });
+});

tests/fixtures/pages-basic/pages/script-dedupe.tsx

@@ -0,0 +1,11 @@
+import Script from "next/script";
+
+export default function ScriptDedupePage() {
+  return (
+    <main>
+      <h1>Script Dedupe</h1>
+      <Script src="/dedupe-script.js" />
+      <Script src="/dedupe-script.js" />
+    </main>
+  );
+}

tests/fixtures/pages-basic/public/dedupe-script.js

@@ -0,0 +1 @@
+window.__vinextScriptDedupeExecutions = (window.__vinextScriptDedupeExecutions || 0) + 1;