Filter certain DOM attributes (e.g. src) if value is empty string (#18513)
* Filter certain DOM attributes (e.g. src, href) if their values are empty strings This prevents e.g. <img src=""> from making an unnecessar HTTP request for certain browsers. * Expanded warning recommendation * Improved error message * Further refined error message
Brian Vaughn committed
Apr 7, 2020 at 09:52 UTC
dc49ea108c3a33fcc717ef346847dd1ad4d14f66
11 files changed
+165
-1
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+105
@@ -445,6 +445,111 @@ describe('ReactDOMComponent', () => {
445
expect(node.hasAttribute('data-foo')).toBe(false);
446
});
447
448
+ if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) {
449
+ it('should not add an empty src attribute', () => {
450
+ const container = document.createElement('div');
451
+ expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
452
+ 'An empty string ("") was passed to the src attribute. ' +
453
+ 'This may cause the browser to download the whole page again over the network. ' +
454
+ 'To fix this, either do not render the element at all ' +
455
+ 'or pass null to src instead of an empty string.',
456
+ );
457
+ const node = container.firstChild;
458
+ expect(node.hasAttribute('src')).toBe(false);
459
+
460
+ ReactDOM.render(<img src="abc" />, container);
461
+ expect(node.hasAttribute('src')).toBe(true);
462
+
463
+ expect(() => ReactDOM.render(<img src="" />, container)).toErrorDev(
464
+ 'An empty string ("") was passed to the src attribute. ' +
465
+ 'This may cause the browser to download the whole page again over the network. ' +
466
+ 'To fix this, either do not render the element at all ' +
467
+ 'or pass null to src instead of an empty string.',
468
+ );
469
+ expect(node.hasAttribute('src')).toBe(false);
470
+ });
471
+
472
+ it('should not add an empty href attribute', () => {
473
+ const container = document.createElement('div');
474
+ expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
475
+ 'An empty string ("") was passed to the href attribute. ' +
476
+ 'To fix this, either do not render the element at all ' +
477
+ 'or pass null to href instead of an empty string.',
478
+ );
479
+ const node = container.firstChild;
480
+ expect(node.hasAttribute('href')).toBe(false);
481
+
482
+ ReactDOM.render(<link href="abc" />, container);
483
+ expect(node.hasAttribute('href')).toBe(true);
484
+
485
+ expect(() => ReactDOM.render(<link href="" />, container)).toErrorDev(
486
+ 'An empty string ("") was passed to the href attribute. ' +
487
+ 'To fix this, either do not render the element at all ' +
488
+ 'or pass null to href instead of an empty string.',
489
+ );
490
+ expect(node.hasAttribute('href')).toBe(false);
491
+ });
492
+
493
+ it('should not add an empty action attribute', () => {
494
+ const container = document.createElement('div');
495
+ expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
496
+ 'An empty string ("") was passed to the action attribute. ' +
497
+ 'To fix this, either do not render the element at all ' +
498
+ 'or pass null to action instead of an empty string.',
499
+ );
500
+ const node = container.firstChild;
501
+ expect(node.hasAttribute('action')).toBe(false);
502
+
503
+ ReactDOM.render(<form action="abc" />, container);
504
+ expect(node.hasAttribute('action')).toBe(true);
505
+
506
+ expect(() => ReactDOM.render(<form action="" />, container)).toErrorDev(
507
+ 'An empty string ("") was passed to the action attribute. ' +
508
+ 'To fix this, either do not render the element at all ' +
509
+ 'or pass null to action instead of an empty string.',
510
+ );
511
+ expect(node.hasAttribute('action')).toBe(false);
512
+ });
513
+
514
+ it('should not add an empty formAction attribute', () => {
515
+ const container = document.createElement('div');
516
+ expect(() =>
517
+ ReactDOM.render(<button formAction="" />, container),
518
+ ).toErrorDev(
519
+ 'An empty string ("") was passed to the formAction attribute. ' +
520
+ 'To fix this, either do not render the element at all ' +
521
+ 'or pass null to formAction instead of an empty string.',
522
+ );
523
+ const node = container.firstChild;
524
+ expect(node.hasAttribute('formAction')).toBe(false);
525
+
526
+ ReactDOM.render(<button formAction="abc" />, container);
527
+ expect(node.hasAttribute('formAction')).toBe(true);
528
+
529
+ expect(() =>
530
+ ReactDOM.render(<button formAction="" />, container),
531
+ ).toErrorDev(
532
+ 'An empty string ("") was passed to the formAction attribute. ' +
533
+ 'To fix this, either do not render the element at all ' +
534
+ 'or pass null to formAction instead of an empty string.',
535
+ );
536
+ expect(node.hasAttribute('formAction')).toBe(false);
537
+ });
538
+
539
+ it('should not filter attributes for custom elements', () => {
540
+ const container = document.createElement('div');
541
+ ReactDOM.render(
542
+ <some-custom-element action="" formAction="" href="" src="" />,
543
+ container,
544
+ );
545
+ const node = container.firstChild;
546
+ expect(node.hasAttribute('action')).toBe(true);
547
+ expect(node.hasAttribute('formAction')).toBe(true);
548
+ expect(node.hasAttribute('href')).toBe(true);
549
+ expect(node.hasAttribute('src')).toBe(true);
550
+ });
551
+ }
552
+
553
it('should apply React-specific aliases to HTML elements', () => {
554
const container = document.createElement('div');
555
ReactDOM.render(<form acceptCharset="foo" />, container);
packages/react-dom/src/shared/DOMProperty.js
+48
-1
@@ -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
@@ -52,6 +55,7 @@ export type PropertyInfo = {|
55
+propertyName: string,
56
+type: PropertyType,
57
+sanitizeURL: boolean,
58
+ +removeEmptyString: boolean,
59
|};
60
61
/* eslint-disable max-len */
@@ -163,6 +167,32 @@ export function shouldRemoveAttribute(
167
return false;
168
}
169
if (propertyInfo !== null) {
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
+ }
194
+ }
195
+
196
switch (propertyInfo.type) {
197
case BOOLEAN:
198
return !value;
@@ -188,6 +218,7 @@ function PropertyInfoRecord(
218
attributeName: string,
219
attributeNamespace: string | null,
220
sanitizeURL: boolean,
221
+ removeEmptyString: boolean,
222
) {
223
this.acceptsBooleans =
224
type === BOOLEANISH_STRING ||
@@ -199,6 +230,7 @@ function PropertyInfoRecord(
230
this.propertyName = name;
231
this.type = type;
232
this.sanitizeURL = sanitizeURL;
233
+ this.removeEmptyString = removeEmptyString;
234
}
235
236
// When adding attributes to this list, be sure to also add them to
@@ -232,6 +264,7 @@ reservedProps.forEach(name => {
264
name, // attributeName
265
null, // attributeNamespace
266
false, // sanitizeURL
267
+ false, // removeEmptyString
268
);
269
});
270
@@ -250,6 +283,7 @@ reservedProps.forEach(name => {
283
attributeName, // attributeName
284
null, // attributeNamespace
285
false, // sanitizeURL
286
+ false, // removeEmptyString
287
);
288
});
289
@@ -264,6 +298,7 @@ reservedProps.forEach(name => {
298
name.toLowerCase(), // attributeName
299
null, // attributeNamespace
300
false, // sanitizeURL
301
+ false, // removeEmptyString
302
);
303
});
304
@@ -284,6 +319,7 @@ reservedProps.forEach(name => {
319
name, // attributeName
320
null, // attributeNamespace
321
false, // sanitizeURL
322
+ false, // removeEmptyString
323
);
324
});
325
@@ -322,6 +358,7 @@ reservedProps.forEach(name => {
358
name.toLowerCase(), // attributeName
359
null, // attributeNamespace
360
false, // sanitizeURL
361
+ false, // removeEmptyString
362
);
363
});
364
@@ -346,6 +383,7 @@ reservedProps.forEach(name => {
383
name, // attributeName
384
null, // attributeNamespace
385
false, // sanitizeURL
386
+ false, // removeEmptyString
387
);
388
});
389
@@ -366,6 +404,7 @@ reservedProps.forEach(name => {
404
name, // attributeName
405
null, // attributeNamespace
406
false, // sanitizeURL
407
+ false, // removeEmptyString
408
);
409
});
410
@@ -387,6 +426,7 @@ reservedProps.forEach(name => {
426
name, // attributeName
427
null, // attributeNamespace
428
false, // sanitizeURL
429
+ false, // removeEmptyString
430
);
431
});
432
@@ -399,6 +439,7 @@ reservedProps.forEach(name => {
439
name.toLowerCase(), // attributeName
440
null, // attributeNamespace
441
false, // sanitizeURL
442
+ false, // removeEmptyString
443
);
444
});
445
@@ -497,6 +538,7 @@ const capitalize = token => token[1].toUpperCase();
538
attributeName,
539
null, // attributeNamespace
540
false, // sanitizeURL
541
+ false, // removeEmptyString
542
);
543
});
544
@@ -521,6 +563,7 @@ const capitalize = token => token[1].toUpperCase();
563
attributeName,
564
'http://www.w3.org/1999/xlink',
565
false, // sanitizeURL
566
+ false, // removeEmptyString
567
);
568
});
569
@@ -542,6 +585,7 @@ const capitalize = token => token[1].toUpperCase();
585
attributeName,
586
'http://www.w3.org/XML/1998/namespace',
587
false, // sanitizeURL
588
+ false, // removeEmptyString
589
);
590
});
591
@@ -556,6 +600,7 @@ const capitalize = token => token[1].toUpperCase();
600
attributeName.toLowerCase(), // attributeName
601
null, // attributeNamespace
602
false, // sanitizeURL
603
+ false, // removeEmptyString
604
);
605
});
606
@@ -569,6 +614,7 @@ properties[xlinkHref] = new PropertyInfoRecord(
614
'xlink:href',
615
'http://www.w3.org/1999/xlink',
616
true, // sanitizeURL
617
+ false, // removeEmptyString
618
);
619
620
['src', 'href', 'action', 'formAction'].forEach(attributeName => {
@@ -579,5 +625,6 @@ properties[xlinkHref] = new PropertyInfoRecord(
625
attributeName.toLowerCase(), // attributeName
626
null, // attributeNamespace
627
true, // sanitizeURL
628
+ true, // removeEmptyString
629
);
630
});
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 unnecessar HTTP request for certain browsers.
12
+export const enableFilterEmptyStringAttributesDOM = false;
13
+
14
// Helps identify side effects in render-phase lifecycle hooks and setState
15
// reducers by double invoking them in Strict Mode.
16
export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -44,6 +44,7 @@ export const enableModernEventSystem = false;
44
export const warnAboutSpreadingKeyToJSX = false;
45
export const enableComponentStackLocations = false;
46
export const enableLegacyFBSupport = false;
47
+export const enableFilterEmptyStringAttributesDOM = false;
48
49
// Internal-only attempt to debug a React Native issue. See D20130868.
50
export const throwEarlyForMysteriousError = true;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -43,6 +43,7 @@ export const enableModernEventSystem = false;
43
export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = false;
46
+export const enableFilterEmptyStringAttributesDOM = false;
47
48
// Internal-only attempt to debug a React Native issue. See D20130868.
49
export const throwEarlyForMysteriousError = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -43,6 +43,7 @@ export const enableModernEventSystem = false;
43
export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = false;
46
+export const enableFilterEmptyStringAttributesDOM = false;
47
48
// Internal-only attempt to debug a React Native issue. See D20130868.
49
export const throwEarlyForMysteriousError = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -43,6 +43,7 @@ export const enableModernEventSystem = false;
43
export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = false;
46
+export const enableFilterEmptyStringAttributesDOM = false;
47
48
// Internal-only attempt to debug a React Native issue. See D20130868.
49
export const throwEarlyForMysteriousError = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -43,6 +43,7 @@ export const enableModernEventSystem = false;
43
export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = false;
46
+export const enableFilterEmptyStringAttributesDOM = false;
47
48
// Internal-only attempt to debug a React Native issue. See D20130868.
49
export const throwEarlyForMysteriousError = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -43,6 +43,7 @@ export const enableModernEventSystem = false;
43
export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
46
+export const enableFilterEmptyStringAttributesDOM = false;
47
48
// Internal-only attempt to debug a React Native issue. See D20130868.
49
export const throwEarlyForMysteriousError = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -17,6 +17,7 @@ export const warnAboutSpreadingKeyToJSX = __VARIANT__;
17
export const enableComponentStackLocations = __VARIANT__;
18
export const disableModulePatternComponents = __VARIANT__;
19
export const disableInputAttributeSyncing = __VARIANT__;
20
+export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
21
22
// These are already tested in both modes using the build type dimension,
23
// so we don't need to use __VARIANT__ to get extra coverage.
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -24,6 +24,7 @@ export const {
24
enableComponentStackLocations,
25
replayFailedUnitOfWorkWithInvokeGuardedCallback,
26
enableModernEventSystem,
27
+ enableFilterEmptyStringAttributesDOM,
28
} = dynamicFeatureFlags;
29
30
// On WWW, __EXPERIMENTAL__ is used for a new modern build.