@samitouri / QOS-React-2 / commits / f9abf493b4

Validate node nesting, take 3

Nicer version of #644 and #735. Fixes #101. Context is neat.

Ben Alpert committed Mar 19, 2015 at 20:41 UTC f9abf493b4685869e9feed5738d8271b0dc4e944
8 files changed +329 -13
src/browser/ReactDOM.js
+1
@@ -84,6 +84,7 @@ var ReactDOM = mapObject({
84 h6: 'h6',
85 head: 'head',
86 header: 'header',
87 + hgroup: 'hgroup',
88 hr: 'hr',
89 html: 'html',
90 i: 'i',
src/browser/ui/ReactDOMComponent.js
+27 -2
@@ -29,6 +29,7 @@ var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
29 var invariant = require('invariant');
30 var isEventSupported = require('isEventSupported');
31 var keyOf = require('keyOf');
32 +var validateDOMNesting = require('validateDOMNesting');
33 var warning = require('warning');
34
35 var deleteListener = ReactBrowserEventEmitter.deleteListener;
@@ -172,6 +173,15 @@ function validateDangerousTag(tag) {
173 }
174 }
175
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 +
185 /**
186 * Creates a new React class that is idempotent and capable of containing other
187 * React components. It accepts event listeners and DOM properties that are
@@ -213,7 +223,18 @@ ReactDOMComponent.Mixin = {
223 */
224 mountComponent: function(rootID, transaction, context) {
225 this._rootNodeID = rootID;
226 +
227 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 +
238 var tagOpen = this._createOpenTagMarkupAndPutListeners(transaction);
239 var tagContent = this._createContentMarkup(transaction, context);
240 if (!tagContent && omittedCloseTags[this._tag]) {
@@ -301,7 +322,7 @@ ReactDOMComponent.Mixin = {
322 var mountImages = this.mountChildren(
323 childrenToUse,
324 transaction,
304 - context
325 + processChildContext(context, this._tag)
326 );
327 ret = mountImages.join('');
328 }
@@ -342,7 +363,11 @@ ReactDOMComponent.Mixin = {
363 updateComponent: function(transaction, prevElement, nextElement, context) {
364 assertValidProps(this, this._currentElement.props);
365 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 + );
371 },
372
373 /**
src/browser/ui/ReactDOMTextComponent.js
+11
@@ -19,6 +19,7 @@ var ReactDOMComponent = require('ReactDOMComponent');
19
20 var assign = require('Object.assign');
21 var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
22 +var validateDOMNesting = require('validateDOMNesting');
23
24 /**
25 * Text nodes violate a couple assumptions that React makes about components:
@@ -65,6 +66,16 @@ assign(ReactDOMTextComponent.prototype, {
66 * @internal
67 */
68 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 +
79 this._rootNodeID = rootID;
80 var escapedText = escapeTextContentForBrowser(this._stringText);
81
src/browser/ui/ReactDefaultInjection.js
+7 -4
@@ -38,6 +38,7 @@ var ReactElement = require('ReactElement');
38 var ReactEventListener = require('ReactEventListener');
39 var ReactInjection = require('ReactInjection');
40 var ReactInstanceHandles = require('ReactInstanceHandles');
41 +var ReactInstanceMap = require('ReactInstanceMap');
42 var ReactMount = require('ReactMount');
43 var ReactReconcileTransaction = require('ReactReconcileTransaction');
44 var SelectEventPlugin = require('SelectEventPlugin');
@@ -51,12 +52,14 @@ function autoGenerateWrapperClass(type) {
52 return ReactClass.createClass({
53 tagName: type.toUpperCase(),
54 render: function() {
55 + // Copy owner down for debugging info
56 + var internalInstance = ReactInstanceMap.get(this);
57 return new ReactElement(
58 type,
56 - null,
57 - null,
58 - null,
59 - null,
59 + null, // key
60 + null, // ref
61 + internalInstance._currentElement._owner, // owner
62 + null, // context
63 this.props
64 );
65 }
src/browser/ui/ReactMount.js
+8 -1
@@ -32,6 +32,7 @@ var instantiateReactComponent = require('instantiateReactComponent');
32 var invariant = require('invariant');
33 var setInnerHTML = require('setInnerHTML');
34 var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
35 +var validateDOMNesting = require('validateDOMNesting');
36 var warning = require('warning');
37
38 var SEPARATOR = ReactInstanceHandles.SEPARATOR;
@@ -246,8 +247,14 @@ function mountComponentIntoNode(
247 container,
248 transaction,
249 shouldReuseMarkup) {
250 + var context = emptyObject;
251 + if (__DEV__) {
252 + context = {};
253 + context[validateDOMNesting.parentTagContextKey] =
254 + container.nodeName.toLowerCase();
255 + }
256 var markup = ReactReconciler.mountComponent(
250 - componentInstance, rootID, transaction, emptyObject
257 + componentInstance, rootID, transaction, context
258 );
259 componentInstance._isTopLevel = true;
260 ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup);
src/browser/ui/__tests__/ReactDOMComponent-test.js
+50 -2
@@ -373,11 +373,11 @@ describe('ReactDOMComponent', function() {
373
374 var container = document.createElement('div');
375
376 - React.render(<menuitem />, container);
376 + React.render(<menu><menuitem /></menu>, container);
377
378 expect(container.innerHTML).toContain('</menuitem>');
379
380 - React.render(<menuitem>children</menuitem>, container);
380 + React.render(<menu><menuitem>children</menuitem></menu>, container);
381
382 expect(console.warn.argsForCall.length).toBe(1);
383 expect(console.warn.argsForCall[0][0]).toContain('void element');
@@ -651,6 +651,54 @@ describe('ReactDOMComponent', function() {
651 'Invariant Violation: Invalid tag: div><img /><div'
652 );
653 });
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 + });
674
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 + });
702 });
703 +
704 });
src/browser/validateDOMNesting.js new
+194
@@ -0,0 +1,194 @@
1 +/**
2 + * Copyright 2015, Facebook, Inc.
3 + * All rights reserved.
4 + *
5 + * This source code is licensed under the BSD-style license found in the
6 + * LICENSE file in the root directory of this source tree. An additional grant
7 + * of patent rights can be found in the PATENTS file in the same directory.
8 + *
9 + * @providesModule validateDOMNesting
10 + */
11 +
12 +'use strict';
13 +
14 +var emptyFunction = require('emptyFunction');
15 +var warning = require('warning');
16 +
17 +var validateDOMNesting = emptyFunction;
18 +
19 +if (__DEV__) {
20 + // The below rules were created from the HTML5 spec and using
21 + // https://github.com/facebook/xhp-lib/blob/1.6.0/src/html.php
22 +
23 + // Flow elements are block or inline elements that can appear in a <div>
24 + var flow = [
25 + 'a', 'abbr', 'address', 'area', 'article', 'aside', 'audio', 'b', 'bdi',
26 + 'bdo', 'blockquote', 'br', 'button', 'canvas', 'cite', 'code', 'data',
27 + 'datalist', 'del', 'details', 'dfn', 'div', 'dl', 'em', 'embed',
28 + 'fieldset', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6',
29 + 'header', 'hr', 'i', 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen',
30 + 'label', 'link', 'main', 'map', 'mark', 'menu', 'meta', 'meter', 'nav',
31 + 'noscript', 'object', 'ol', 'output', 'p', 'pre', 'progress', 'q', 'ruby',
32 + 's', 'samp', 'script', 'section', 'select', 'small', 'span', 'strong',
33 + 'style', 'sub', 'sup', 'svg', 'table', 'textarea', 'time', 'u', 'ul',
34 + 'var', 'video', 'wbr', '#text'
35 + ];
36 +
37 + // Phrasing elements are inline elements that can appear in a <span>
38 + var phrase = [
39 + 'a', 'abbr', 'area', 'audio', 'b', 'bdi', 'bdo', 'br', 'button', 'canvas',
40 + 'cite', 'code', 'data', 'datalist', 'del', 'dfn', 'em', 'embed', 'i',
41 + 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', 'label', 'link', 'map',
42 + 'mark', 'meta', 'meter', 'noscript', 'object', 'output', 'progress', 'q',
43 + 'ruby', 's', 'samp', 'script', 'select', 'small', 'span', 'strong', 'sub',
44 + 'sup', 'svg', 'textarea', 'time', 'u', 'var', 'video', 'wbr', '#text'
45 + ];
46 +
47 + // Metadata elements can appear in <head>
48 + var metadata = [
49 + 'base', 'link', 'meta', 'noscript', 'script', 'style', 'title'
50 + ];
51 +
52 + // By default, we assume that flow elements can contain other flow elements
53 + // and phrasing elements can contain other phrasing elements. Here are the
54 + // exceptions:
55 + var allowedChildren = {
56 + '#document': ['html'],
57 +
58 + 'a': flow,
59 + 'audio': ['source', 'track'].concat(flow),
60 + 'body': flow,
61 + 'button': phrase,
62 + 'caption': flow,
63 + 'canvas': flow,
64 + 'colgroup': ['col'],
65 + 'dd': flow,
66 + 'del': flow,
67 + 'details': ['summary'].concat(flow),
68 + 'dl': ['dt', 'dd'],
69 + 'dt': flow,
70 + 'fieldset': flow,
71 + 'figcaption': flow,
72 + 'figure': ['figcaption'].concat(flow),
73 + 'h1': phrase,
74 + 'h2': phrase,
75 + 'h3': phrase,
76 + 'h4': phrase,
77 + 'h5': phrase,
78 + 'h6': phrase,
79 + 'head': metadata,
80 + 'hgroup': ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'],
81 + 'html': ['body', 'head'],
82 + 'iframe': [],
83 + 'ins': flow,
84 + 'label': phrase,
85 + 'legend': phrase,
86 + 'li': flow,
87 + 'map': flow,
88 + 'menu': ['li', 'menuitem'].concat(flow),
89 + 'noscript': '*',
90 + 'object': ['param'].concat(flow),
91 + 'ol': ['li'],
92 + 'optgroup': ['option'],
93 + 'p': phrase,
94 + 'pre': phrase,
95 + 'rp': phrase,
96 + 'rt': phrase,
97 + 'ruby': ['rp', 'rt', '#text'],
98 + 'script': ['#text'],
99 + 'select': ['option', 'optgroup'],
100 + 'style': ['#text'],
101 + 'summary': phrase,
102 + 'table': ['caption', 'colgroup', 'tbody', 'tfoot', 'thead'],
103 + 'tbody': ['tr'],
104 + 'td': flow,
105 + 'textarea': ['#text'],
106 + 'tfoot': ['tr'],
107 + 'th': flow,
108 + 'thead': ['tr'],
109 + 'title': ['#text'],
110 + 'tr': ['td', 'th'],
111 + 'ul': ['li'],
112 + 'video': ['source', 'track'].concat(flow),
113 +
114 + // SVG
115 + // TODO: Validate nesting of all svg elements
116 + 'svg': [
117 + 'circle', 'defs', 'g', 'line', 'linearGradient', 'path', 'polygon',
118 + 'polyline', 'radialGradient', 'rect', 'stop', 'text'
119 + ],
120 +
121 + // Self-closing tags
122 + 'area': [],
123 + 'base': [],
124 + 'br': [],
125 + 'col': [],
126 + 'embed': [],
127 + 'hr': [],
128 + 'img': [],
129 + 'input': [],
130 + 'keygen': [],
131 + 'link': [],
132 + 'menuitem': [],
133 + 'meta': [],
134 + 'param': [],
135 + 'source': [],
136 + 'track': [],
137 + 'wbr': []
138 + };
139 +
140 + var i, l;
141 + var allowedChildrenMap = {};
142 + for (i = 0, l = flow.length; i < l; i++) {
143 + allowedChildrenMap[flow[i]] = flow;
144 + }
145 + for (i = 0, l = phrase.length; i < l; i++) {
146 + allowedChildrenMap[phrase[i]] = flow;
147 + }
148 + for (var el in allowedChildren) {
149 + if (allowedChildren.hasOwnProperty(el)) {
150 + allowedChildrenMap[el] = allowedChildren[el];
151 + }
152 + }
153 +
154 + var nodeCanContainNode = function(parentTag, childTag) {
155 + var allowed = allowedChildrenMap[parentTag];
156 + if (!allowed || !allowedChildrenMap[childTag]) {
157 + // We don't recognize one of the tags; err on the side of not warning
158 + return true;
159 + }
160 +
161 + var result = allowed === '*' || allowed.indexOf(childTag) !== -1;
162 + return !!result;
163 + };
164 +
165 + validateDOMNesting = function(parentTag, childTag, element) {
166 + if (!nodeCanContainNode(parentTag, childTag)) {
167 + var info = '';
168 + if (parentTag === 'table' && childTag === 'tr') {
169 + info +=
170 + ' Add a <tbody> to your code to match the DOM tree generated by ' +
171 + 'the browser.';
172 + }
173 + if (element && element._owner) {
174 + var name = element._owner.getName();
175 + if (name) {
176 + info += ` Check the render method of \`${name}\`.`;
177 + }
178 + }
179 +
180 + warning(
181 + false,
182 + 'validateDOMNesting(...): <%s> cannot contain a <%s> node.%s',
183 + parentTag,
184 + childTag,
185 + info
186 + );
187 + }
188 + };
189 +
190 + validateDOMNesting.parentTagContextKey =
191 + '__validateDOMNesting_parentTag$' + Math.random().toString(36).slice(2);
192 +}
193 +
194 +module.exports = validateDOMNesting;
src/test/__tests__/ReactTestUtils-test.js
+31 -4
@@ -208,6 +208,8 @@ describe('ReactTestUtils', function() {
208 });
209
210 it('should support injected wrapper components as DOM components', function() {
211 + var getTestDocument = require('getTestDocument');
212 +
213 var injectedDOMComponents = [
214 'button',
215 'form',
@@ -216,10 +218,7 @@ describe('ReactTestUtils', function() {
218 'input',
219 'option',
220 'select',
219 - 'textarea',
220 - 'html',
221 - 'head',
222 - 'body'
221 + 'textarea'
222 ];
223
224 injectedDOMComponents.forEach(function(type) {
@@ -229,5 +228,33 @@ describe('ReactTestUtils', function() {
228 expect(component.tagName).toBe(type.toUpperCase());
229 expect(ReactTestUtils.isDOMComponent(component)).toBe(true);
230 });
231 +
232 + // Full-page components (html, head, body) can't be rendered into a div
233 + // directly...
234 + var Root = React.createClass({
235 + render: function() {
236 + return (
237 + <html ref="html">
238 + <head ref="head">
239 + <title>hello</title>
240 + </head>
241 + <body ref="body">
242 + hello, world
243 + </body>
244 + </html>
245 + );
246 + }
247 + });
248 +
249 + var markup = React.renderToString(<Root />);
250 + var testDocument = getTestDocument(markup);
251 + var component = React.render(<Root />, testDocument);
252 +
253 + expect(component.refs.html.tagName).toBe('HTML');
254 + expect(component.refs.head.tagName).toBe('HEAD');
255 + expect(component.refs.body.tagName).toBe('BODY');
256 + expect(ReactTestUtils.isDOMComponent(component.refs.html)).toBe(true);
257 + expect(ReactTestUtils.isDOMComponent(component.refs.head)).toBe(true);
258 + expect(ReactTestUtils.isDOMComponent(component.refs.body)).toBe(true);
259 });
260 });