[fizz] fix empty string href double warning

[rickhanlonii]

I think this is the suggested change from #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?

react/packages/react-dom-bindings/src/client/ReactDOMComponent.js

Lines 400 to 427 in 9d9f12f

	if (
	value === '' &&
	// <a href=""> is fine for "reload" links.
	!(tag === 'a' && key === 'href')
	) {
	if (__DEV__) {
	if (key === 'src') {
	console.error(
	'An empty string ("") was passed to the %s attribute. ' +
	'This may cause the browser to download the whole page again over the network. ' +
	'To fix this, either do not render the element at all ' +
	'or pass null to %s instead of an empty string.',
	key,
	key,
	);
	} else {
	console.error(
	'An empty string ("") was passed to the %s attribute. ' +
	'To fix this, either do not render the element at all ' +
	'or pass null to %s instead of an empty string.',
	key,
	key,
	);
	}
	}
	domElement.removeAttribute(key);
	break;
	}

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 15, 2024 8:30pm

[react-sizebot]

Comparing: e06c72f...3e7761c

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.68 kB	6.68 kB	+0.05%	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	+0.03%	510.76 kB	510.90 kB	+0.02%	91.36 kB	91.38 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	+0.03%	515.55 kB	515.69 kB	+0.02%	92.17 kB	92.19 kB
facebook-www/ReactDOM-prod.classic.js	+0.02%	594.69 kB	594.83 kB	+0.01%	104.93 kB	104.94 kB
facebook-www/ReactDOM-prod.modern.js	+0.02%	584.96 kB	585.10 kB	+0.02%	103.37 kB	103.39 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 6052c9a

[sebmarkbage]

It's probably expressible with the server integration tests somehow. But you can test it in /fixture/ssr which actually has this error already.

[rickhanlonii]

@sebmarkbage see my comment in the description about the removeAttribute in setProp

[sebmarkbage]

Also seems like the removeAttribute here should be changed?

No I think that's correct because the server also omits the attribute.

[rickhanlonii]

@sebmarkbage I repro'd in the fixture and this doesn't fix it (or I'm missing a feature flag or something).

What happens today (same on main and this PR, I added logs, and don't think this branch is actually getting hit):

I'm confused what the behavior you're expecting is? The server and the client both strip the attribute, but the prop is still empty string. So canHydrateInstance is going to return null if they mismatch because of the check here:

react/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines 1141 to 1142 in ef63718

	element.getAttribute('href') !==
	(anyProps.href == null ? null : anyProps.href) ||

I'll push a commit that fixes this so you can see the difference.

[rickhanlonii]

Ok latest commit fixes it so that empty string hydrates as null, with tests, and this is the result: