next/script: Correctly apply async and defer props

packages/next/src/client/head-manager.ts

@@ -1,30 +1,8 @@
-export const DOMAttributeNames: Record<string, string> = {
-  acceptCharset: 'accept-charset',
-  className: 'class',
-  htmlFor: 'for',
-  httpEquiv: 'http-equiv',
-  noModule: 'noModule',
-}
+import { setAttributesFromProps } from './set-attributes-from-props'
 
 function reactElementToDOM({ type, props }: JSX.Element): HTMLElement {
   const el: HTMLElement = document.createElement(type)
-  for (const p in props) {
-    if (!props.hasOwnProperty(p)) continue
-    if (p === 'children' || p === 'dangerouslySetInnerHTML') continue
-
-    // we don't render undefined props to the DOM
-    if (props[p] === undefined) continue
-
-    const attr = DOMAttributeNames[p] || p.toLowerCase()
-    if (
-      type === 'script' &&
-      (attr === 'async' || attr === 'defer' || attr === 'noModule')
-    ) {
-      ;(el as HTMLScriptElement)[attr] = !!props[p]
-    } else {
-      el.setAttribute(attr, props[p])
-    }
-  }
+  setAttributesFromProps(el, props)
 
   const { children, dangerouslySetInnerHTML } = props
   if (dangerouslySetInnerHTML) {

packages/next/src/client/script.tsx

@@ -4,7 +4,7 @@ import ReactDOM from 'react-dom'
 import React, { useEffect, useContext, useRef } from 'react'
 import type { ScriptHTMLAttributes } from 'react'
 import { HeadManagerContext } from '../shared/lib/head-manager-context.shared-runtime'
-import { DOMAttributeNames } from './head-manager'
+import { setAttributesFromProps } from './set-attributes-from-props'
 import { requestIdleCallback } from './request-idle-callback'
 
 const ScriptCache = new Map()
@@ -25,16 +25,6 @@ export interface ScriptProps extends ScriptHTMLAttributes<HTMLScriptElement> {
  */
 export type Props = ScriptProps
 
-const ignoreProps = [
-  'onLoad',
-  'onReady',
-  'dangerouslySetInnerHTML',
-  'children',
-  'onError',
-  'strategy',
-  'stylesheets',
-]
-
 const insertStylesheets = (stylesheets: string[]) => {
   // Case 1: Styles for afterInteractive/lazyOnload with appDir injected via handleClientScriptLoad
   //
@@ -148,14 +138,7 @@ const loadScript = (props: ScriptProps): void => {
     ScriptCache.set(src, loadPromise)
   }
 
-  for (const [k, value] of Object.entries(props)) {
-    if (value === undefined || ignoreProps.includes(k)) {
-      continue
-    }
-
-    const attr = DOMAttributeNames[k] || k.toLowerCase()
-    el.setAttribute(attr, value)
-  }
+  setAttributesFromProps(el, props)
 
   if (strategy === 'worker') {
     el.setAttribute('type', 'text/partytown')

packages/next/src/client/set-attributes-from-props.ts

@@ -0,0 +1,59 @@
+const DOMAttributeNames: Record<string, string> = {
+  acceptCharset: 'accept-charset',
+  className: 'class',
+  htmlFor: 'for',
+  httpEquiv: 'http-equiv',
+  noModule: 'noModule',
+}
+
+const ignoreProps = [
+  'onLoad',
+  'onReady',
+  'dangerouslySetInnerHTML',
+  'children',
+  'onError',
+  'strategy',
+  'stylesheets',
+]
+
+function isBooleanScriptAttribute(
+  attr: string
+): attr is 'async' | 'defer' | 'noModule' {
+  return ['async', 'defer', 'noModule'].includes(attr)
+}
+
+export function setAttributesFromProps(el: HTMLElement, props: object) {
+  for (const [p, value] of Object.entries(props)) {
+    if (!props.hasOwnProperty(p)) continue
+    if (ignoreProps.includes(p)) continue
+
+    // we don't render undefined props to the DOM
+    if (value === undefined) {
+      continue
+    }
+
+    const attr = DOMAttributeNames[p] || p.toLowerCase()
+
+    if (el.tagName === 'SCRIPT' && isBooleanScriptAttribute(attr)) {
+      // Correctly assign boolean script attributes
+      // https://github.com/vercel/next.js/pull/20748
+      ;(el as HTMLScriptElement)[attr] = !!value
+    } else {
+      el.setAttribute(attr, String(value))
+    }
+
+    // Remove falsy non-zero boolean attributes so they are correctly interpreted
+    // (e.g. if we set them to false, this coerces to the string "false", which the browser interprets as true)
+    if (
+      value === false ||
+      (el.tagName === 'SCRIPT' &&
+        isBooleanScriptAttribute(attr) &&
+        (!value || value === 'false'))
+    ) {
+      // Call setAttribute before, as we need to set and unset the attribute to override force async:
+      // https://html.spec.whatwg.org/multipage/scripting.html#script-force-async
+      el.setAttribute(attr, '')
+      el.removeAttribute(attr)
+    }
+  }
+}

test/development/pages-dir/client-navigation/fixture/pages/head.js

@@ -116,7 +116,9 @@ export default () => (
       <link rel="stylesheet" href="dedupe-style.css" key="my-style" />
 
       {/* this should not execute twice on the client */}
-      <script src="/test-async.js" async></script>
+      <script src="/test-async-true.js" async></script>
+      {/* this should have async set to false on the client */}
+      <script src="/test-async-false.js" async={false}></script>
       {/* this should not execute twice on the client (intentionally sets defer to `yas` to test boolean coercion) */}
       <script src="/test-defer.js" defer="yas"></script>
 

test/development/pages-dir/client-navigation/fixture/pages/script.js

@@ -0,0 +1,11 @@
+import React from 'react'
+import Script from 'next/script'
+
+export default () => (
+  <div>
+    <h1>I am a page to test next/script</h1>
+    <Script src="/test-async-true.js" async />
+    <Script src="/test-async-false.js" async={false} />
+    <Script src="/test-defer.js" defer />
+  </div>
+)

test/development/pages-dir/client-navigation/index.test.ts

@@ -1773,6 +1773,54 @@ createNextDescribe(
       expect(value).toBe(false)
     })
 
+    it.each([true, false])(
+      'should handle boolean async prop in next/head client-side: %s',
+      async (bool) => {
+        const browser = await webdriver(next.appPort, '/head')
+        const value = await browser.eval(
+          `document.querySelector('script[src="/test-async-${JSON.stringify(
+            bool
+          )}.js"]').async`
+        )
+
+        expect(value).toBe(bool)
+      }
+    )
+
+    it.each([true, false])(
+      'should handle boolean async prop in next/script client-side: %s',
+      async (bool) => {
+        const browser = await webdriver(next.appPort, '/script')
+        const value = await browser.eval(
+          `document.querySelector('script[src="/test-async-${JSON.stringify(
+            bool
+          )}.js"]').async`
+        )
+
+        expect(value).toBe(bool)
+      }
+    )
+
+    it('should only execute async and defer scripts with next/script once', async () => {
+      let browser
+      try {
+        browser = await webdriver(next.appPort, '/script')
+
+        await browser.waitForElementByCss('h1')
+        await waitFor(2000)
+        expect(
+          Number(await browser.eval('window.__test_async_executions'))
+        ).toBe(1)
+        expect(
+          Number(await browser.eval('window.__test_defer_executions'))
+        ).toBe(1)
+      } finally {
+        if (browser) {
+          await browser.close()
+        }
+      }
+    })
+
     it('should emit routeChangeError on hash change cancel', async () => {
       const browser = await webdriver(next.appPort, '/')
 

test/development/pages-dir/client-navigation/rendering.ts

@@ -187,7 +187,8 @@ export default function (next: NextInstance, render, fetch, ctx) {
 
     test('header helper renders boolean attributes correctly children', async () => {
       const html = await render('/head')
-      expect(html).toContain('<script src="/test-async.js" async="">')
+      expect(html).toContain('<script src="/test-async-true.js" async="">')
+      expect(html).toContain('<script src="/test-async-false.js">')
       expect(html).toContain('<script src="/test-defer.js" defer="">')
     })
 

tsec-exemptions.json

@@ -8,6 +8,7 @@
   "ban-element-setattribute": [
     "packages/next/src/client/head-manager.ts",
     "packages/next/src/client/script.tsx",
+    "packages/next/src/client/set-attributes-from-props.ts",
     "packages/next/src/build/webpack/loaders/next-style-loader/runtime/injectStylesIntoLinkTag.ts",
     "packages/next/src/build/webpack/loaders/next-style-loader/runtime/injectStylesIntoStyleTag.ts",
     "packages/next/src/client/app-bootstrap.ts"