diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md index 0adb07ad60e82..f3a8ebfde9923 100644 --- a/fixtures/attribute-behavior/AttributeTableSnapshot.md +++ b/fixtures/attribute-behavior/AttributeTableSnapshot.md @@ -5077,7 +5077,7 @@ | Test Case | Flags | Result | | --- | --- | --- | | `href=(string)`| (changed)| `"https://reactjs.com/"` | -| `href=(empty string)`| (initial, warning)| `` | +| `href=(empty string)`| (changed)| `"http://localhost:3000/"` | | `href=(array with string)`| (changed)| `"https://reactjs.com/"` | | `href=(empty array)`| (changed)| `"http://localhost:3000/"` | | `href=(object)`| (changed)| `"http://localhost:3000/result%20of%20toString()"` | diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index a94c990733915..7dcf9e52be472 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -434,7 +434,11 @@ function setProp( case 'src': case 'href': { if (enableFilterEmptyStringAttributesDOM) { - if (value === '') { + if ( + value === '' && + // is fine for "reload" links. + !(tag === 'a' && key === 'href') + ) { if (__DEV__) { if (key === 'src') { console.error( @@ -2350,7 +2354,11 @@ function diffHydratedGenericElement( case 'src': case 'href': if (enableFilterEmptyStringAttributesDOM) { - if (value === '') { + if ( + value === '' && + // is fine for "reload" links. + !(tag === 'a' && propKey === 'href') + ) { if (__DEV__) { if (propKey === 'src') { console.error( diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index ae409cdeed554..b6fe8d9109a6f 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -1503,6 +1503,54 @@ function checkSelectProp(props: any, propName: string) { } } +function pushStartAnchor( + target: Array, + props: Object, +): ReactNodeList { + target.push(startChunkForTag('a')); + + let children = null; + let innerHTML = null; + for (const propKey in props) { + if (hasOwnProperty.call(props, propKey)) { + const propValue = props[propKey]; + if (propValue == null) { + continue; + } + switch (propKey) { + case 'children': + children = propValue; + break; + case 'dangerouslySetInnerHTML': + innerHTML = propValue; + break; + case 'href': + if (propValue === '') { + // Empty `href` is special on anchors so we're short-circuiting here. + // On other tags it should trigger a warning + pushStringAttribute(target, 'href', ''); + } else { + pushAttribute(target, propKey, propValue); + } + break; + default: + pushAttribute(target, propKey, propValue); + break; + } + } + } + + target.push(endOfStartTag); + pushInnerHTML(target, innerHTML, children); + if (typeof children === 'string') { + // Special case children as a string to avoid the unnecessary comment. + // TODO: Remove this special case after the general optimization is in place. + target.push(stringToChunk(encodeHTMLTextNode(children))); + return null; + } + return children; +} + function pushStartSelect( target: Array, props: Object, @@ -3535,7 +3583,14 @@ export function pushStartInstance( case 'span': case 'svg': case 'path': + // Fast track very common tags + break; case 'a': + if (enableFilterEmptyStringAttributesDOM) { + return pushStartAnchor(target, props); + } else { + break; + } case 'g': case 'p': case 'li': diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index f904d372052aa..006b29fd97ab3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -640,6 +640,16 @@ describe('ReactDOMComponent', () => { expect(node.hasAttribute('href')).toBe(false); }); + it('should allow an empty href attribute on anchors', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + const node = container.firstChild; + expect(node.getAttribute('href')).toBe(''); + }); + it('should allow an empty action attribute', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js index 31e4e94ac8c4a..36c3c0cb94840 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js @@ -54,6 +54,38 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('width')).toBe('30'); }); + itRenders('empty src on img', async render => { + const e = await render( + , + ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0, + ); + expect(e.getAttribute('src')).toBe( + ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '', + ); + }); + + itRenders('empty href on anchor', async render => { + const e = await render(); + expect(e.getAttribute('href')).toBe(''); + }); + + itRenders('empty href on other tags', async render => { + const e = await render( + // 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
instead. +
, + ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0, + ); + expect(e.getAttribute('href')).toBe( + ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '', + ); + }); + itRenders('no string prop with true value', async render => { const e = await render(, 1); expect(e.hasAttribute('href')).toBe(false);