Skip to content

Commit 1ea4e1e

Browse files
gaearongaearon
committed
(WIP) Nesting warnings
1 parent e36b38c commit 1ea4e1e

File tree

12 files changed

+334
-157
lines changed

12 files changed

+334
-157
lines changed

‎scripts/fiber/tests-passing-except-dev.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@ src/addons/transitions/__tests__/ReactTransitionGroup-test.js
33

44
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
55
* should not warn when server-side rendering `onScroll`
6-
* warns on invalid nesting
7-
* warns on invalid nesting at root
8-
* warns nicely for table rows
9-
* gives useful context in warnings
106
* should warn about incorrect casing on properties (ssr)
117
* should warn about incorrect casing on event handlers (ssr)
128
* should warn about class (ssr)

‎scripts/fiber/tests-passing.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
690690
* should throw when an attack vector is used server-side
691691
* should throw when an invalid tag name is used
692692
* should throw when an attack vector is used
693+
* warns on invalid nesting
694+
* warns on invalid nesting at root
695+
* warns nicely for table rows
696+
* gives useful context in warnings
693697
* should warn about incorrect casing on properties
694698
* should warn about incorrect casing on event handlers
695699
* should warn about class

‎src/renderers/art/ReactARTFiber.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,10 @@ const ARTRenderer = ReactFiberReconciler({
485485
// Noop
486486
},
487487

488+
getRootHostContext() {
489+
return null;
490+
},
491+
488492
getChildHostContext() {
489493
return null;
490494
},

‎src/renderers/dom/fiber/ReactDOMFiber.js

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ var {
3939
} = ReactDOMFiberComponent;
4040
var { precacheFiberNode } = ReactDOMComponentTree;
4141

42+
if (__DEV__) {
43+
var {
44+
getChildAncestorInfo,
45+
validateElementNesting,
46+
validateTextNesting,
47+
validateInlineTextNesting,
48+
} = ReactDOMFiberComponent;
49+
}
50+
4251
const DOCUMENT_NODE = 9;
4352

4453
ReactDOMInjection.inject();
@@ -61,9 +70,29 @@ let selectionInformation : ?mixed = null;
6170

6271
var DOMRenderer = ReactFiberReconciler({
6372

64-
getChildHostContext(parentHostContext : string | null, type : string) {
65-
const parentNamespace = parentHostContext;
66-
return getChildNamespace(parentNamespace, type);
73+
getRootHostContext(rootContainerInstance) {
74+
const type = rootContainerInstance.tagName.toLowerCase();
75+
if (__DEV__) {
76+
const namespace = getChildNamespace(null, type);
77+
const isMountingIntoDocument = rootContainerInstance.ownerDocument.documentElement === rootContainerInstance;
78+
const ancestorInfo = getChildAncestorInfo(null, isMountingIntoDocument ? '#document' : type);
79+
return {namespace, ancestorInfo};
80+
} else {
81+
return getChildNamespace(null, type);
82+
}
83+
},
84+
85+
getChildHostContext(
86+
parentHostContext : string,
87+
type : string,
88+
) {
89+
if (__DEV__) {
90+
const namespace = getChildNamespace(parentHostContext.namespace, type);
91+
const ancestorInfo = getChildAncestorInfo(parentHostContext.ancestorInfo, type);
92+
return {namespace, ancestorInfo};
93+
} else {
94+
return getChildNamespace(parentHostContext, type);
95+
}
6796
},
6897

6998
prepareForCommit() : void {
@@ -86,7 +115,20 @@ var DOMRenderer = ReactFiberReconciler({
86115
hostContext : string | null,
87116
internalInstanceHandle : Object,
88117
) : Instance {
89-
const domElement : Instance = createElement(type, props, rootContainerInstance, hostContext);
118+
let parentNamespace;
119+
if (__DEV__) {
120+
validateElementNesting(hostContext.ancestorInfo, type);
121+
if (
122+
typeof props.children === 'string' ||
123+
typeof props.children === 'number'
124+
) {
125+
validateInlineTextNesting(hostContext.ancestorInfo, type, String(props.children));
126+
}
127+
parentNamespace = hostContext.namespace;
128+
} else {
129+
parentNamespace = hostContext;
130+
}
131+
const domElement : Instance = createElement(type, props, rootContainerInstance, parentNamespace);
90132
precacheFiberNode(internalInstanceHandle, domElement);
91133
return domElement;
92134
},
@@ -108,8 +150,17 @@ var DOMRenderer = ReactFiberReconciler({
108150
domElement : Instance,
109151
type : string,
110152
oldProps : Props,
111-
newProps : Props
153+
newProps : Props,
154+
hostContext : string
112155
) : boolean {
156+
if (__DEV__) {
157+
if (oldProps.children !== newProps.children && (
158+
typeof newProps.children === 'string' ||
159+
typeof newProps.children === 'number'
160+
)) {
161+
validateInlineTextNesting(hostContext.ancestorInfo, type, String(newProps.children));
162+
}
163+
}
113164
return true;
114165
},
115166

@@ -143,7 +194,15 @@ var DOMRenderer = ReactFiberReconciler({
143194
domElement.textContent = '';
144195
},
145196

146-
createTextInstance(text : string, rootContainerInstance : Container, internalInstanceHandle : Object) : TextInstance {
197+
createTextInstance(
198+
text : string,
199+
rootContainerInstance : Container,
200+
hostContext,
201+
internalInstanceHandle : Object
202+
) : TextInstance {
203+
if (__DEV__) {
204+
validateTextNesting(hostContext.ancestorInfo, text);
205+
}
147206
var textNode : TextInstance = document.createTextNode(text);
148207
precacheFiberNode(internalInstanceHandle, textNode);
149208
return textNode;

‎src/renderers/dom/fiber/ReactDOMFiberComponent.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ if (__DEV__) {
3939
var ReactDOMInvalidARIAHook = require('ReactDOMInvalidARIAHook');
4040
var ReactDOMNullInputValuePropHook = require('ReactDOMNullInputValuePropHook');
4141
var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook');
42+
var validateDOMNesting = require('validateDOMNesting');
43+
var { updatedAncestorInfo } = validateDOMNesting;
4244
var { validateProperties: validateARIAProperties } = ReactDOMInvalidARIAHook;
4345
var { validateProperties: validateInputPropertes } = ReactDOMNullInputValuePropHook;
4446
var { validateProperties: validateUnknownPropertes } = ReactDOMUnknownPropertyHook;
@@ -649,7 +651,7 @@ var ReactDOMFiberComponent = {
649651
tag : string,
650652
lastRawProps : Object,
651653
nextRawProps : Object,
652-
rootContainerElement : Element
654+
rootContainerElement : Element,
653655
) : void {
654656
if (__DEV__) {
655657
validatePropertiesInDevelopment(tag, nextRawProps);
@@ -731,4 +733,20 @@ var ReactDOMFiberComponent = {
731733

732734
};
733735

736+
if (__DEV__) {
737+
ReactDOMFiberComponent.getChildAncestorInfo = function(parentAncestorInfo, type) {
738+
return updatedAncestorInfo(parentAncestorInfo, type, null);
739+
};
740+
ReactDOMFiberComponent.validateElementNesting = function(parentAncestorInfo, type) {
741+
validateDOMNesting(type, null, null, parentAncestorInfo);
742+
};
743+
ReactDOMFiberComponent.validateTextNesting = function(parentAncestorInfo, text) {
744+
validateDOMNesting(null, text, null, parentAncestorInfo);
745+
};
746+
ReactDOMFiberComponent.validateInlineTextNesting = function(parentAncestorInfo, type, text) {
747+
const ownAncestorInfo = updatedAncestorInfo(parentAncestorInfo, type, null);
748+
validateDOMNesting(null, text, null, ownAncestorInfo);
749+
};
750+
}
751+
734752
module.exports = ReactDOMFiberComponent;

‎src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,20 @@
1515
var React = require('React');
1616

1717
var warning = require('warning');
18-
var didWarnInvalidOptionChildren = false;
1918

2019
function flattenChildren(children) {
2120
var content = '';
2221

2322
// Flatten children and warn if they aren't strings or numbers;
2423
// invalid types are ignored.
24+
// We can silently skip them because invalid DOM nesting warning
25+
// catches these cases in Fiber.
2526
React.Children.forEach(children, function(child) {
2627
if (child == null) {
2728
return;
2829
}
2930
if (typeof child === 'string' || typeof child === 'number') {
3031
content += child;
31-
} else if (!didWarnInvalidOptionChildren) {
32-
didWarnInvalidOptionChildren = true;
33-
warning(
34-
false,
35-
'Only strings and numbers are supported as <option> children.'
36-
);
3732
}
3833
});
3934

‎src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

Lines changed: 90 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,10 +1272,15 @@ describe('ReactDOMComponent', () => {
12721272
spyOn(console, 'error');
12731273
ReactTestUtils.renderIntoDocument(<div><tr /><tr /></div>);
12741274

1275+
var addendum = ReactDOMFeatureFlags.useFiber ?
1276+
'\n in tr (at **)' +
1277+
'\n in div (at **)' :
1278+
' See div > tr.';
1279+
12751280
expectDev(console.error.calls.count()).toBe(1);
1276-
expectDev(console.error.calls.argsFor(0)[0]).toBe(
1281+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
12771282
'Warning: validateDOMNesting(...): <tr> cannot appear as a child of ' +
1278-
'<div>. See div > tr.'
1283+
'<div>.' + addendum
12791284
);
12801285
});
12811286

@@ -1284,10 +1289,16 @@ describe('ReactDOMComponent', () => {
12841289
var p = document.createElement('p');
12851290
ReactDOM.render(<span><p /></span>, p);
12861291

1292+
var addendum = ReactDOMFeatureFlags.useFiber ?
1293+
// There is no outer `p` here because root container is not part of the stack.
1294+
'\n in p (at **)' +
1295+
'\n in span (at **)' :
1296+
' See p > ... > p.';
1297+
12871298
expectDev(console.error.calls.count()).toBe(1);
1288-
expectDev(console.error.calls.argsFor(0)[0]).toBe(
1299+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
12891300
'Warning: validateDOMNesting(...): <p> cannot appear as a descendant ' +
1290-
'of <p>. See p > ... > p.'
1301+
'of <p>.' + addendum
12911302
);
12921303
});
12931304

@@ -1307,22 +1318,39 @@ describe('ReactDOMComponent', () => {
13071318
}
13081319

13091320
ReactTestUtils.renderIntoDocument(<Foo />);
1310-
13111321
expectDev(console.error.calls.count()).toBe(3);
1312-
expectDev(console.error.calls.argsFor(0)[0]).toBe(
1322+
1323+
var addendum1 = ReactDOMFeatureFlags.useFiber ?
1324+
'\n in tr (at **)' +
1325+
'\n in Row (at **)' +
1326+
'\n in table (at **)' +
1327+
'\n in Foo (at **)' :
1328+
' See Foo > table > Row > tr.';
1329+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
13131330
'Warning: validateDOMNesting(...): <tr> cannot appear as a child of ' +
1314-
'<table>. See Foo > table > Row > tr. Add a <tbody> to your code to ' +
1315-
'match the DOM tree generated by the browser.'
1331+
'<table>. Add a <tbody> to your code to match the DOM tree generated ' +
1332+
'by the browser.' + addendum1
13161333
);
1317-
expectDev(console.error.calls.argsFor(1)[0]).toBe(
1334+
1335+
var addendum2 = ReactDOMFeatureFlags.useFiber ?
1336+
'\n in tr (at **)' +
1337+
'\n in Row (at **)' +
1338+
'\n in table (at **)' +
1339+
'\n in Foo (at **)' :
1340+
' See Row > tr > #text.';
1341+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
13181342
'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' +
1319-
'child of <tr>. See Row > tr > #text.'
1343+
'child of <tr>.' + addendum2
13201344
);
1321-
expectDev(console.error.calls.argsFor(2)[0]).toBe(
1345+
1346+
var addendum3 = ReactDOMFeatureFlags.useFiber ?
1347+
'\n in table (at **)' +
1348+
'\n in Foo (at **)' :
1349+
' See Foo > table > #text.';
1350+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe(
13221351
'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' +
13231352
'appear as a child of <table>. Make sure you don\'t have any extra ' +
1324-
'whitespace between tags on each line of your source code. See Foo > ' +
1325-
'table > #text.'
1353+
'whitespace between tags on each line of your source code.' + addendum3
13261354
);
13271355
});
13281356

@@ -1355,8 +1383,14 @@ describe('ReactDOMComponent', () => {
13551383
});
13561384
ReactTestUtils.renderIntoDocument(<App1 />);
13571385
expectDev(console.error.calls.count()).toBe(1);
1358-
expectDev(console.error.calls.argsFor(0)[0]).toContain(
1359-
'See Viz1 > table > FancyRow > Row > tr.'
1386+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain(
1387+
ReactDOMFeatureFlags.useFiber ?
1388+
'\n in tr (at **)' +
1389+
'\n in Row (at **)' +
1390+
'\n in FancyRow (at **)' +
1391+
'\n in table (at **)' +
1392+
'\n in Viz1 (at **)' :
1393+
'See Viz1 > table > FancyRow > Row > tr.'
13601394
);
13611395

13621396
var Viz2 = React.createClass({
@@ -1367,26 +1401,51 @@ describe('ReactDOMComponent', () => {
13671401
});
13681402
ReactTestUtils.renderIntoDocument(<App2 />);
13691403
expectDev(console.error.calls.count()).toBe(2);
1370-
expectDev(console.error.calls.argsFor(1)[0]).toContain(
1371-
'See Viz2 > FancyTable > Table > table > FancyRow > Row > tr.'
1404+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toContain(
1405+
ReactDOMFeatureFlags.useFiber ?
1406+
'\n in tr (at **)' +
1407+
'\n in Row (at **)' +
1408+
'\n in FancyRow (at **)' +
1409+
'\n in table (at **)' +
1410+
'\n in Table (at **)' +
1411+
'\n in FancyTable (at **)' +
1412+
'\n in Viz2 (at **)' :
1413+
'See Viz2 > FancyTable > Table > table > FancyRow > Row > tr.'
13721414
);
13731415

13741416
ReactTestUtils.renderIntoDocument(<FancyTable><FancyRow /></FancyTable>);
13751417
expectDev(console.error.calls.count()).toBe(3);
1376-
expectDev(console.error.calls.argsFor(2)[0]).toContain(
1377-
'See FancyTable > Table > table > FancyRow > Row > tr.'
1418+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toContain(
1419+
ReactDOMFeatureFlags.useFiber ?
1420+
'\n in tr (at **)' +
1421+
'\n in Row (at **)' +
1422+
'\n in FancyRow (at **)' +
1423+
'\n in table (at **)' +
1424+
'\n in Table (at **)' +
1425+
'\n in FancyTable (at **)' :
1426+
'See FancyTable > Table > table > FancyRow > Row > tr.'
13781427
);
13791428

13801429
ReactTestUtils.renderIntoDocument(<table><FancyRow /></table>);
13811430
expectDev(console.error.calls.count()).toBe(4);
1382-
expectDev(console.error.calls.argsFor(3)[0]).toContain(
1383-
'See table > FancyRow > Row > tr.'
1431+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(3)[0])).toContain(
1432+
ReactDOMFeatureFlags.useFiber ?
1433+
'\n in tr (at **)' +
1434+
'\n in Row (at **)' +
1435+
'\n in FancyRow (at **)' +
1436+
'\n in table (at **)' :
1437+
'See table > FancyRow > Row > tr.'
13841438
);
13851439

13861440
ReactTestUtils.renderIntoDocument(<FancyTable><tr /></FancyTable>);
13871441
expectDev(console.error.calls.count()).toBe(5);
1388-
expectDev(console.error.calls.argsFor(4)[0]).toContain(
1389-
'See FancyTable > Table > table > tr.'
1442+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(4)[0])).toContain(
1443+
ReactDOMFeatureFlags.useFiber ?
1444+
'\n in tr (at **)' +
1445+
'\n in table (at **)' +
1446+
'\n in Table (at **)' +
1447+
'\n in FancyTable (at **)' :
1448+
'See FancyTable > Table > table > tr.'
13901449
);
13911450

13921451
class Link extends React.Component {
@@ -1397,8 +1456,14 @@ describe('ReactDOMComponent', () => {
13971456

13981457
ReactTestUtils.renderIntoDocument(<Link><div><Link /></div></Link>);
13991458
expectDev(console.error.calls.count()).toBe(6);
1400-
expectDev(console.error.calls.argsFor(5)[0]).toContain(
1401-
'See Link > a > ... > Link > a.'
1459+
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(5)[0])).toContain(
1460+
ReactDOMFeatureFlags.useFiber ?
1461+
'\n in a (at **)' +
1462+
'\n in Link (at **)' +
1463+
'\n in div (at **)' +
1464+
'\n in a (at **)' +
1465+
'\n in Link (at **)' :
1466+
'See Link > a > ... > Link > a.'
14021467
);
14031468
});
14041469

0 commit comments

Comments
 (0)