ReactDOMComponent should warn when provided children for void elements
Andreas Svensson committed
Sep 28, 2014 at 11:47 UTC
20dd247292c36cc00c9441ee3327eda6a2222912
2 files changed
+85
-3
src/browser/ui/ReactDOMComponent.js
+20
-3
@@ -50,11 +50,21 @@ var BackendIDOperations = null;
50
/**
51
* @param {?object} props
52
*/
53
-function assertValidProps(props) {
53
+function assertValidProps(component, props) {
54
if (!props) {
55
return;
56
}
57
// Note the use of `==` which checks for null or undefined.
58
+ if (__DEV__) {
59
+ if (voidElementTags[component._tag]) {
60
+ warning(
61
+ props.children == null && props.dangerouslySetInnerHTML == null,
62
+ '%s is a void element tag and must not have `children` or ' +
63
+ 'use `props.dangerouslySetInnerHTML`.',
64
+ component._tag
65
+ );
66
+ }
67
+ }
68
if (props.dangerouslySetInnerHTML != null) {
69
invariant(
70
props.children == null,
@@ -134,6 +144,13 @@ var omittedCloseTags = {
144
// NOTE: menuitem's close tag should be omitted, but that causes problems.
145
};
146
147
+// For HTML, certain tags cannot have children. This has the same purpose as
148
+// `omittedCloseTags` except that `menuitem` should still have its closing tag.
149
+
150
+var voidElementTags = assign({
151
+ 'menuitem': true
152
+}, omittedCloseTags);
153
+
154
// We accept any tag to be rendered but since this gets injected into abitrary
155
// HTML, we want to make sure that it's a safe tag.
156
// http://www.w3.org/TR/REC-xml/#NT-Name
@@ -190,7 +207,7 @@ ReactDOMComponent.Mixin = {
207
*/
208
mountComponent: function(rootID, transaction, context) {
209
this._rootNodeID = rootID;
193
- assertValidProps(this._currentElement.props);
210
+ assertValidProps(this, this._currentElement.props);
211
var closeTag = omittedCloseTags[this._tag] ? '' : '</' + this._tag + '>';
212
return (
213
this._createOpenTagMarkupAndPutListeners(transaction) +
@@ -312,7 +329,7 @@ ReactDOMComponent.Mixin = {
329
* @overridable
330
*/
331
updateComponent: function(transaction, prevElement, nextElement, context) {
315
- assertValidProps(this._currentElement.props);
332
+ assertValidProps(this, this._currentElement.props);
333
this._updateDOMProperties(prevElement.props, transaction);
334
this._updateDOMChildren(prevElement.props, transaction, context);
335
},
src/browser/ui/__tests__/ReactDOMComponent-test.js
+65
@@ -303,11 +303,13 @@ describe('ReactDOMComponent', function() {
303
});
304
305
describe('mountComponent', function() {
306
+ var React;
307
var mountComponent;
308
309
beforeEach(function() {
310
require('mock-modules').dumpCache();
311
312
+ React = require('React');
313
var ReactMultiChild = require('ReactMultiChild');
314
var ReactDOMComponent = require('ReactDOMComponent');
315
var ReactReconcileTransaction = require('ReactReconcileTransaction');
@@ -330,6 +332,46 @@ describe('ReactDOMComponent', function() {
332
};
333
});
334
335
+ it("should warn against children for void elements", function() {
336
+ spyOn(console, 'warn');
337
+
338
+ var container = document.createElement('div');
339
+
340
+ React.render(<input>children</input>, container);
341
+
342
+ expect(console.warn.argsForCall.length).toBe(1);
343
+ expect(console.warn.argsForCall[0][0]).toContain('void element');
344
+ });
345
+
346
+ it("should warn against dangerouslySetInnerHTML for void elements", function() {
347
+ spyOn(console, 'warn');
348
+
349
+ var container = document.createElement('div');
350
+
351
+ React.render(
352
+ <input dangerouslySetInnerHTML={{__html: 'content'}} />,
353
+ container
354
+ );
355
+
356
+ expect(console.warn.argsForCall.length).toBe(1);
357
+ expect(console.warn.argsForCall[0][0]).toContain('void element');
358
+ });
359
+
360
+ it("should treat menuitem as a void element but still create the closing tag", function() {
361
+ spyOn(console, 'warn');
362
+
363
+ var container = document.createElement('div');
364
+
365
+ React.render(<menuitem />, container);
366
+
367
+ expect(container.innerHTML).toContain('</menuitem>');
368
+
369
+ React.render(<menuitem>children</menuitem>, container);
370
+
371
+ expect(console.warn.argsForCall.length).toBe(1);
372
+ expect(console.warn.argsForCall[0][0]).toContain('void element');
373
+ });
374
+
375
it("should validate against multiple children props", function() {
376
expect(function() {
377
mountComponent({ children: '', dangerouslySetInnerHTML: '' });
@@ -396,6 +438,29 @@ describe('ReactDOMComponent', function() {
438
container = document.createElement('div');
439
});
440
441
+ it("should warn against children for void elements", function() {
442
+ spyOn(console, 'warn');
443
+
444
+ React.render(<input />, container);
445
+ React.render(<input>children</input>, container);
446
+
447
+ expect(console.warn.argsForCall.length).toBe(1);
448
+ expect(console.warn.argsForCall[0][0]).toContain('void element');
449
+ });
450
+
451
+ it("should warn against dangerouslySetInnerHTML for void elements", function() {
452
+ spyOn(console, 'warn');
453
+
454
+ React.render(<input />, container);
455
+ React.render(
456
+ <input dangerouslySetInnerHTML={{__html: 'content'}} />,
457
+ container
458
+ );
459
+
460
+ expect(console.warn.argsForCall.length).toBe(1);
461
+ expect(console.warn.argsForCall[0][0]).toContain('void element');
462
+ });
463
+
464
it("should validate against multiple children props", function() {
465
React.render(<div></div>, container);
466