@samitouri / QOS-React / commits / 3092f63c6b

Warn if upper-case tags are used

In #2756 we ended up using toLowerCase to allow case insensitive HTML tags. However, this requires extra processing every time we access the tag or at least we need to process it for mount and store it in an extra field which wastes memory. So instead, we can just enforce case sensitivity for HTML since this might matter for the XML namespaces like SVG anyway.

Sebastian Markbage committed Dec 13, 2016 at 14:52 UTC 3092f63c6bbd6190c3b4410fb41643e4959bfd22
4 files changed +46 -20
scripts/fiber/tests-passing.txt
+1
@@ -640,6 +640,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
640 * should handle dangerouslySetInnerHTML
641 * should work error event on <source> element
642 * should not duplicate uppercased selfclosing tags
643 +* should warn on upper case HTML tags, not SVG nor custom tags
644 * should warn against children for void elements
645 * should warn against dangerouslySetInnerHTML for void elements
646 * should emit a warning once for an unnamed custom component using shady DOM
src/renderers/dom/fiber/ReactDOMFiberComponent.js
+11 -4
@@ -489,8 +489,6 @@ var ReactDOMFiberComponent = {
489 parentNamespace : string | null
490 ) : Element {
491 validateDangerousTag(type);
492 - // TODO:
493 - // const tag = type.toLowerCase(); Do we need to apply lower case only on non-custom elements?
492
493 // We create tags in the namespace of their parent container, except HTML
494 // tags get no namespace.
@@ -498,8 +496,17 @@ var ReactDOMFiberComponent = {
496 var domElement : Element;
497 var namespaceURI = parentNamespace || getIntrinsicNamespace(type);
498 if (namespaceURI == null) {
501 - const tag = type.toLowerCase();
502 - if (tag === 'script') {
499 + if (__DEV__) {
500 + warning(
501 + type === type.toLowerCase() ||
502 + isCustomComponent(type, props),
503 + '<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
504 + 'in React.',
505 + type
506 + );
507 + }
508 +
509 + if (type === 'script') {
510 // Create the script via .innerHTML so its "parser-inserted" flag is
511 // set to true and it does not execute
512 var div = ownerDocument.createElement('div');
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+25 -16
@@ -14,6 +14,7 @@
14
15 describe('ReactDOMComponent', () => {
16 var React;
17 + var ReactTestUtils;
18 var ReactDOM;
19 var ReactDOMFeatureFlags;
20 var ReactDOMServer;
@@ -29,16 +30,11 @@ describe('ReactDOMComponent', () => {
30 ReactDOM = require('ReactDOM');
31 ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
32 ReactDOMServer = require('ReactDOMServer');
33 + ReactTestUtils = require('ReactTestUtils');
34 inputValueTracking = require('inputValueTracking');
35 });
36
37 describe('updateDOM', () => {
36 - var ReactTestUtils;
37 -
38 - beforeEach(() => {
39 - ReactTestUtils = require('ReactTestUtils');
40 - });
41 -
38 it('should handle className', () => {
39 var container = document.createElement('div');
40 ReactDOM.render(<div style={{}} />, container);
@@ -795,6 +791,7 @@ describe('ReactDOMComponent', () => {
791 });
792
793 it('should not duplicate uppercased selfclosing tags', () => {
794 + spyOn(console, 'error');
795 class Container extends React.Component {
796 render() {
797 return React.createElement('BR', null);
@@ -803,6 +800,27 @@ describe('ReactDOMComponent', () => {
800
801 var returnedValue = ReactDOMServer.renderToString(<Container/>);
802 expect(returnedValue).not.toContain('</BR>');
803 + expectDev(console.error.calls.count()).toBe(1);
804 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
805 + '<BR /> is using uppercase HTML.'
806 + );
807 + });
808 +
809 + it('should warn on upper case HTML tags, not SVG nor custom tags', () => {
810 + spyOn(console, 'error');
811 + ReactTestUtils.renderIntoDocument(
812 + React.createElement('svg', null, React.createElement('PATH'))
813 + );
814 + expectDev(console.error.calls.count()).toBe(0);
815 + ReactTestUtils.renderIntoDocument(
816 + React.createElement('CUSTOM-TAG')
817 + );
818 + expectDev(console.error.calls.count()).toBe(0);
819 + ReactTestUtils.renderIntoDocument(React.createElement('IMG'));
820 + expectDev(console.error.calls.count()).toBe(1);
821 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
822 + '<IMG /> is using uppercase HTML.'
823 + );
824 });
825
826 it('should warn against children for void elements', () => {
@@ -1171,8 +1189,7 @@ describe('ReactDOMComponent', () => {
1189 .mock('isEventSupported');
1190 var isEventSupported = require('isEventSupported');
1191 isEventSupported.mockReturnValueOnce(false);
1174 -
1175 - var ReactTestUtils = require('ReactTestUtils');
1192 + ReactTestUtils = require('ReactTestUtils');
1193
1194 spyOn(console, 'error');
1195 ReactTestUtils.renderIntoDocument(<div onScroll={function() {}} />);
@@ -1191,7 +1208,6 @@ describe('ReactDOMComponent', () => {
1208
1209 describe('tag sanitization', () => {
1210 it('should throw when an invalid tag name is used', () => {
1194 - var ReactTestUtils = require('ReactTestUtils');
1211 var hackzor = React.createElement('script tag');
1212 expect(
1213 () => ReactTestUtils.renderIntoDocument(hackzor)
@@ -1201,7 +1217,6 @@ describe('ReactDOMComponent', () => {
1217 });
1218
1219 it('should throw when an attack vector is used', () => {
1204 - var ReactTestUtils = require('ReactTestUtils');
1220 var hackzor = React.createElement('div><img /><div');
1221 expect(
1222 () => ReactTestUtils.renderIntoDocument(hackzor)
@@ -1212,12 +1227,6 @@ describe('ReactDOMComponent', () => {
1227 });
1228
1229 describe('nesting validation', () => {
1215 - var ReactTestUtils;
1216 -
1217 - beforeEach(() => {
1218 - ReactTestUtils = require('ReactTestUtils');
1219 - });
1220 -
1230 it('warns on invalid nesting', () => {
1231 spyOn(console, 'error');
1232 ReactTestUtils.renderIntoDocument(<div><tr /><tr /></div>);
src/renderers/dom/stack/client/ReactDOMComponent.js
+9
@@ -524,6 +524,15 @@ ReactDOMComponent.Mixin = {
524 namespaceURI = DOMNamespaces.html;
525 }
526 if (namespaceURI === DOMNamespaces.html) {
527 + if (__DEV__) {
528 + warning(
529 + isCustomComponent(this._tag, props) ||
530 + this._tag === this._currentElement.type,
531 + '<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
532 + 'in React.',
533 + this._currentElement.type
534 + );
535 + }
536 if (this._tag === 'svg') {
537 namespaceURI = DOMNamespaces.svg;
538 } else if (this._tag === 'math') {