From 0f1d2059730ef302393d2caab7f8c5f7622095df Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Thu, 4 Apr 2024 10:15:52 -0400 Subject: [PATCH] Cleanup enableFilterEmptyStringAttributesDOM flag It's `true` everywhere. --- .../src/client/ReactDOMComponent.js | 119 +++++----- .../src/server/ReactFizzConfigDOM.js | 53 ++--- .../src/__tests__/ReactDOMComponent-test.js | 208 +++++++++--------- .../src/__tests__/ReactDOMFloat-test.js | 4 +- ...eactDOMServerIntegrationAttributes-test.js | 15 +- packages/shared/ReactFeatureFlags.js | 5 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - 12 files changed, 187 insertions(+), 223 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 673fd47eca46..06a0d78c2a92 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -68,7 +68,6 @@ import sanitizeURL from '../shared/sanitizeURL'; import { disableIEWorkarounds, enableTrustedTypesIntegration, - enableFilterEmptyStringAttributesDOM, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -407,35 +406,33 @@ function setProp( // These attributes accept URLs. These must not allow javascript: URLS. case 'src': case 'href': { - if (enableFilterEmptyStringAttributesDOM) { - if ( - value === '' && - // 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, - ); - } + if ( + value === '' && + // 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; } + domElement.removeAttribute(key); + break; } if ( value == null || @@ -2435,42 +2432,40 @@ function diffHydratedGenericElement( } case 'src': case 'href': - if (enableFilterEmptyStringAttributesDOM) { - if ( - value === '' && - // is fine for "reload" links. - !(tag === 'a' && propKey === 'href') - ) { - if (__DEV__) { - if (propKey === '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.', - propKey, - propKey, - ); - } 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.', - propKey, - propKey, - ); - } + if ( + value === '' && + // is fine for "reload" links. + !(tag === 'a' && propKey === 'href') + ) { + if (__DEV__) { + if (propKey === '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.', + propKey, + propKey, + ); + } 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.', + propKey, + propKey, + ); } - hydrateSanitizedAttribute( - domElement, - propKey, - propKey, - null, - extraAttributes, - serverDifferences, - ); - continue; } + hydrateSanitizedAttribute( + domElement, + propKey, + propKey, + null, + extraAttributes, + serverDifferences, + ); + continue; } hydrateSanitizedAttribute( domElement, diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 475e934a6049..faef1b54b2bd 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -27,10 +27,7 @@ import { import {Children} from 'react'; -import { - enableFilterEmptyStringAttributesDOM, - enableFizzExternalRuntime, -} from 'shared/ReactFeatureFlags'; +import {enableFizzExternalRuntime} from 'shared/ReactFeatureFlags'; import type { Destination, @@ -1191,30 +1188,28 @@ function pushAttribute( } case 'src': case 'href': { - if (enableFilterEmptyStringAttributesDOM) { - if (value === '') { - if (__DEV__) { - if (name === '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.', - name, - name, - ); - } 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.', - name, - name, - ); - } + if (value === '') { + if (__DEV__) { + if (name === '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.', + name, + name, + ); + } 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.', + name, + name, + ); } - return; } + return; } } // Fall through to the last case which shouldn't remove empty strings. @@ -3511,11 +3506,7 @@ export function pushStartInstance( // Fast track very common tags break; case 'a': - if (enableFilterEmptyStringAttributesDOM) { - return pushStartAnchor(target, props); - } else { - break; - } + return pushStartAnchor(target, props); 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 1d0c3581387d..acc297ca36d3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -587,133 +587,131 @@ describe('ReactDOMComponent', () => { expect(node.hasAttribute('data-foo')).toBe(false); }); - if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) { - it('should not add an empty src attribute', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the src 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 src instead of an empty string.', - ); - const node = container.firstChild; - expect(node.hasAttribute('src')).toBe(false); - + it('should not add an empty src attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - expect(node.hasAttribute('src')).toBe(true); + }).toErrorDev( + 'An empty string ("") was passed to the src 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 src instead of an empty string.', + ); + const node = container.firstChild; + expect(node.hasAttribute('src')).toBe(false); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the src 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 src instead of an empty string.', - ); - expect(node.hasAttribute('src')).toBe(false); + await act(() => { + root.render(); }); + expect(node.hasAttribute('src')).toBe(true); - it('should not add an empty href attribute', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the href attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); - const node = container.firstChild; - expect(node.hasAttribute('href')).toBe(false); + await expect(async () => { + await act(() => { + root.render(); + }); + }).toErrorDev( + 'An empty string ("") was passed to the src 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 src instead of an empty string.', + ); + expect(node.hasAttribute('src')).toBe(false); + }); + it('should not add an empty href attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - expect(node.hasAttribute('href')).toBe(true); + }).toErrorDev( + 'An empty string ("") was passed to the href attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to href instead of an empty string.', + ); + const node = container.firstChild; + expect(node.hasAttribute('href')).toBe(false); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'An empty string ("") was passed to the href attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); - expect(node.hasAttribute('href')).toBe(false); + await act(() => { + root.render(); }); + expect(node.hasAttribute('href')).toBe(true); - it('should allow an empty href attribute on anchors', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); + await expect(async () => { await act(() => { - root.render(); + root.render(); }); - const node = container.firstChild; - expect(node.getAttribute('href')).toBe(''); + }).toErrorDev( + 'An empty string ("") was passed to the href attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to href instead of an empty string.', + ); + 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); - await act(() => { - root.render(
); - }); - const node = container.firstChild; - expect(node.getAttribute('action')).toBe(''); + it('should allow an empty action attribute', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + const node = container.firstChild; + expect(node.getAttribute('action')).toBe(''); - await act(() => { - root.render(); - }); - expect(node.hasAttribute('action')).toBe(true); + await act(() => { + root.render(); + }); + expect(node.hasAttribute('action')).toBe(true); - await act(() => { - root.render(); - }); - expect(node.getAttribute('action')).toBe(''); + await act(() => { + root.render(); }); + expect(node.getAttribute('action')).toBe(''); + }); - it('allows empty string of a formAction to override the default of a parent', async () => { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - -