Skip to content

Commit b688bb3

Browse files
jimfbzpao
authored andcommitted
Warn if people mutate children. (#7001)
(cherry picked from commit 49238b9)
1 parent 9138b45 commit b688bb3

File tree

6 files changed

+142
-0
lines changed

6 files changed

+142
-0
lines changed

src/isomorphic/classic/element/ReactElement.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
9797
// This can be replaced with a WeakMap once they are implemented in
9898
// commonly used development environments.
9999
element._store = {};
100+
var shadowChildren = Array.isArray(props.children) ? props.children.slice(0) : props.children;
100101

101102
// To make comparing ReactElements easier for testing purposes, we make
102103
// the validation flag non-enumerable (where possible, which should
@@ -116,6 +117,12 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
116117
writable: false,
117118
value: self,
118119
});
120+
Object.defineProperty(element, '_shadowChildren', {
121+
configurable: false,
122+
enumerable: false,
123+
writable: false,
124+
value: shadowChildren,
125+
});
119126
// Two elements created in two different places should be considered
120127
// equal for testing purposes and therefore we hide it from enumeration.
121128
Object.defineProperty(element, '_source', {
@@ -127,6 +134,7 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
127134
} else {
128135
element._store.validated = false;
129136
element._self = self;
137+
element._shadowChildren = shadowChildren;
130138
element._source = source;
131139
}
132140
if (Object.freeze) {

src/renderers/dom/shared/ReactDOMComponent.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,13 @@ ReactDOMComponent.Mixin = {
706706
break;
707707
}
708708

709+
if (__DEV__) {
710+
if (this._debugID) {
711+
var callback = () => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
712+
transaction.getReactMountReady().enqueue(callback, this);
713+
}
714+
}
715+
709716
return mountImage;
710717
},
711718

@@ -926,6 +933,13 @@ ReactDOMComponent.Mixin = {
926933
// reconciliation
927934
transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
928935
}
936+
937+
if (__DEV__) {
938+
if (this._debugID) {
939+
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
940+
transaction.getReactMountReady().enqueue(callback, this);
941+
}
942+
}
929943
},
930944

931945
/**

src/renderers/shared/ReactDebugTool.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool');
1515
var ReactHostOperationHistoryDevtool = require('ReactHostOperationHistoryDevtool');
1616
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
17+
var ReactChildrenMutationWarningDevtool = require('ReactChildrenMutationWarningDevtool');
1718
var ExecutionEnvironment = require('ExecutionEnvironment');
1819

1920
var performanceNow = require('performanceNow');
@@ -255,6 +256,14 @@ var ReactDebugTool = {
255256
checkDebugID(debugID);
256257
emitEvent('onHostOperation', debugID, type, payload);
257258
},
259+
onComponentHasMounted(debugID) {
260+
checkDebugID(debugID);
261+
emitEvent('onComponentHasMounted', debugID);
262+
},
263+
onComponentHasUpdated(debugID) {
264+
checkDebugID(debugID);
265+
emitEvent('onComponentHasUpdated', debugID);
266+
},
258267
onSetState() {
259268
emitEvent('onSetState');
260269
},
@@ -310,6 +319,7 @@ var ReactDebugTool = {
310319

311320
ReactDebugTool.addDevtool(ReactInvalidSetStateWarningDevTool);
312321
ReactDebugTool.addDevtool(ReactComponentTreeDevtool);
322+
ReactDebugTool.addDevtool(ReactChildrenMutationWarningDevtool);
313323
var url = (ExecutionEnvironment.canUseDOM && window.location.href) || '';
314324
if ((/[?&]react_perf\b/).test(url)) {
315325
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
@@ -343,6 +343,13 @@ var ReactCompositeComponentMixin = {
343343
}
344344
}
345345

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

@@ -960,6 +967,13 @@ var ReactCompositeComponentMixin = {
960967
);
961968
}
962969
}
970+
971+
if (__DEV__) {
972+
if (this._debugID) {
973+
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
974+
transaction.getReactMountReady().enqueue(callback, this);
975+
}
976+
}
963977
},
964978

965979
/**

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)