From 31ebcb7975e09579acf004c037ad51e87f808f7c Mon Sep 17 00:00:00 2001 From: Jim Date: Wed, 8 Jun 2016 12:51:08 -0700 Subject: [PATCH] Warn if people mutate children. --- .../classic/element/ReactElement.js | 6 ++ src/renderers/shared/ReactDebugTool.js | 2 + .../ReactChildrenMutationWarningDevtool.js | 56 +++++++++++++++++++ .../__tests__/ReactComponent-test.js | 16 ++++++ 4 files changed, 80 insertions(+) create mode 100644 src/renderers/shared/devtools/ReactChildrenMutationWarningDevtool.js diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js index 86ca0c9aa5332..d3f3d2a657309 100644 --- a/src/isomorphic/classic/element/ReactElement.js +++ b/src/isomorphic/classic/element/ReactElement.js @@ -116,6 +116,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: Array.isArray(props.children) ? props.children.slice(0) : props.children, + }); // 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', { diff --git a/src/renderers/shared/ReactDebugTool.js b/src/renderers/shared/ReactDebugTool.js index 6deb84bc85be6..270932c971992 100644 --- a/src/renderers/shared/ReactDebugTool.js +++ b/src/renderers/shared/ReactDebugTool.js @@ -314,9 +314,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(); diff --git a/src/renderers/shared/devtools/ReactChildrenMutationWarningDevtool.js b/src/renderers/shared/devtools/ReactChildrenMutationWarningDevtool.js new file mode 100644 index 0000000000000..e465260370cb0 --- /dev/null +++ b/src/renderers/shared/devtools/ReactChildrenMutationWarningDevtool.js @@ -0,0 +1,56 @@ +/** + * 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'); + +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) { + handleElement(debugID, element); + }, + onBeforeUpdateComponent(debugID, element) { + handleElement(debugID, element); + }, +}; + +module.exports = ReactDOMUnknownPropertyDevtool; diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js index cd0c13d6e5f92..4c6fb26599041 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js @@ -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'); @@ -45,6 +49,18 @@ describe('ReactComponent', function() { }).toThrow(); }); + it('should warn when children are mutated', function() { + spyOn(console, 'error'); + var children = [, , ]; + var element =
{children}
; + children[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 support refs on owned components', function() { var innerObj = {}; var outerObj = {};