Statically enable enableFilterEmptyStringAttributesDOM (facebook#19502)

rickhanlonii · Jul 31, 2020 · 815ee89 · 815ee89
1 parent ede9170
commit 815ee89
Show file tree

Hide file tree

Showing 12 changed files with 111 additions and 131 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -441,110 +441,108 @@ describe('ReactDOMComponent', () => {
       expect(node.hasAttribute('data-foo')).toBe(false);
     });
 
-    if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) {
-      it('should not add an empty src attribute', () => {
-        const container = document.createElement('div');
-        expect(() => ReactDOM.render(<img src="" />, container)).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', () => {
+      const container = document.createElement('div');
+      expect(() => ReactDOM.render(<img src="" />, container)).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);
 
-        ReactDOM.render(<img src="abc" />, container);
-        expect(node.hasAttribute('src')).toBe(true);
+      ReactDOM.render(<img src="abc" />, container);
+      expect(node.hasAttribute('src')).toBe(true);
 
-        expect(() => ReactDOM.render(<img src="" />, container)).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);
-      });
+      expect(() => ReactDOM.render(<img src="" />, container)).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', () => {
-        const container = document.createElement('div');
-        expect(() => ReactDOM.render(<link href="" />, container)).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);
+    it('should not add an empty href attribute', () => {
+      const container = document.createElement('div');
+      expect(() => ReactDOM.render(<link href="" />, container)).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);
 
-        ReactDOM.render(<link href="abc" />, container);
-        expect(node.hasAttribute('href')).toBe(true);
+      ReactDOM.render(<link href="abc" />, container);
+      expect(node.hasAttribute('href')).toBe(true);
 
-        expect(() => ReactDOM.render(<link href="" />, container)).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);
-      });
+      expect(() => ReactDOM.render(<link href="" />, container)).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 not add an empty action attribute', () => {
-        const container = document.createElement('div');
-        expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
-          'An empty string ("") was passed to the action attribute. ' +
-            'To fix this, either do not render the element at all ' +
-            'or pass null to action instead of an empty string.',
-        );
-        const node = container.firstChild;
-        expect(node.hasAttribute('action')).toBe(false);
+    it('should not add an empty action attribute', () => {
+      const container = document.createElement('div');
+      expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
+        'An empty string ("") was passed to the action attribute. ' +
+          'To fix this, either do not render the element at all ' +
+          'or pass null to action instead of an empty string.',
+      );
+      const node = container.firstChild;
+      expect(node.hasAttribute('action')).toBe(false);
 
-        ReactDOM.render(<form action="abc" />, container);
-        expect(node.hasAttribute('action')).toBe(true);
+      ReactDOM.render(<form action="abc" />, container);
+      expect(node.hasAttribute('action')).toBe(true);
 
-        expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
-          'An empty string ("") was passed to the action attribute. ' +
-            'To fix this, either do not render the element at all ' +
-            'or pass null to action instead of an empty string.',
-        );
-        expect(node.hasAttribute('action')).toBe(false);
-      });
+      expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
+        'An empty string ("") was passed to the action attribute. ' +
+          'To fix this, either do not render the element at all ' +
+          'or pass null to action instead of an empty string.',
+      );
+      expect(node.hasAttribute('action')).toBe(false);
+    });
 
-      it('should not add an empty formAction attribute', () => {
-        const container = document.createElement('div');
-        expect(() =>
-          ReactDOM.render(<button formAction="" />, container),
-        ).toErrorDev(
-          'An empty string ("") was passed to the formAction attribute. ' +
-            'To fix this, either do not render the element at all ' +
-            'or pass null to formAction instead of an empty string.',
-        );
-        const node = container.firstChild;
-        expect(node.hasAttribute('formAction')).toBe(false);
+    it('should not add an empty formAction attribute', () => {
+      const container = document.createElement('div');
+      expect(() =>
+        ReactDOM.render(<button formAction="" />, container),
+      ).toErrorDev(
+        'An empty string ("") was passed to the formAction attribute. ' +
+          'To fix this, either do not render the element at all ' +
+          'or pass null to formAction instead of an empty string.',
+      );
+      const node = container.firstChild;
+      expect(node.hasAttribute('formAction')).toBe(false);
 
-        ReactDOM.render(<button formAction="abc" />, container);
-        expect(node.hasAttribute('formAction')).toBe(true);
+      ReactDOM.render(<button formAction="abc" />, container);
+      expect(node.hasAttribute('formAction')).toBe(true);
 
-        expect(() =>
-          ReactDOM.render(<button formAction="" />, container),
-        ).toErrorDev(
-          'An empty string ("") was passed to the formAction attribute. ' +
-            'To fix this, either do not render the element at all ' +
-            'or pass null to formAction instead of an empty string.',
-        );
-        expect(node.hasAttribute('formAction')).toBe(false);
-      });
+      expect(() =>
+        ReactDOM.render(<button formAction="" />, container),
+      ).toErrorDev(
+        'An empty string ("") was passed to the formAction attribute. ' +
+          'To fix this, either do not render the element at all ' +
+          'or pass null to formAction instead of an empty string.',
+      );
+      expect(node.hasAttribute('formAction')).toBe(false);
+    });
 
-      it('should not filter attributes for custom elements', () => {
-        const container = document.createElement('div');
-        ReactDOM.render(
-          <some-custom-element action="" formAction="" href="" src="" />,
-          container,
-        );
-        const node = container.firstChild;
-        expect(node.hasAttribute('action')).toBe(true);
-        expect(node.hasAttribute('formAction')).toBe(true);
-        expect(node.hasAttribute('href')).toBe(true);
-        expect(node.hasAttribute('src')).toBe(true);
-      });
-    }
+    it('should not filter attributes for custom elements', () => {
+      const container = document.createElement('div');
+      ReactDOM.render(
+        <some-custom-element action="" formAction="" href="" src="" />,
+        container,
+      );
+      const node = container.firstChild;
+      expect(node.hasAttribute('action')).toBe(true);
+      expect(node.hasAttribute('formAction')).toBe(true);
+      expect(node.hasAttribute('href')).toBe(true);
+      expect(node.hasAttribute('src')).toBe(true);
+    });
 
     it('should apply React-specific aliases to HTML elements', () => {
       const container = document.createElement('div');

diff --git a/packages/react-dom/src/shared/DOMProperty.js b/packages/react-dom/src/shared/DOMProperty.js
@@ -7,10 +7,7 @@
  * @flow
  */
 
-import {
-  enableDeprecatedFlareAPI,
-  enableFilterEmptyStringAttributesDOM,
-} from 'shared/ReactFeatureFlags';
+import {enableDeprecatedFlareAPI} from 'shared/ReactFeatureFlags';
 
 type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6;
 
@@ -167,30 +164,28 @@ export function shouldRemoveAttribute(
     return false;
   }
   if (propertyInfo !== null) {
-    if (enableFilterEmptyStringAttributesDOM) {
-      if (propertyInfo.removeEmptyString && 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 (propertyInfo.removeEmptyString && 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 true;
       }
+      return true;
     }
 
     switch (propertyInfo.type) {

diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -7,10 +7,6 @@
  * @flow strict
  */
 
-// Filter certain DOM attributes (e.g. src, href) if their values are empty strings.
-// This prevents e.g. <img src=""> from making an unnecessary HTTP request for certain browsers.
-export const enableFilterEmptyStringAttributesDOM = false;
-
 // Adds verbose console logging for e.g. state updates, suspense, and work loop stuff.
 // Intended to enable React core members to more easily debug scheduling issues in DEV builds.
 export const enableDebugTracing = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -44,7 +44,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = false;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js
@@ -43,7 +43,6 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
 export const warnAboutSpreadingKeyToJSX = false;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = !__EXPERIMENTAL__;
-export const enableFilterEmptyStringAttributesDOM = false;
 export const disableOnScrollBubbling = false;
 
 export const enableNewReconciler = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
@@ -15,7 +15,6 @@
 
 export const warnAboutSpreadingKeyToJSX = __VARIANT__;
 export const disableInputAttributeSyncing = __VARIANT__;
-export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
 export const enableLegacyFBSupport = __VARIANT__;
 export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
 export const disableOnScrollBubbling = __VARIANT__;

diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -21,7 +21,6 @@ export const {
   disableSchedulerTimeoutBasedOnReactExpirationTime,
   warnAboutSpreadingKeyToJSX,
   replayFailedUnitOfWorkWithInvokeGuardedCallback,
-  enableFilterEmptyStringAttributesDOM,
   enableLegacyFBSupport,
   deferRenderPhaseUpdateToNextBatch,
   decoupleUpdatePriorityFromScheduler,