Warn when mutating props on a ReactElement
Test Plan: jest. Also used ballmer-peak in IE8 to verify that it still works.
Ben Alpert committed
Nov 16, 2014 at 12:47 UTC
a5aacb947f88c7ed95ac301d2a7d2e412f36b956
5 files changed
+167
-3
src/browser/ui/ReactMount.js
+5
@@ -15,6 +15,7 @@ var DOMProperty = require('DOMProperty');
15
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
16
var ReactCurrentOwner = require('ReactCurrentOwner');
17
var ReactElement = require('ReactElement');
18
+var ReactElementValidator = require('ReactElementValidator');
19
var ReactEmptyComponent = require('ReactEmptyComponent');
20
var ReactInstanceHandles = require('ReactInstanceHandles');
21
var ReactInstanceMap = require('ReactInstanceMap');
@@ -291,6 +292,10 @@ var ReactMount = {
292
nextElement,
293
container,
294
callback) {
295
+ if (__DEV__) {
296
+ ReactElementValidator.checkAndWarnForMutatedProps(nextElement);
297
+ }
298
+
299
var nextProps = nextElement.props;
300
ReactMount.scrollMonitor(container, function() {
301
prevComponent.replaceProps(nextProps, callback);
src/classic/element/ReactElement.js
+4
-3
@@ -14,6 +14,7 @@
14
var ReactContext = require('ReactContext');
15
var ReactCurrentOwner = require('ReactCurrentOwner');
16
17
+var assign = require('Object.assign');
18
var warning = require('warning');
19
20
var RESERVED_PROPS = {
@@ -44,8 +45,8 @@ function defineWarningProperty(object, key) {
45
set: function(value) {
46
warning(
47
false,
47
- 'Don\'t set the ' + key + ' property of the component. ' +
48
- 'Mutate the existing props object instead.'
48
+ 'Don\'t set the ' + key + ' property of the React element. Instead, ' +
49
+ 'specify the correct value when initially creating the element.'
50
);
51
this._store[key] = value;
52
}
@@ -106,7 +107,7 @@ var ReactElement = function(type, key, ref, owner, context, props) {
107
// an external backing store so that we can freeze the whole object.
108
// This can be replaced with a WeakMap once they are implemented in
109
// commonly used development environments.
109
- this._store = { props: props };
110
+ this._store = { props: props, originalProps: assign({}, props) };
111
112
// To make comparing ReactElements easier for testing purposes, we make
113
// the validation flag non-enumerable (where possible, which should
src/classic/element/ReactElementValidator.js
+70
@@ -266,8 +266,78 @@ function checkPropTypes(componentName, propTypes, props, location) {
266
}
267
}
268
269
+var warnedPropsMutations = {};
270
+
271
+/**
272
+ * Warn about mutating props when setting `propName` on `element`.
273
+ *
274
+ * @param {string} propName The string key within props that was set
275
+ * @param {ReactElement} element
276
+ */
277
+function warnForPropsMutation(propName, element) {
278
+ var type = element.type;
279
+ var elementName = typeof type === 'string' ? type : type.displayName;
280
+ var ownerName = element._owner ?
281
+ element._owner.getPublicInstance().constructor.displayName : null;
282
+
283
+ var warningKey = propName + '|' + elementName + '|' + ownerName;
284
+ if (warnedPropsMutations.hasOwnProperty(warningKey)) {
285
+ return;
286
+ }
287
+ warnedPropsMutations[warningKey] = true;
288
+
289
+ var elementInfo = '';
290
+ if (elementName) {
291
+ elementInfo = ' <' + elementName + ' />';
292
+ }
293
+ var ownerInfo = '';
294
+ if (ownerName) {
295
+ ownerInfo = ' The element was created by ' + ownerName + '.';
296
+ }
297
+
298
+ warning(
299
+ false,
300
+ 'Don\'t set .props.' + propName + ' of the React component' +
301
+ elementInfo + '. Instead, specify the correct value when ' +
302
+ 'initially creating the element.' + ownerInfo
303
+ );
304
+}
305
+
306
+/**
307
+ * Given an element, check if its props have been mutated since element
308
+ * creation (or the last call to this function). In particular, check if any
309
+ * new props have been added, which we can't directly catch by defining warning
310
+ * properties on the props object.
311
+ *
312
+ * @param {ReactElement} element
313
+ */
314
+function checkAndWarnForMutatedProps(element) {
315
+ if (!element._store) {
316
+ // Element was created using `new ReactElement` directly or with
317
+ // `ReactElement.createElement`; skip mutation checking
318
+ return;
319
+ }
320
+
321
+ var originalProps = element._store.originalProps;
322
+ var props = element.props;
323
+
324
+ for (var propName in props) {
325
+ if (props.hasOwnProperty(propName)) {
326
+ if (!originalProps.hasOwnProperty(propName) ||
327
+ originalProps[propName] !== props[propName]) {
328
+ warnForPropsMutation(propName, element);
329
+
330
+ // Copy over the new value so that the two props objects match again
331
+ originalProps[propName] = props[propName];
332
+ }
333
+ }
334
+ }
335
+}
336
+
337
var ReactElementValidator = {
338
339
+ checkAndWarnForMutatedProps: checkAndWarnForMutatedProps,
340
+
341
createElement: function(type, props, children) {
342
// We warn in this case but don't throw. We expect the element creation to
343
// succeed and there will likely be errors in render.
src/classic/element/__tests__/ReactElement-test.js
+79
@@ -269,4 +269,83 @@ describe('ReactElement', function() {
269
expect(inst2.props.prop).toBe(null);
270
});
271
272
+ it('warns when changing a prop after element creation', function() {
273
+ spyOn(console, 'warn');
274
+ var Outer = React.createClass({
275
+ render: function() {
276
+ var el = <div className="moo" />;
277
+
278
+ // This assignment warns but should still work for now.
279
+ el.props.className = 'quack';
280
+ expect(el.props.className).toBe('quack');
281
+
282
+ return el;
283
+ }
284
+ });
285
+ var outer = ReactTestUtils.renderIntoDocument(<Outer color="orange" />);
286
+ expect(outer.getDOMNode().className).toBe('quack');
287
+
288
+ expect(console.warn.argsForCall.length).toBe(1);
289
+ expect(console.warn.argsForCall[0][0]).toContain(
290
+ 'Don\'t set .props.className of the React component <div />.'
291
+ );
292
+ expect(console.warn.argsForCall[0][0]).toContain(
293
+ 'The element was created by Outer.'
294
+ );
295
+
296
+ console.warn.reset();
297
+
298
+ // This also warns (just once per key/type pair)
299
+ outer.props.color = 'green';
300
+ outer.forceUpdate();
301
+ outer.props.color = 'purple';
302
+ outer.forceUpdate();
303
+
304
+ expect(console.warn.argsForCall.length).toBe(1);
305
+ expect(console.warn.argsForCall[0][0]).toContain(
306
+ 'Don\'t set .props.color of the React component <Outer />.'
307
+ );
308
+ });
309
+
310
+ it('warns when adding a prop after element creation', function() {
311
+ spyOn(console, 'warn');
312
+ var el = document.createElement('div');
313
+ var Outer = React.createClass({
314
+ getDefaultProps: () => ({sound: 'meow'}),
315
+ render: function() {
316
+ var el = <div>{this.props.sound}</div>;
317
+
318
+ // This assignment doesn't warn immediately (because we can't) but it
319
+ // warns upon mount.
320
+ el.props.className = 'quack';
321
+ expect(el.props.className).toBe('quack');
322
+
323
+ return el;
324
+ }
325
+ });
326
+ var outer = React.render(<Outer />, el);
327
+ expect(outer.getDOMNode().textContent).toBe('meow');
328
+ expect(outer.getDOMNode().className).toBe('quack');
329
+
330
+ expect(console.warn.argsForCall.length).toBe(1);
331
+ expect(console.warn.argsForCall[0][0]).toContain(
332
+ 'Don\'t set .props.className of the React component <div />.'
333
+ );
334
+ expect(console.warn.argsForCall[0][0]).toContain(
335
+ 'The element was created by Outer.'
336
+ );
337
+
338
+ console.warn.reset();
339
+
340
+ var newOuterEl = <Outer />;
341
+ newOuterEl.props.sound = 'oink';
342
+ outer = React.render(newOuterEl, el);
343
+ expect(outer.getDOMNode().textContent).toBe('oink');
344
+ expect(outer.getDOMNode().className).toBe('quack');
345
+
346
+ expect(console.warn.argsForCall.length).toBe(1);
347
+ expect(console.warn.argsForCall[0][0]).toContain(
348
+ 'Don\'t set .props.sound of the React component <Outer />.'
349
+ );
350
+ });
351
});
src/core/ReactComponent.js
+9
@@ -11,6 +11,7 @@
11
12
"use strict";
13
14
+var ReactElementValidator = require('ReactElementValidator');
15
var ReactOwner = require('ReactOwner');
16
var ReactRef = require('ReactRef');
17
@@ -110,6 +111,10 @@ var ReactComponent = {
111
* @internal
112
*/
113
mountComponent: function(rootID, transaction, context) {
114
+ if (__DEV__) {
115
+ ReactElementValidator.checkAndWarnForMutatedProps(this._currentElement);
116
+ }
117
+
118
var ref = this._currentElement.ref;
119
if (ref != null) {
120
var owner = this._currentElement._owner;
@@ -144,6 +149,10 @@ var ReactComponent = {
149
* @internal
150
*/
151
updateComponent: function(transaction, prevElement, nextElement, context) {
152
+ if (__DEV__) {
153
+ ReactElementValidator.checkAndWarnForMutatedProps(nextElement);
154
+ }
155
+
156
// If either the owner or a `ref` has changed, make sure the newest owner
157
// has stored a reference to `this`, and the previous owner (if different)
158
// has forgotten the reference to `this`. We use the element instead