[fizz] fix empty string href double warning (facebook#31783)

I think this is the suggested change from facebook#31765 (comment) But no tests fail and I'm not sure how to test it? Seems sus. Also seems like the `removeAttribute` here should be changed? https://github.com/facebook/react/blob/9d9f12f2699a049777fa88914306ad4de9e2b74d/packages/react-dom-bindings/src/client/ReactDOMComponent.js#L400-L427
HitenChawda46 · Jan 3, 2025 · bf883be · bf883be
1 parent f42f8c0
commit bf883be
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 29 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -2510,26 +2510,17 @@ function diffHydratedGenericElement(
               );
             }
           }
-          hydrateSanitizedAttribute(
-            domElement,
-            propKey,
-            propKey,
-            null,
-            extraAttributes,
-            serverDifferences,
-          );
-          continue;
-        } else {
-          hydrateSanitizedAttribute(
-            domElement,
-            propKey,
-            propKey,
-            value,
-            extraAttributes,
-            serverDifferences,
-          );
           continue;
         }
+        hydrateSanitizedAttribute(
+          domElement,
+          propKey,
+          propKey,
+          value,
+          extraAttributes,
+          serverDifferences,
+        );
+        continue;
       case 'action':
       case 'formAction': {
         const serverValue = domElement.getAttribute(propKey);

diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
@@ -1139,7 +1139,9 @@ export function canHydrateInstance(
           } else if (
             rel !== anyProps.rel ||
             element.getAttribute('href') !==
-              (anyProps.href == null ? null : anyProps.href) ||
+              (anyProps.href == null || anyProps.href === ''
+                ? null
+                : anyProps.href) ||
             element.getAttribute('crossorigin') !==
               (anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
             element.getAttribute('title') !==
@@ -2984,7 +2986,7 @@ export function hydrateHoistable(
           const node = nodes[i];
           if (
             node.getAttribute('href') !==
-              (props.href == null ? null : props.href) ||
+              (props.href == null || props.href === '' ? null : props.href) ||
             node.getAttribute('rel') !==
               (props.rel == null ? null : props.rel) ||
             node.getAttribute('title') !==

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
@@ -62,18 +62,23 @@ describe('ReactDOMServerIntegration', () => {
         expect(e.getAttribute('href')).toBe('');
       });
 
-      itRenders('empty href on other tags', async render => {
+      itRenders('empty href on base tags as null', async render => {
+        const e = await render(<base href="" />, 1);
+        expect(e.getAttribute('href')).toBe(null);
+      });
+
+      itRenders('empty href on area tags as null', async render => {
         const e = await render(
-          // <link href="" /> would be more sensible.
-          // However, that results in a hydration warning as well.
-          // Our test helpers do not support different error counts for initial
-          // server render and hydration.
-          // The number of errors on the server need to be equal to the number of
-          // errors during hydration.
-          // So we use a <div> instead.
-          <div href="" />,
+          <map>
+            <area alt="" href="" />
+          </map>,
           1,
         );
+        expect(e.firstChild.getAttribute('href')).toBe(null);
+      });
+
+      itRenders('empty href on link tags as null', async render => {
+        const e = await render(<link rel="stylesheet" href="" />, 1);
         expect(e.getAttribute('href')).toBe(null);
       });