diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index fc3a1e447b..c1c0ae545c 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -29,6 +29,7 @@ var escapeTextContentForBrowser = require('escapeTextContentForBrowser'); var invariant = require('invariant'); var isEventSupported = require('isEventSupported'); var keyOf = require('keyOf'); +var shallowEqual = require('shallowEqual'); var validateDOMNesting = require('validateDOMNesting'); var warning = require('warning'); @@ -43,6 +44,44 @@ var STYLE = keyOf({style: null}); var ELEMENT_NODE_TYPE = 1; +var styleMutationWarning = {}; + +function checkAndWarnForMutatedStyle(style1, style2, component) { + if (style1 == null || style2 == null) { + return; + } + if (shallowEqual(style1, style2)) { + return; + } + + var componentName = component._tag; + var owner = component._currentElement._owner; + var ownerName; + if (owner) { + ownerName = owner.getName(); + } + + var hash = ownerName + '|' + componentName; + + if (styleMutationWarning.hasOwnProperty(hash)) { + return; + } + + styleMutationWarning[hash] = true; + + warning( + false, + '`%s` was passed a style object that has previously been mutated. ' + + 'Mutating `style` will be deprecated. Consider cloning it beforehand. ' + + 'Check the `render` %s. Previous style: %s. Mutated style: %s.', + componentName, + owner ? 'of `' + ownerName + '`' : 'using <' + componentName + '>', + JSON.stringify(style1), + JSON.stringify(style2) + ); +} + + /** * Optionally injectable operations for mutating the DOM */ @@ -201,6 +240,7 @@ function ReactDOMComponent(tag) { validateDangerousTag(tag); this._tag = tag; this._renderedChildren = null; + this._previousStyle = null; this._previousStyleCopy = null; this._rootNodeID = null; } @@ -274,6 +314,10 @@ ReactDOMComponent.Mixin = { } else { if (propKey === STYLE) { if (propValue) { + if (__DEV__) { + // See `_updateDOMProperties`. style block + this._previousStyle = propValue; + } propValue = this._previousStyleCopy = assign({}, props.style); } propValue = CSSPropertyOperations.createMarkupForStyles(propValue); @@ -441,6 +485,14 @@ ReactDOMComponent.Mixin = { } if (propKey === STYLE) { if (nextProp) { + if (__DEV__) { + checkAndWarnForMutatedStyle( + this._previousStyleCopy, + this._previousStyle, + this + ); + this._previousStyle = nextProp; + } nextProp = this._previousStyleCopy = assign({}, nextProp); } if (lastProp) { diff --git a/src/browser/ui/__tests__/ReactDOMComponent-test.js b/src/browser/ui/__tests__/ReactDOMComponent-test.js index de7ad39ba0..378aef828d 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -60,7 +60,11 @@ describe('ReactDOMComponent', function() { expect(stubStyle.fontFamily).toEqual(''); }); + // TODO: (poshannessy) deprecate this pattern. it("should update styles when mutating style object", function() { + // not actually used. Just to suppress the style mutation warning + spyOn(console, 'error'); + var styles = {display: 'none', fontFamily: 'Arial', lineHeight: 1.2}; var container = document.createElement('div'); React.render(
, container); @@ -96,6 +100,46 @@ describe('ReactDOMComponent', function() { expect(stubStyle.lineHeight).toBe(''); }); + it('should warn when mutating style', function() { + spyOn(console, 'error'); + + var style = {border: '1px solid black'}; + var App = React.createClass({ + getInitialState: function() { + return {style: style}; + }, + render: function() { + return