Skip to content

Commit f9abf49

Browse files
committed
Validate node nesting, take 3
Nicer version of facebook#644 and facebook#735. Fixes facebook#101. Context is neat.
1 parent 7fe5a3a commit f9abf49

File tree

8 files changed

+329
-13
lines changed

8 files changed

+329
-13
lines changed

src/browser/ReactDOM.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ var ReactDOM = mapObject({
8484
h6: 'h6',
8585
head: 'head',
8686
header: 'header',
87+
hgroup: 'hgroup',
8788
hr: 'hr',
8889
html: 'html',
8990
i: 'i',

src/browser/ui/ReactDOMComponent.js

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
2929
var invariant = require('invariant');
3030
var isEventSupported = require('isEventSupported');
3131
var keyOf = require('keyOf');
32+
var validateDOMNesting = require('validateDOMNesting');
3233
var warning = require('warning');
3334

3435
var deleteListener = ReactBrowserEventEmitter.deleteListener;
@@ -172,6 +173,15 @@ function validateDangerousTag(tag) {
172173
}
173174
}
174175

176+
function processChildContext(context, tagName) {
177+
if (__DEV__) {
178+
// Pass down our tag name to child components for validation purposes
179+
context = assign({}, context);
180+
context[validateDOMNesting.parentTagContextKey] = tagName;
181+
}
182+
return context;
183+
}
184+
175185
/**
176186
* Creates a new React class that is idempotent and capable of containing other
177187
* React components. It accepts event listeners and DOM properties that are
@@ -213,7 +223,18 @@ ReactDOMComponent.Mixin = {
213223
*/
214224
mountComponent: function(rootID, transaction, context) {
215225
this._rootNodeID = rootID;
226+
216227
assertValidProps(this, this._currentElement.props);
228+
if (__DEV__) {
229+
if (context[validateDOMNesting.parentTagContextKey]) {
230+
validateDOMNesting(
231+
context[validateDOMNesting.parentTagContextKey],
232+
this._tag,
233+
this._currentElement
234+
);
235+
}
236+
}
237+
217238
var tagOpen = this._createOpenTagMarkupAndPutListeners(transaction);
218239
var tagContent = this._createContentMarkup(transaction, context);
219240
if (!tagContent && omittedCloseTags[this._tag]) {
@@ -301,7 +322,7 @@ ReactDOMComponent.Mixin = {
301322
var mountImages = this.mountChildren(
302323
childrenToUse,
303324
transaction,
304-
context
325+
processChildContext(context, this._tag)
305326
);
306327
ret = mountImages.join('');
307328
}
@@ -342,7 +363,11 @@ ReactDOMComponent.Mixin = {
342363
updateComponent: function(transaction, prevElement, nextElement, context) {
343364
assertValidProps(this, this._currentElement.props);
344365
this._updateDOMProperties(prevElement.props, transaction);
345-
this._updateDOMChildren(prevElement.props, transaction, context);
366+
this._updateDOMChildren(
367+
prevElement.props,
368+
transaction,
369+
processChildContext(context, this._tag)
370+
);
346371
},
347372

348373
/**

src/browser/ui/ReactDOMTextComponent.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var ReactDOMComponent = require('ReactDOMComponent');
1919

2020
var assign = require('Object.assign');
2121
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
22+
var validateDOMNesting = require('validateDOMNesting');
2223

2324
/**
2425
* Text nodes violate a couple assumptions that React makes about components:
@@ -65,6 +66,16 @@ assign(ReactDOMTextComponent.prototype, {
6566
* @internal
6667
*/
6768
mountComponent: function(rootID, transaction, context) {
69+
if (__DEV__) {
70+
if (context[validateDOMNesting.parentTagContextKey]) {
71+
validateDOMNesting(
72+
context[validateDOMNesting.parentTagContextKey],
73+
'span',
74+
null
75+
);
76+
}
77+
}
78+
6879
this._rootNodeID = rootID;
6980
var escapedText = escapeTextContentForBrowser(this._stringText);
7081

src/browser/ui/ReactDefaultInjection.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ var ReactElement = require('ReactElement');
3838
var ReactEventListener = require('ReactEventListener');
3939
var ReactInjection = require('ReactInjection');
4040
var ReactInstanceHandles = require('ReactInstanceHandles');
41+
var ReactInstanceMap = require('ReactInstanceMap');
4142
var ReactMount = require('ReactMount');
4243
var ReactReconcileTransaction = require('ReactReconcileTransaction');
4344
var SelectEventPlugin = require('SelectEventPlugin');
@@ -51,12 +52,14 @@ function autoGenerateWrapperClass(type) {
5152
return ReactClass.createClass({
5253
tagName: type.toUpperCase(),
5354
render: function() {
55+
// Copy owner down for debugging info
56+
var internalInstance = ReactInstanceMap.get(this);
5457
return new ReactElement(
5558
type,
56-
null,
57-
null,
58-
null,
59-
null,
59+
null, // key
60+
null, // ref
61+
internalInstance._currentElement._owner, // owner
62+
null, // context
6063
this.props
6164
);
6265
}

src/browser/ui/ReactMount.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ var instantiateReactComponent = require('instantiateReactComponent');
3232
var invariant = require('invariant');
3333
var setInnerHTML = require('setInnerHTML');
3434
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
35+
var validateDOMNesting = require('validateDOMNesting');
3536
var warning = require('warning');
3637

3738
var SEPARATOR = ReactInstanceHandles.SEPARATOR;
@@ -246,8 +247,14 @@ function mountComponentIntoNode(
246247
container,
247248
transaction,
248249
shouldReuseMarkup) {
250+
var context = emptyObject;
251+
if (__DEV__) {
252+
context = {};
253+
context[validateDOMNesting.parentTagContextKey] =
254+
container.nodeName.toLowerCase();
255+
}
249256
var markup = ReactReconciler.mountComponent(
250-
componentInstance, rootID, transaction, emptyObject
257+
componentInstance, rootID, transaction, context
251258
);
252259
componentInstance._isTopLevel = true;
253260
ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup);

src/browser/ui/__tests__/ReactDOMComponent-test.js

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,11 +373,11 @@ describe('ReactDOMComponent', function() {
373373

374374
var container = document.createElement('div');
375375

376-
React.render(<menuitem />, container);
376+
React.render(<menu><menuitem /></menu>, container);
377377

378378
expect(container.innerHTML).toContain('</menuitem>');
379379

380-
React.render(<menuitem>children</menuitem>, container);
380+
React.render(<menu><menuitem>children</menuitem></menu>, container);
381381

382382
expect(console.warn.argsForCall.length).toBe(1);
383383
expect(console.warn.argsForCall[0][0]).toContain('void element');
@@ -651,6 +651,54 @@ describe('ReactDOMComponent', function() {
651651
'Invariant Violation: Invalid tag: div><img /><div'
652652
);
653653
});
654+
});
655+
656+
describe('nesting validation', function() {
657+
var React;
658+
var ReactTestUtils;
659+
660+
beforeEach(function() {
661+
React = require('React');
662+
ReactTestUtils = require('ReactTestUtils');
663+
});
664+
665+
it('warns on invalid nesting', () => {
666+
spyOn(console, 'warn');
667+
ReactTestUtils.renderIntoDocument(<div><tr /></div>);
668+
669+
expect(console.warn.calls.length).toBe(1);
670+
expect(console.warn.calls[0].args[0]).toBe(
671+
'Warning: validateDOMNesting(...): <div> cannot contain a <tr> node.'
672+
);
673+
});
654674

675+
it('warns on invalid nesting at root', () => {
676+
spyOn(console, 'warn');
677+
var p = document.createElement('p');
678+
React.render(<tr />, p);
679+
680+
expect(console.warn.calls.length).toBe(1);
681+
expect(console.warn.calls[0].args[0]).toBe(
682+
'Warning: validateDOMNesting(...): <p> cannot contain a <tr> node.'
683+
);
684+
});
685+
686+
it('warns nicely for table rows', () => {
687+
spyOn(console, 'warn');
688+
var Foo = React.createClass({
689+
render: function() {
690+
return <table><tr /></table>;
691+
}
692+
});
693+
ReactTestUtils.renderIntoDocument(<Foo />);
694+
695+
expect(console.warn.calls.length).toBe(1);
696+
expect(console.warn.calls[0].args[0]).toBe(
697+
'Warning: validateDOMNesting(...): <table> cannot contain a <tr> ' +
698+
'node. Add a <tbody> to your code to match the DOM tree generated by ' +
699+
'the browser. Check the render method of `Foo`.'
700+
);
701+
});
655702
});
703+
656704
});

0 commit comments

Comments
 (0)