Skip to content

Commit 332ecef

Browse files
gaearongaearon
authored
Revert "Statically enable enableFilterEmptyStringAttributesDOM (facebook#19502)" (facebook#19504)
This reverts commit 815ee89.
1 parent 815ee89 commit 332ecef

12 files changed

+131
-111
lines changed

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

Lines changed: 92 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -441,108 +441,110 @@ describe('ReactDOMComponent', () => {
441441
expect(node.hasAttribute('data-foo')).toBe(false);
442442
});
443443

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);
454455

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);
457458

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+
});
466467

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);
476477

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);
479480

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+
});
487488

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);
497498

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);
500501

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+
});
508509

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);
520521

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);
523524

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+
});
533534

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+
}
546548

547549
it('should apply React-specific aliases to HTML elements', () => {
548550
const container = document.createElement('div');

‎packages/react-dom/src/shared/DOMProperty.js

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
* @flow
88
*/
99

10-
import {enableDeprecatedFlareAPI} from 'shared/ReactFeatureFlags';
10+
import {
11+
enableDeprecatedFlareAPI,
12+
enableFilterEmptyStringAttributesDOM,
13+
} from 'shared/ReactFeatureFlags';
1114

1215
type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6;
1316

@@ -164,28 +167,30 @@ export function shouldRemoveAttribute(
164167
return false;
165168
}
166169
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+
}
186191
}
192+
return true;
187193
}
188-
return true;
189194
}
190195

191196
switch (propertyInfo.type) {

‎packages/shared/ReactFeatureFlags.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
* @flow strict
88
*/
99

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+
1014
// Adds verbose console logging for e.g. state updates, suspense, and work loop stuff.
1115
// Intended to enable React core members to more easily debug scheduling issues in DEV builds.
1216
export const enableDebugTracing = false;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4444
export const warnAboutSpreadingKeyToJSX = false;
4545
export const enableComponentStackLocations = false;
4646
export const enableLegacyFBSupport = false;
47+
export const enableFilterEmptyStringAttributesDOM = false;
4748
export const disableOnScrollBubbling = false;
4849

4950
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = false;
4545
export const enableLegacyFBSupport = false;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = false;
4545
export const enableLegacyFBSupport = false;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647

4748
export const enableNewReconciler = false;
4849
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.testing.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.testing.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const warnUnstableRenderSubtreeIntoContainer = false;
4343
export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
46+
export const enableFilterEmptyStringAttributesDOM = false;
4647
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
export const warnAboutSpreadingKeyToJSX = __VARIANT__;
1717
export const disableInputAttributeSyncing = __VARIANT__;
18+
export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
1819
export const enableLegacyFBSupport = __VARIANT__;
1920
export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
2021
export const disableOnScrollBubbling = __VARIANT__;

‎packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export const {
2121
disableSchedulerTimeoutBasedOnReactExpirationTime,
2222
warnAboutSpreadingKeyToJSX,
2323
replayFailedUnitOfWorkWithInvokeGuardedCallback,
24+
enableFilterEmptyStringAttributesDOM,
2425
enableLegacyFBSupport,
2526
deferRenderPhaseUpdateToNextBatch,
2627
decoupleUpdatePriorityFromScheduler,

0 commit comments

Comments
 (0)