@samitouri / QOS-React / commits / 332eceface

Revert "Statically enable enableFilterEmptyStringAttributesDOM (#19502)" (#19504)

This reverts commit 815ee89bf55078f2447fe49a1c68336e70c1ce44.

Dan Abramov committed Jul 31, 2020 at 15:01 UTC 332ecefacec7a15d9088b37e2a158fe91641ed67
12 files changed +131 -111
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+92 -90
@@ -441,108 +441,110 @@ describe('ReactDOMComponent', () => {
441 expect(node.hasAttribute('data-foo')).toBe(false);
442 });
443
444 - it('should not add an empty src attribute', () => {
445 - const container = document.createElement('div');
446 - expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
447 - 'An empty string ("") was passed to the src attribute. ' +
448 - 'This may cause the browser to download the whole page again over the network. ' +
449 - 'To fix this, either do not render the element at all ' +
450 - 'or pass null to src instead of an empty string.',
451 - );
452 - const node = container.firstChild;
453 - expect(node.hasAttribute('src')).toBe(false);
444 + if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) {
445 + it('should not add an empty src attribute', () => {
446 + const container = document.createElement('div');
447 + expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
448 + 'An empty string ("") was passed to the src attribute. ' +
449 + 'This may cause the browser to download the whole page again over the network. ' +
450 + 'To fix this, either do not render the element at all ' +
451 + 'or pass null to src instead of an empty string.',
452 + );
453 + const node = container.firstChild;
454 + expect(node.hasAttribute('src')).toBe(false);
455
455 - ReactDOM.render(<img src="abc" />, container);
456 - expect(node.hasAttribute('src')).toBe(true);
456 + ReactDOM.render(<img src="abc" />, container);
457 + expect(node.hasAttribute('src')).toBe(true);
458
458 - expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
459 - 'An empty string ("") was passed to the src attribute. ' +
460 - 'This may cause the browser to download the whole page again over the network. ' +
461 - 'To fix this, either do not render the element at all ' +
462 - 'or pass null to src instead of an empty string.',
463 - );
464 - expect(node.hasAttribute('src')).toBe(false);
465 - });
459 + expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
460 + 'An empty string ("") was passed to the src attribute. ' +
461 + 'This may cause the browser to download the whole page again over the network. ' +
462 + 'To fix this, either do not render the element at all ' +
463 + 'or pass null to src instead of an empty string.',
464 + );
465 + expect(node.hasAttribute('src')).toBe(false);
466 + });
467
467 - it('should not add an empty href attribute', () => {
468 - const container = document.createElement('div');
469 - expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
470 - 'An empty string ("") was passed to the href attribute. ' +
471 - 'To fix this, either do not render the element at all ' +
472 - 'or pass null to href instead of an empty string.',
473 - );
474 - const node = container.firstChild;
475 - expect(node.hasAttribute('href')).toBe(false);
468 + it('should not add an empty href attribute', () => {
469 + const container = document.createElement('div');
470 + expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
471 + 'An empty string ("") was passed to the href attribute. ' +
472 + 'To fix this, either do not render the element at all ' +
473 + 'or pass null to href instead of an empty string.',
474 + );
475 + const node = container.firstChild;
476 + expect(node.hasAttribute('href')).toBe(false);
477
477 - ReactDOM.render(<link href="abc" />, container);
478 - expect(node.hasAttribute('href')).toBe(true);
478 + ReactDOM.render(<link href="abc" />, container);
479 + expect(node.hasAttribute('href')).toBe(true);
480
480 - expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
481 - 'An empty string ("") was passed to the href attribute. ' +
482 - 'To fix this, either do not render the element at all ' +
483 - 'or pass null to href instead of an empty string.',
484 - );
485 - expect(node.hasAttribute('href')).toBe(false);
486 - });
481 + expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
482 + 'An empty string ("") was passed to the href attribute. ' +
483 + 'To fix this, either do not render the element at all ' +
484 + 'or pass null to href instead of an empty string.',
485 + );
486 + expect(node.hasAttribute('href')).toBe(false);
487 + });
488
488 - it('should not add an empty action attribute', () => {
489 - const container = document.createElement('div');
490 - expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
491 - 'An empty string ("") was passed to the action attribute. ' +
492 - 'To fix this, either do not render the element at all ' +
493 - 'or pass null to action instead of an empty string.',
494 - );
495 - const node = container.firstChild;
496 - expect(node.hasAttribute('action')).toBe(false);
489 + it('should not add an empty action attribute', () => {
490 + const container = document.createElement('div');
491 + expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
492 + 'An empty string ("") was passed to the action attribute. ' +
493 + 'To fix this, either do not render the element at all ' +
494 + 'or pass null to action instead of an empty string.',
495 + );
496 + const node = container.firstChild;
497 + expect(node.hasAttribute('action')).toBe(false);
498
498 - ReactDOM.render(<form action="abc" />, container);
499 - expect(node.hasAttribute('action')).toBe(true);
499 + ReactDOM.render(<form action="abc" />, container);
500 + expect(node.hasAttribute('action')).toBe(true);
501
501 - expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
502 - 'An empty string ("") was passed to the action attribute. ' +
503 - 'To fix this, either do not render the element at all ' +
504 - 'or pass null to action instead of an empty string.',
505 - );
506 - expect(node.hasAttribute('action')).toBe(false);
507 - });
502 + expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
503 + 'An empty string ("") was passed to the action attribute. ' +
504 + 'To fix this, either do not render the element at all ' +
505 + 'or pass null to action instead of an empty string.',
506 + );
507 + expect(node.hasAttribute('action')).toBe(false);
508 + });
509
509 - it('should not add an empty formAction attribute', () => {
510 - const container = document.createElement('div');
511 - expect(() =>
512 - ReactDOM.render(<button formAction="" />, container),
513 - ).toErrorDev(
514 - 'An empty string ("") was passed to the formAction attribute. ' +
515 - 'To fix this, either do not render the element at all ' +
516 - 'or pass null to formAction instead of an empty string.',
517 - );
518 - const node = container.firstChild;
519 - expect(node.hasAttribute('formAction')).toBe(false);
510 + it('should not add an empty formAction attribute', () => {
511 + const container = document.createElement('div');
512 + expect(() =>
513 + ReactDOM.render(<button formAction="" />, container),
514 + ).toErrorDev(
515 + 'An empty string ("") was passed to the formAction attribute. ' +
516 + 'To fix this, either do not render the element at all ' +
517 + 'or pass null to formAction instead of an empty string.',
518 + );
519 + const node = container.firstChild;
520 + expect(node.hasAttribute('formAction')).toBe(false);
521
521 - ReactDOM.render(<button formAction="abc" />, container);
522 - expect(node.hasAttribute('formAction')).toBe(true);
522 + ReactDOM.render(<button formAction="abc" />, container);
523 + expect(node.hasAttribute('formAction')).toBe(true);
524
524 - expect(() =>
525 - ReactDOM.render(<button formAction="" />, container),
526 - ).toErrorDev(
527 - 'An empty string ("") was passed to the formAction attribute. ' +
528 - 'To fix this, either do not render the element at all ' +
529 - 'or pass null to formAction instead of an empty string.',
530 - );
531 - expect(node.hasAttribute('formAction')).toBe(false);
532 - });
525 + expect(() =>
526 + ReactDOM.render(<button formAction="" />, container),
527 + ).toErrorDev(
528 + 'An empty string ("") was passed to the formAction attribute. ' +
529 + 'To fix this, either do not render the element at all ' +
530 + 'or pass null to formAction instead of an empty string.',
531 + );
532 + expect(node.hasAttribute('formAction')).toBe(false);
533 + });
534
534 - it('should not filter attributes for custom elements', () => {
535 - const container = document.createElement('div');
536 - ReactDOM.render(
537 - <some-custom-element action="" formAction="" href="" src="" />,
538 - container,
539 - );
540 - const node = container.firstChild;
541 - expect(node.hasAttribute('action')).toBe(true);
542 - expect(node.hasAttribute('formAction')).toBe(true);
543 - expect(node.hasAttribute('href')).toBe(true);
544 - expect(node.hasAttribute('src')).toBe(true);
545 - });
535 + it('should not filter attributes for custom elements', () => {
536 + const container = document.createElement('div');
537 + ReactDOM.render(
538 + <some-custom-element action="" formAction="" href="" src="" />,
539 + container,
540 + );
541 + const node = container.firstChild;
542 + expect(node.hasAttribute('action')).toBe(true);
543 + expect(node.hasAttribute('formAction')).toBe(true);
544 + expect(node.hasAttribute('href')).toBe(true);
545 + expect(node.hasAttribute('src')).toBe(true);
546 + });
547 + }
548
549 it('should apply React-specific aliases to HTML elements', () => {
550 const container = document.createElement('div');
packages/react-dom/src/shared/DOMProperty.js
+26 -21
@@ -7,7 +7,10 @@
7 * @flow
8 */
9
10 -import {enableDeprecatedFlareAPI} from 'shared/ReactFeatureFlags';
10 +import {
11 + enableDeprecatedFlareAPI,
12 + enableFilterEmptyStringAttributesDOM,
13 +} from 'shared/ReactFeatureFlags';
14
15 type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6;
16
@@ -164,28 +167,30 @@ export function shouldRemoveAttribute(
167 return false;
168 }
169 if (propertyInfo !== null) {
167 - if (propertyInfo.removeEmptyString && value === '') {
168 - if (__DEV__) {
169 - if (name === 'src') {
170 - console.error(
171 - 'An empty string ("") was passed to the %s attribute. ' +
172 - 'This may cause the browser to download the whole page again over the network. ' +
173 - 'To fix this, either do not render the element at all ' +
174 - 'or pass null to %s instead of an empty string.',
175 - name,
176 - name,
177 - );
178 - } else {
179 - console.error(
180 - 'An empty string ("") was passed to the %s attribute. ' +
181 - 'To fix this, either do not render the element at all ' +
182 - 'or pass null to %s instead of an empty string.',
183 - name,
184 - name,
185 - );
170 + if (enableFilterEmptyStringAttributesDOM) {
171 + if (propertyInfo.removeEmptyString && value === '') {
172 + if (__DEV__) {
173 + if (name === 'src') {
174 + console.error(
175 + 'An empty string ("") was passed to the %s attribute. ' +
176 + 'This may cause the browser to download the whole page again over the network. ' +
177 + 'To fix this, either do not render the element at all ' +
178 + 'or pass null to %s instead of an empty string.',
179 + name,
180 + name,
181 + );
182 + } else {
183 + console.error(
184 + 'An empty string ("") was passed to the %s attribute. ' +
185 + 'To fix this, either do not render the element at all ' +
186 + 'or pass null to %s instead of an empty string.',
187 + name,
188 + name,
189 + );
190 + }
191 }
192 + return true;
193 }
188 - return true;
194 }
195
196 switch (propertyInfo.type) {
packages/shared/ReactFeatureFlags.js
+4
@@ -7,6 +7,10 @@
7 * @flow strict
8 */
9
10 +// Filter certain DOM attributes (e.g. src, href) if their values are empty strings.
11 +// This prevents e.g. <img src=""> from making an unnecessary HTTP request for certain browsers.
12 +export const enableFilterEmptyStringAttributesDOM = false;
13 +
14 // Adds verbose console logging for e.g. state updates, suspense, and work loop stuff.
15 // Intended to enable React core members to more easily debug scheduling issues in DEV builds.
16 export const enableDebugTracing = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -44,6 +44,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
44 export const warnAboutSpreadingKeyToJSX = false;
45 export const enableComponentStackLocations = false;
46 export const enableLegacyFBSupport = false;
47 +export const enableFilterEmptyStringAttributesDOM = false;
48 export const disableOnScrollBubbling = false;
49
50 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = false;
45 export const enableLegacyFBSupport = false;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47 export const disableOnScrollBubbling = false;
48
49 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = true;
45 export const enableLegacyFBSupport = false;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47 export const disableOnScrollBubbling = false;
48
49 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = false;
45 export const enableLegacyFBSupport = false;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47
48 export const enableNewReconciler = false;
49 export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = true;
45 export const enableLegacyFBSupport = false;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47 export const disableOnScrollBubbling = false;
48
49 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = true;
45 export const enableLegacyFBSupport = false;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47 export const disableOnScrollBubbling = false;
48
49 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
43 export const warnAboutSpreadingKeyToJSX = false;
44 export const enableComponentStackLocations = true;
45 export const enableLegacyFBSupport = !__EXPERIMENTAL__;
46 +export const enableFilterEmptyStringAttributesDOM = false;
47 export const disableOnScrollBubbling = false;
48
49 export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -15,6 +15,7 @@
15
16 export const warnAboutSpreadingKeyToJSX = __VARIANT__;
17 export const disableInputAttributeSyncing = __VARIANT__;
18 +export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
19 export const enableLegacyFBSupport = __VARIANT__;
20 export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
21 export const disableOnScrollBubbling = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -21,6 +21,7 @@ export const {
21 disableSchedulerTimeoutBasedOnReactExpirationTime,
22 warnAboutSpreadingKeyToJSX,
23 replayFailedUnitOfWorkWithInvokeGuardedCallback,
24 + enableFilterEmptyStringAttributesDOM,
25 enableLegacyFBSupport,
26 deferRenderPhaseUpdateToNextBatch,
27 decoupleUpdatePriorityFromScheduler,