@samitouri / QOS-React-2 / commits / bf883bebbc

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

I think this is the suggested change from https://github.com/facebook/react/pull/31765#discussion_r1884541447 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

Ricky committed Jan 3, 2025 at 12:53 UTC bf883bebbc4973dea0e4801a5a62f82043ff57ee
3 files changed +27 -29
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+9 -18
@@ -2510,26 +2510,17 @@ function diffHydratedGenericElement(
2510 );
2511 }
2512 }
2513 - hydrateSanitizedAttribute(
2514 - domElement,
2515 - propKey,
2516 - propKey,
2517 - null,
2518 - extraAttributes,
2519 - serverDifferences,
2520 - );
2521 - continue;
2522 - } else {
2523 - hydrateSanitizedAttribute(
2524 - domElement,
2525 - propKey,
2526 - propKey,
2527 - value,
2528 - extraAttributes,
2529 - serverDifferences,
2530 - );
2513 continue;
2514 }
2515 + hydrateSanitizedAttribute(
2516 + domElement,
2517 + propKey,
2518 + propKey,
2519 + value,
2520 + extraAttributes,
2521 + serverDifferences,
2522 + );
2523 + continue;
2524 case 'action':
2525 case 'formAction': {
2526 const serverValue = domElement.getAttribute(propKey);
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+4 -2
@@ -1139,7 +1139,9 @@ export function canHydrateInstance(
1139 } else if (
1140 rel !== anyProps.rel ||
1141 element.getAttribute('href') !==
1142 - (anyProps.href == null ? null : anyProps.href) ||
1142 + (anyProps.href == null || anyProps.href === ''
1143 + ? null
1144 + : anyProps.href) ||
1145 element.getAttribute('crossorigin') !==
1146 (anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
1147 element.getAttribute('title') !==
@@ -2984,7 +2986,7 @@ export function hydrateHoistable(
2986 const node = nodes[i];
2987 if (
2988 node.getAttribute('href') !==
2987 - (props.href == null ? null : props.href) ||
2989 + (props.href == null || props.href === '' ? null : props.href) ||
2990 node.getAttribute('rel') !==
2991 (props.rel == null ? null : props.rel) ||
2992 node.getAttribute('title') !==
packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
+14 -9
@@ -62,18 +62,23 @@ describe('ReactDOMServerIntegration', () => {
62 expect(e.getAttribute('href')).toBe('');
63 });
64
65 - itRenders('empty href on other tags', async render => {
65 + itRenders('empty href on base tags as null', async render => {
66 + const e = await render(<base href="" />, 1);
67 + expect(e.getAttribute('href')).toBe(null);
68 + });
69 +
70 + itRenders('empty href on area tags as null', async render => {
71 const e = await render(
67 - // <link href="" /> would be more sensible.
68 - // However, that results in a hydration warning as well.
69 - // Our test helpers do not support different error counts for initial
70 - // server render and hydration.
71 - // The number of errors on the server need to be equal to the number of
72 - // errors during hydration.
73 - // So we use a <div> instead.
74 - <div href="" />,
72 + <map>
73 + <area alt="" href="" />
74 + </map>,
75 1,
76 );
77 + expect(e.firstChild.getAttribute('href')).toBe(null);
78 + });
79 +
80 + itRenders('empty href on link tags as null', async render => {
81 + const e = await render(<link rel="stylesheet" href="" />, 1);
82 expect(e.getAttribute('href')).toBe(null);
83 });
84