Boolean props should coerce consistently between HostHoistable and Ho…

…stComponent and Fizz equivalents
facebook · May 2, 2023 · fc393ad · fc393ad
1 parent 8ea96ef
commit fc393ad
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 10 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
@@ -1027,7 +1027,13 @@ export function isHydratableType(type: string, props: Props): boolean {
   if (enableFloat) {
     if (type === 'script') {
       const {async, onLoad, onError} = (props: any);
-      return !(async && (onLoad || onError));
+      return !(
+        !async ||
+        onLoad ||
+        onError ||
+        typeof async === 'function' ||
+        typeof async === 'symbol'
+      );
     }
     return true;
   } else {
@@ -2455,9 +2461,15 @@ export function getResource(
       return null;
     }
     case 'script': {
-      if (typeof pendingProps.src === 'string' && pendingProps.async === true) {
-        const scriptProps: ScriptProps = pendingProps;
-        const key = getScriptKey(scriptProps.src);
+      const async = pendingProps.async;
+      const src = pendingProps.src;
+      if (
+        typeof src === 'string' &&
+        async &&
+        typeof async !== 'function' &&
+        typeof async !== 'symbol'
+      ) {
+        const key = getScriptKey(src);
         const scripts = getResourcesFromRoot(resourceRoot).hoistableScripts;
 
         let resource = scripts.get(key);
@@ -3103,15 +3115,24 @@ export function isHostHoistableType(
     }
     case 'script': {
       if (
-        props.async !== true ||
+        // This is ordered to hopefully hit an exclusion case as early as possible
+        // Logically the async checks together form a "truthy and not function/symbol" which is
+        // how the prop is serialized on a regular HostComponent
+        !props.async ||
         props.onLoad ||
         props.onError ||
-        typeof props.src !== 'string' ||
-        !props.src
+        !props.src ||
+        typeof props.async === 'function' ||
+        typeof props.async === 'symbol' ||
+        typeof props.src !== 'string'
       ) {
         if (__DEV__) {
+          const isAsync =
+            props.async &&
+            typeof props.async !== 'function' &&
+            typeof props.async !== 'symbol';
           if (outsideHostContainerContext) {
-            if (props.async !== true) {
+            if (!isAsync) {
               console.error(
                 'Cannot render a sync or defer <script> outside the main document without knowing its order.' +
                   ' Try adding async="" or moving it into the root <head> tag.',

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -2575,7 +2575,13 @@ function pushScript(
 
     const src = props.src;
     const key = getResourceKey('script', src);
-    if (props.async !== true || props.onLoad || props.onError) {
+
+    const isAsync =
+      props.async &&
+      typeof props.async !== 'function' &&
+      typeof props.async !== 'symbol';
+
+    if (!isAsync || props.onLoad || props.onError) {
       // we don't want to preload nomodule scripts
       if (props.noModule !== true) {
         // We can't resourcify scripts with load listeners. To avoid ambiguity with
@@ -2600,7 +2606,7 @@ function pushScript(
         }
       }
 
-      if (props.async !== true) {
+      if (!isAsync) {
         // This is not an async script, we can preloaded it but it still needs to
         // be emitted in place since it needs to hydrate on the client
         pushScriptImpl(target, props);