{"parents":["4dbccea975866e48ce45c23bc2cb30c8c61b5d3c","6e992fbbddbdcafdd9510897a770f4ef913f7555"],"patch":"diff --git a\/src\/browser\/ui\/ReactDOMComponent.js b\/src\/browser\/ui\/ReactDOMComponent.js\nindex 2b1f592e13..40fcf8e396 100644\n--- a\/src\/browser\/ui\/ReactDOMComponent.js\n+++ b\/src\/browser\/ui\/ReactDOMComponent.js\n@@ -29,6 +29,7 @@ var escapeTextContentForBrowser = require('escapeTextContentForBrowser');\n var invariant = require('invariant');\n var isEventSupported = require('isEventSupported');\n var keyOf = require('keyOf');\n+var shallowEqual = require('shallowEqual');\n var validateDOMNesting = require('validateDOMNesting');\n var warning = require('warning');\n \n@@ -43,6 +44,44 @@ var STYLE = keyOf({style: null});\n \n var ELEMENT_NODE_TYPE = 1;\n \n+var styleMutationWarning = {};\n+\n+function checkAndWarnForMutatedStyle(style1, style2, component) {\n+ if (style1 == null || style2 == null) {\n+ return;\n+ }\n+ if (shallowEqual(style1, style2)) {\n+ return;\n+ }\n+\n+ var componentName = component._tag;\n+ var owner = component._currentElement._owner;\n+ var ownerName;\n+ if (owner) {\n+ ownerName = owner.getName();\n+ }\n+\n+ var hash = ownerName + '|' + componentName;\n+\n+ if (styleMutationWarning.hasOwnProperty(hash)) {\n+ return;\n+ }\n+\n+ styleMutationWarning[hash] = true;\n+\n+ warning(\n+ false,\n+ '`%s` was passed a style object that has previously been mutated. ' +\n+ 'Mutating `style` will be deprecated. Consider cloning it beforehand. ' +\n+ 'Check the `render` %s. Previous style: %s. Mutated style: %s.',\n+ componentName,\n+ owner ? 'of `' + ownerName + '`' : 'using <' + componentName + '>',\n+ JSON.stringify(style1),\n+ JSON.stringify(style2)\n+ );\n+}\n+\n+\n \/**\n * Optionally injectable operations for mutating the DOM\n *\/\n@@ -211,6 +250,7 @@ function ReactDOMComponent(tag) {\n validateDangerousTag(tag);\n this._tag = tag;\n this._renderedChildren = null;\n+ this._previousStyle = null;\n this._previousStyleCopy = null;\n this._rootNodeID = null;\n }\n@@ -284,6 +324,10 @@ ReactDOMComponent.Mixin = {\n } else {\n if (propKey === STYLE) {\n if (propValue) {\n+ if (__DEV__) {\n+ \/\/ See `_updateDOMProperties`. style block\n+ this._previousStyle = propValue;\n+ }\n propValue = this._previousStyleCopy = assign({}, props.style);\n }\n propValue = CSSPropertyOperations.createMarkupForStyles(propValue);\n@@ -451,6 +495,14 @@ ReactDOMComponent.Mixin = {\n }\n if (propKey === STYLE) {\n if (nextProp) {\n+ if (__DEV__) {\n+ checkAndWarnForMutatedStyle(\n+ this._previousStyleCopy,\n+ this._previousStyle,\n+ this\n+ );\n+ this._previousStyle = nextProp;\n+ }\n nextProp = this._previousStyleCopy = assign({}, nextProp);\n } else {\n this._previousStyleCopy = null;\ndiff --git a\/src\/browser\/ui\/__tests__\/ReactDOMComponent-test.js b\/src\/browser\/ui\/__tests__\/ReactDOMComponent-test.js\nindex f69f369f55..961cea166b 100644\n--- a\/src\/browser\/ui\/__tests__\/ReactDOMComponent-test.js\n+++ b\/src\/browser\/ui\/__tests__\/ReactDOMComponent-test.js\n@@ -60,7 +60,11 @@ describe('ReactDOMComponent', function() {\n expect(stubStyle.fontFamily).toEqual('');\n });\n \n+ \/\/ TODO: (poshannessy) deprecate this pattern.\n it(\"should update styles when mutating style object\", function() {\n+ \/\/ not actually used. Just to suppress the style mutation warning\n+ spyOn(console, 'error');\n+\n var styles = {display: 'none', fontFamily: 'Arial', lineHeight: 1.2};\n var container = document.createElement('div');\n React.render(

, container);\n@@ -96,6 +100,46 @@ describe('ReactDOMComponent', function() {\n expect(stubStyle.lineHeight).toBe('');\n });\n \n+ it('should warn when mutating style', function() {\n+ spyOn(console, 'error');\n+\n+ var style = {border: '1px solid black'};\n+ var App = React.createClass({\n+ getInitialState: function() {\n+ return {style: style};\n+ },\n+ render: function() {\n+ return

asd<\/div>;\n+ }\n+ });\n+\n+ var stub = ReactTestUtils.renderIntoDocument();\n+ style.position = 'absolute';\n+ stub.setState({style: style});\n+ expect(console.error.argsForCall.length).toBe(1);\n+ expect(console.error.argsForCall[0][0]).toEqual(\n+ 'Warning: `div` was passed a style object that has previously been ' +\n+ 'mutated. Mutating `style` will be deprecated. Consider cloning it ' +\n+ 'beforehand. Check the `render` of `App`. Previous style: ' +\n+ '{\"border\":\"1px solid black\"}. Mutated style: ' +\n+ '{\"border\":\"1px solid black\",\"position\":\"absolute\"}.'\n+ );\n+\n+ style = {background: 'red'};\n+ stub = ReactTestUtils.renderIntoDocument();\n+ style.background = 'green';\n+ stub.setState({style: {background: 'green'}});\n+ \/\/ already warned once for the same component and owner\n+ expect(console.error.argsForCall.length).toBe(1);\n+\n+ style = {background: 'red'};\n+ var div = document.createElement('div');\n+ React.render(<\/span>, div);\n+ style.background = 'blue';\n+ React.render(<\/span>, div);\n+ expect(console.error.argsForCall.length).toBe(2);\n+ });\n+\n it(\"should update styles if initially null\", function() {\n var styles = null;\n var container = document.createElement('div');\n@@ -173,6 +217,7 @@ describe('ReactDOMComponent', function() {\n expect(stubStyle.color).toEqual('');\n });\n \n+ \/\/ trouble\n it(\"should update styles when 'style' changes from null to object\", function() {\n var container = document.createElement('div');\n var styles = {color: 'red'};\n","commit":{"revision":"288c7ed1393f45ee254a8ce9b2b05015822bbe52","shortRevision":"288c7ed139","authorName":"Cheng Lou","authorEmail":"chenglou92@gmail.com","subject":"Merge pull request #2027 from chenglou\/warn-style-mutation","authoredAt":"2015-04-24T14:51:33-04:00"},"files":[{"status":"M","path":"src\/browser\/ui\/ReactDOMComponent.js"},{"status":"M","path":"src\/browser\/ui\/__tests__\/ReactDOMComponent-test.js"}],"body":"Set up `style` mutation monitoring"}

dillon1000/react

Files changed