Skip to content

Commit

Permalink
Warn if people mutate children.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimfb authored and jim committed Jun 15, 2016
1 parent 2282894 commit 3204c58
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/isomorphic/classic/element/ReactElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
// This can be replaced with a WeakMap once they are implemented in
// commonly used development environments.
element._store = {};
var shadowChildren = Array.isArray(props.children) ? props.children.slice(0) : props.children;

// To make comparing ReactElements easier for testing purposes, we make
// the validation flag non-enumerable (where possible, which should
Expand All @@ -116,6 +117,12 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
writable: false,
value: self,
});
Object.defineProperty(element, '_shadowChildren', {
configurable: false,
enumerable: false,
writable: false,
value: shadowChildren,
});
// Two elements created in two different places should be considered
// equal for testing purposes and therefore we hide it from enumeration.
Object.defineProperty(element, '_source', {
Expand All @@ -127,6 +134,7 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
} else {
element._store.validated = false;
element._self = self;
element._shadowChildren = shadowChildren;
element._source = source;
}
if (Object.freeze) {
Expand Down
14 changes: 14 additions & 0 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,13 @@ ReactDOMComponent.Mixin = {
break;
}

if (__DEV__) {
if (this._debugID) {
var callback = () => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
transaction.getReactMountReady().enqueue(callback, this);
}
}

return mountImage;
},

Expand Down Expand Up @@ -933,6 +940,13 @@ ReactDOMComponent.Mixin = {
// reconciliation
transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
}

if (__DEV__) {
if (this._debugID) {
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
transaction.getReactMountReady().enqueue(callback, this);
}
}
},

/**
Expand Down
10 changes: 10 additions & 0 deletions src/renderers/shared/ReactDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,14 @@ var ReactDebugTool = {
checkDebugID(debugID);
emitEvent('onHostOperation', debugID, type, payload);
},
onComponentHasMounted(debugID) {
checkDebugID(debugID);
emitEvent('onComponentHasMounted', debugID);
},
onComponentHasUpdated(debugID) {
checkDebugID(debugID);
emitEvent('onComponentHasUpdated', debugID);
},
onSetState() {
emitEvent('onSetState');
},
Expand Down Expand Up @@ -314,9 +322,11 @@ if (__DEV__) {
var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool');
var ReactHostOperationHistoryDevtool = require('ReactHostOperationHistoryDevtool');
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
var ReactChildrenMutationWarningDevtool = require('ReactChildrenMutationWarningDevtool');
ReactDebugTool.addDevtool(ReactInvalidSetStateWarningDevTool);
ReactDebugTool.addDevtool(ReactComponentTreeDevtool);
ReactDebugTool.addDevtool(ReactHostOperationHistoryDevtool);
ReactDebugTool.addDevtool(ReactChildrenMutationWarningDevtool);
var url = (ExecutionEnvironment.canUseDOM && window.location.href) || '';
if ((/[?&]react_perf\b/).test(url)) {
ReactDebugTool.beginProfiling();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactChildrenMutationWarningDevtool
*/

'use strict';

var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');

var warning = require('warning');

var elements = {};

function handleElement(debugID, element) {
if (element == null) {
return;
}
if (element._shadowChildren === undefined) {
return;
}
if (element._shadowChildren === element.props.children) {
return;
}
var isMutated = false;
if (Array.isArray(element._shadowChildren)) {
if (element._shadowChildren.length === element.props.children.length) {
for (var i = 0; i < element._shadowChildren.length; i++) {
if (element._shadowChildren[i] !== element.props.children[i]) {
isMutated = true;
}
}
} else {
isMutated = true;
}
}
warning(
Array.isArray(element._shadowChildren) && !isMutated,
'Component\'s children should not be mutated.%s',
ReactComponentTreeDevtool.getStackAddendumByID(debugID),
);
}

var ReactDOMUnknownPropertyDevtool = {
onBeforeMountComponent(debugID, element) {
elements[debugID] = element;
},
onBeforeUpdateComponent(debugID, element) {
elements[debugID] = element;
},
onComponentHasMounted(debugID) {
handleElement(debugID, elements[debugID]);
delete elements[debugID];
},
onComponentHasUpdated(debugID) {
handleElement(debugID, elements[debugID]);
delete elements[debugID];
},
};

module.exports = ReactDOMUnknownPropertyDevtool;
14 changes: 14 additions & 0 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,13 @@ var ReactCompositeComponentMixin = {
}
}

if (__DEV__) {
if (this._debugID) {
var callback = (component) => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
transaction.getReactMountReady().enqueue(callback, this);
}
}

return markup;
},

Expand Down Expand Up @@ -952,6 +959,13 @@ var ReactCompositeComponentMixin = {
);
}
}

if (__DEV__) {
if (this._debugID) {
var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
transaction.getReactMountReady().enqueue(callback, this);
}
}
},

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ var React;
var ReactDOM;
var ReactTestUtils;

function normalizeCodeLocInfo(str) {
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
}

describe('ReactComponent', function() {
beforeEach(function() {
React = require('React');
Expand Down Expand Up @@ -45,6 +49,32 @@ describe('ReactComponent', function() {
}).toThrow();
});

it('should warn when children are mutated before render', function() {
spyOn(console, 'error');
var children = [<span key={0} />, <span key={1} />, <span key={2} />];
var element = <div>{children}</div>;
children[1] = <p key={1} />; // Mutation is illegal
ReactTestUtils.renderIntoDocument(element);
expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Component\'s children should not be mutated.\n in div (at **)'
);
});

it('should warn when children are mutated', function() {
spyOn(console, 'error');
var children = [<span key={0} />, <span key={1} />, <span key={2} />];
function Wrapper(props) {
props.children[1] = <p key={1} />; // Mutation is illegal
return <div>{props.children}</div>;
}
ReactTestUtils.renderIntoDocument(<Wrapper>{children}</Wrapper>);
expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Component\'s children should not be mutated.\n in Wrapper (at **)'
);
});

it('should support refs on owned components', function() {
var innerObj = {};
var outerObj = {};
Expand Down

0 comments on commit 3204c58

Please sign in to comment.