Skip to content

Commit d831436

Browse files
jimfbjim
authored andcommitted
Warn if people mutate children.
1 parent 2282894 commit d831436

File tree

6 files changed

+140
-0
lines changed

6 files changed

+140
-0
lines changed

src/isomorphic/classic/element/ReactElement.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
116116
writable: false,
117117
value: self,
118118
});
119+
Object.defineProperty(element, '_shadowChildren', {
120+
configurable: false,
121+
enumerable: false,
122+
writable: false,
123+
value: Array.isArray(props.children) ? props.children.slice(0) : props.children,
124+
});
119125
// Two elements created in two different places should be considered
120126
// equal for testing purposes and therefore we hide it from enumeration.
121127
Object.defineProperty(element, '_source', {

src/renderers/dom/shared/ReactDOMComponent.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,13 @@ ReactDOMComponent.Mixin = {
713713
break;
714714
}
715715

716+
if (__DEV__) {
717+
if (this._debugID) {
718+
var callback = () => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
719+
transaction.getReactMountReady().enqueue(callback, this);
720+
}
721+
}
722+
716723
return mountImage;
717724
},
718725

@@ -933,6 +940,13 @@ ReactDOMComponent.Mixin = {
933940
// reconciliation
934941
transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
935942
}
943+
944+
if (__DEV__) {
945+
if (this._debugID) {
946+
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
947+
transaction.getReactMountReady().enqueue(callback, this);
948+
}
949+
}
936950
},
937951

938952
/**

src/renderers/shared/ReactDebugTool.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,14 @@ var ReactDebugTool = {
258258
checkDebugID(debugID);
259259
emitEvent('onHostOperation', debugID, type, payload);
260260
},
261+
onComponentHasMounted(debugID) {
262+
checkDebugID(debugID);
263+
emitEvent('onComponentHasMounted', debugID);
264+
},
265+
onComponentHasUpdated(debugID) {
266+
checkDebugID(debugID);
267+
emitEvent('onComponentHasUpdated', debugID);
268+
},
261269
onSetState() {
262270
emitEvent('onSetState');
263271
},
@@ -314,9 +322,11 @@ if (__DEV__) {
314322
var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool');
315323
var ReactHostOperationHistoryDevtool = require('ReactHostOperationHistoryDevtool');
316324
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
325+
var ReactChildrenMutationWarningDevtool = require('ReactChildrenMutationWarningDevtool');
317326
ReactDebugTool.addDevtool(ReactInvalidSetStateWarningDevTool);
318327
ReactDebugTool.addDevtool(ReactComponentTreeDevtool);
319328
ReactDebugTool.addDevtool(ReactHostOperationHistoryDevtool);
329+
ReactDebugTool.addDevtool(ReactChildrenMutationWarningDevtool);
320330
var url = (ExecutionEnvironment.canUseDOM && window.location.href) || '';
321331
if ((/[?&]react_perf\b/).test(url)) {
322332
ReactDebugTool.beginProfiling();
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* Copyright 2013-present, 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 ReactChildrenMutationWarningDevtool
10+
*/
11+
12+
'use strict';
13+
14+
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
15+
16+
var warning = require('warning');
17+
18+
var elements = {};
19+
20+
function handleElement(debugID, element) {
21+
if (element == null) {
22+
return;
23+
}
24+
if (element._shadowChildren === undefined) {
25+
return;
26+
}
27+
if (element._shadowChildren === element.props.children) {
28+
return;
29+
}
30+
var isMutated = false;
31+
if (Array.isArray(element._shadowChildren)) {
32+
if (element._shadowChildren.length === element.props.children.length) {
33+
for (var i = 0; i < element._shadowChildren.length; i++) {
34+
if (element._shadowChildren[i] !== element.props.children[i]) {
35+
isMutated = true;
36+
}
37+
}
38+
} else {
39+
isMutated = true;
40+
}
41+
}
42+
warning(
43+
Array.isArray(element._shadowChildren) && !isMutated,
44+
'Component\'s children should not be mutated.%s',
45+
ReactComponentTreeDevtool.getStackAddendumByID(debugID),
46+
);
47+
}
48+
49+
var ReactDOMUnknownPropertyDevtool = {
50+
onBeforeMountComponent(debugID, element) {
51+
elements[debugID] = element;
52+
},
53+
onBeforeUpdateComponent(debugID, element) {
54+
elements[debugID] = element;
55+
},
56+
onComponentHasMounted(debugID) {
57+
handleElement(debugID, elements[debugID]);
58+
delete elements[debugID];
59+
},
60+
onComponentHasUpdated(debugID) {
61+
handleElement(debugID, elements[debugID]);
62+
delete elements[debugID];
63+
},
64+
};
65+
66+
module.exports = ReactDOMUnknownPropertyDevtool;

src/renderers/shared/stack/reconciler/ReactCompositeComponent.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,13 @@ var ReactCompositeComponentMixin = {
342342
}
343343
}
344344

345+
if (__DEV__) {
346+
if (this._debugID) {
347+
var callback = (component) => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
348+
transaction.getReactMountReady().enqueue(callback, this);
349+
}
350+
}
351+
345352
return markup;
346353
},
347354

@@ -952,6 +959,13 @@ var ReactCompositeComponentMixin = {
952959
);
953960
}
954961
}
962+
963+
if (__DEV__) {
964+
if (this._debugID) {
965+
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
966+
transaction.getReactMountReady().enqueue(callback, this);
967+
}
968+
}
955969
},
956970

957971
/**

src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ var React;
1515
var ReactDOM;
1616
var ReactTestUtils;
1717

18+
function normalizeCodeLocInfo(str) {
19+
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
20+
}
21+
1822
describe('ReactComponent', function() {
1923
beforeEach(function() {
2024
React = require('React');
@@ -45,6 +49,32 @@ describe('ReactComponent', function() {
4549
}).toThrow();
4650
});
4751

52+
it('should warn when children are mutated before render', function() {
53+
spyOn(console, 'error');
54+
var children = [<span key={0} />, <span key={1} />, <span key={2} />];
55+
var element = <div>{children}</div>;
56+
children[1] = <p key={1} />; // Mutation is illegal
57+
ReactTestUtils.renderIntoDocument(element);
58+
expect(console.error.calls.count()).toBe(1);
59+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
60+
'Warning: Component\'s children should not be mutated.\n in div (at **)'
61+
);
62+
});
63+
64+
it('should warn when children are mutated', function() {
65+
spyOn(console, 'error');
66+
var children = [<span key={0} />, <span key={1} />, <span key={2} />];
67+
function Wrapper(props) {
68+
props.children[1] = <p key={1} />; // Mutation is illegal
69+
return <div>{props.children}</div>;
70+
}
71+
ReactTestUtils.renderIntoDocument(<Wrapper>{children}</Wrapper>);
72+
expect(console.error.calls.count()).toBe(1);
73+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
74+
'Warning: Component\'s children should not be mutated.\n in Wrapper (at **)'
75+
);
76+
});
77+
4878
it('should support refs on owned components', function() {
4979
var innerObj = {};
5080
var outerObj = {};

0 commit comments

Comments
 (0)