Skip to content

Commit 1a5d710

Browse files
author
Sebastian Silbermann
committed
Restore old behavior for empty href props on anchor tags
e.g. `<a href="" />`
1 parent 9aef5d2 commit 1a5d710

File tree

5 files changed

+107
-4
lines changed

5 files changed

+107
-4
lines changed

fixtures/attribute-behavior/AttributeTableSnapshot.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5077,7 +5077,7 @@
50775077
| Test Case | Flags | Result |
50785078
| --- | --- | --- |
50795079
| `href=(string)`| (changed)| `"https://reactjs.com/"` |
5080-
| `href=(empty string)`| (initial, warning)| `<empty string>` |
5080+
| `href=(empty string)`| (changed)| `"http://localhost:3000/"` |
50815081
| `href=(array with string)`| (changed)| `"https://reactjs.com/"` |
50825082
| `href=(empty array)`| (changed)| `"http://localhost:3000/"` |
50835083
| `href=(object)`| (changed)| `"http://localhost:3000/result%20of%20toString()"` |

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,11 @@ function setProp(
434434
case 'src':
435435
case 'href': {
436436
if (enableFilterEmptyStringAttributesDOM) {
437-
if (value === '') {
437+
if (
438+
value === '' &&
439+
// <a href=""> is fine for "reload" links.
440+
tag !== 'a'
441+
) {
438442
if (__DEV__) {
439443
if (key === 'src') {
440444
console.error(
@@ -2350,7 +2354,11 @@ function diffHydratedGenericElement(
23502354
case 'src':
23512355
case 'href':
23522356
if (enableFilterEmptyStringAttributesDOM) {
2353-
if (value === '') {
2357+
if (
2358+
value === '' &&
2359+
// <a href=""> is fine for "reload" links.
2360+
(tag !== 'a' || propKey !== 'href')
2361+
) {
23542362
if (__DEV__) {
23552363
if (propKey === 'src') {
23562364
console.error(

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1508,6 +1508,54 @@ function checkSelectProp(props: any, propName: string) {
15081508
}
15091509
}
15101510

1511+
function pushStartAnchor(
1512+
target: Array<Chunk | PrecomputedChunk>,
1513+
props: Object,
1514+
): ReactNodeList {
1515+
target.push(startChunkForTag('a'));
1516+
1517+
let children = null;
1518+
let innerHTML = null;
1519+
for (const propKey in props) {
1520+
if (hasOwnProperty.call(props, propKey)) {
1521+
const propValue = props[propKey];
1522+
if (propValue == null) {
1523+
continue;
1524+
}
1525+
switch (propKey) {
1526+
case 'children':
1527+
children = propValue;
1528+
break;
1529+
case 'dangerouslySetInnerHTML':
1530+
innerHTML = propValue;
1531+
break;
1532+
case 'href':
1533+
if (propValue === '') {
1534+
// Empty `href` is special on anchors so we're short-circuiting here.
1535+
// On other tags it should trigger a warning
1536+
pushStringAttribute(target, 'href', '');
1537+
} else {
1538+
pushAttribute(target, propKey, propValue);
1539+
}
1540+
break;
1541+
default:
1542+
pushAttribute(target, propKey, propValue);
1543+
break;
1544+
}
1545+
}
1546+
}
1547+
1548+
target.push(endOfStartTag);
1549+
pushInnerHTML(target, innerHTML, children);
1550+
if (typeof children === 'string') {
1551+
// Special case children as a string to avoid the unnecessary comment.
1552+
// TODO: Remove this special case after the general optimization is in place.
1553+
target.push(stringToChunk(encodeHTMLTextNode(children)));
1554+
return null;
1555+
}
1556+
return children;
1557+
}
1558+
15111559
function pushStartSelect(
15121560
target: Array<Chunk | PrecomputedChunk>,
15131561
props: Object,
@@ -3513,12 +3561,17 @@ export function pushStartInstance(
35133561
case 'span':
35143562
case 'svg':
35153563
case 'path':
3516-
case 'a':
35173564
case 'g':
35183565
case 'p':
35193566
case 'li':
35203567
// Fast track very common tags
35213568
break;
3569+
case 'a':
3570+
if (enableFilterEmptyStringAttributesDOM) {
3571+
return pushStartAnchor(target, props);
3572+
} else {
3573+
break;
3574+
}
35223575
// Special tags
35233576
case 'select':
35243577
return pushStartSelect(target, props);

packages/react-dom/src/__tests__/ReactDOMComponent-test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,16 @@ describe('ReactDOMComponent', () => {
640640
expect(node.hasAttribute('href')).toBe(false);
641641
});
642642

643+
it('should allow an empty href attribute on anchors', async () => {
644+
const container = document.createElement('div');
645+
const root = ReactDOMClient.createRoot(container);
646+
await act(() => {
647+
root.render(<a href="" />);
648+
});
649+
const node = container.firstChild;
650+
expect(node.getAttribute('href')).toBe('');
651+
});
652+
643653
it('should allow an empty action attribute', async () => {
644654
const container = document.createElement('div');
645655
const root = ReactDOMClient.createRoot(container);

packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,38 @@ describe('ReactDOMServerIntegration', () => {
5454
expect(e.getAttribute('width')).toBe('30');
5555
});
5656

57+
itRenders('empty src on img', async render => {
58+
const e = await render(
59+
<img src="" />,
60+
ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0,
61+
);
62+
expect(e.getAttribute('src')).toBe(
63+
ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '',
64+
);
65+
});
66+
67+
itRenders('empty href on anchor', async render => {
68+
const e = await render(<a href="" />);
69+
expect(e.getAttribute('href')).toBe('');
70+
});
71+
72+
itRenders('empty href on other tags', async render => {
73+
const e = await render(
74+
// <link href="" /> would be more sensible.
75+
// However, that results in a hydration warning as well.
76+
// Our test helpers do not support different error counts for initial
77+
// server render and hydration.
78+
// The number of errors on the server need to be equal to the number of
79+
// errors during hydration.
80+
// So we use a <div> instead.
81+
<div href="" />,
82+
ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0,
83+
);
84+
expect(e.getAttribute('href')).toBe(
85+
ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '',
86+
);
87+
});
88+
5789
itRenders('no string prop with true value', async render => {
5890
const e = await render(<a href={true} />, 1);
5991
expect(e.hasAttribute('href')).toBe(false);

0 commit comments

Comments
 (0)