Freeze ReactElement.props in dev mode
Jim committed
Jun 18, 2015 at 14:33 UTC
95373ce769188aacd25c6485bc2c628ff02a856f
8 files changed
+56
-252
src/isomorphic/classic/element/ReactElement.js
+8
-76
@@ -14,71 +14,12 @@
14
var ReactCurrentOwner = require('ReactCurrentOwner');
15
16
var assign = require('Object.assign');
17
-var warning = require('warning');
17
18
var RESERVED_PROPS = {
19
key: true,
20
ref: true,
21
};
22
24
-/**
25
- * Warn for mutations.
26
- *
27
- * @internal
28
- * @param {object} object
29
- * @param {string} key
30
- */
31
-function defineWarningProperty(object, key) {
32
- Object.defineProperty(object, key, {
33
-
34
- configurable: false,
35
- enumerable: true,
36
-
37
- get: function() {
38
- if (!this._store) {
39
- return null;
40
- }
41
- return this._store[key];
42
- },
43
-
44
- set: function(value) {
45
- warning(
46
- false,
47
- 'Don\'t set the %s property of the React element. Instead, ' +
48
- 'specify the correct value when initially creating the element.',
49
- key
50
- );
51
- this._store[key] = value;
52
- },
53
-
54
- });
55
-}
56
-
57
-/**
58
- * This is updated to true if the membrane is successfully created.
59
- */
60
-var useMutationMembrane = false;
61
-
62
-/**
63
- * Warn for mutations.
64
- *
65
- * @internal
66
- * @param {object} prototype
67
- */
68
-function defineMutationMembrane(prototype) {
69
- try {
70
- var pseudoFrozenProperties = {
71
- props: true,
72
- };
73
- for (var key in pseudoFrozenProperties) {
74
- defineWarningProperty(prototype, key);
75
- }
76
- useMutationMembrane = true;
77
- } catch (x) {
78
- // IE will fail on defineProperty
79
- }
80
-}
81
-
23
/**
24
* Base constructor for all React elements. This is only used to make this
25
* work with a dynamic instanceof check. Nothing should live on this prototype.
@@ -100,11 +41,11 @@ var ReactElement = function(type, key, ref, owner, props) {
41
this._owner = owner;
42
43
if (__DEV__) {
103
- // The validation flag and props are currently mutative. We put them on
44
+ // The validation flag is currently mutative. We put it on
45
// an external backing store so that we can freeze the whole object.
46
// This can be replaced with a WeakMap once they are implemented in
47
// commonly used development environments.
107
- this._store = {props: props, originalProps: assign({}, props)};
48
+ this._store = {};
49
50
// To make comparing ReactElements easier for testing purposes, we make
51
// the validation flag non-enumerable (where possible, which should
@@ -115,21 +56,15 @@ var ReactElement = function(type, key, ref, owner, props) {
56
configurable: false,
57
enumerable: false,
58
writable: true,
59
+ value: false,
60
});
61
} catch (x) {
62
+ this._store.validated = false;
63
}
121
- this._store.validated = false;
122
-
123
- // We're not allowed to set props directly on the object so we early
124
- // return and rely on the prototype membrane to forward to the backing
125
- // store.
126
- if (useMutationMembrane) {
127
- Object.freeze(this);
128
- return;
129
- }
64
+ this.props = props;
65
+ Object.freeze(this.props);
66
+ Object.freeze(this);
67
}
131
-
132
- this.props = props;
68
};
69
70
// We intentionally don't expose the function on the constructor property.
@@ -138,10 +73,6 @@ ReactElement.prototype = {
73
_isReactElement: true,
74
};
75
141
-if (__DEV__) {
142
- defineMutationMembrane(ReactElement.prototype);
143
-}
144
-
76
ReactElement.createElement = function(type, config, children) {
77
var propName;
78
@@ -219,6 +150,7 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) {
150
// If the key on the original is valid, then the clone is valid
151
newElement._store.validated = oldElement._store.validated;
152
}
153
+
154
return newElement;
155
};
156
src/isomorphic/classic/element/ReactElementValidator.js
-86
@@ -286,90 +286,6 @@ function checkPropTypes(componentName, propTypes, props, location) {
286
}
287
}
288
289
-var warnedPropsMutations = {};
290
-
291
-/**
292
- * Warn about mutating props when setting `propName` on `element`.
293
- *
294
- * @param {string} propName The string key within props that was set
295
- * @param {ReactElement} element
296
- */
297
-function warnForPropsMutation(propName, element) {
298
- var type = element.type;
299
- var elementName = typeof type === 'string' ? type : type.displayName;
300
- var ownerName = element._owner ?
301
- element._owner.getPublicInstance().constructor.displayName : null;
302
-
303
- var warningKey = propName + '|' + elementName + '|' + ownerName;
304
- if (warnedPropsMutations.hasOwnProperty(warningKey)) {
305
- return;
306
- }
307
- warnedPropsMutations[warningKey] = true;
308
-
309
- var elementInfo = '';
310
- if (elementName) {
311
- elementInfo = ' <' + elementName + ' />';
312
- }
313
- var ownerInfo = '';
314
- if (ownerName) {
315
- ownerInfo = ' The element was created by ' + ownerName + '.';
316
- }
317
-
318
- warning(
319
- false,
320
- 'Don\'t set .props.%s of the React component%s. Instead, specify the ' +
321
- 'correct value when initially creating the element or use ' +
322
- 'React.cloneElement to make a new element with updated props.%s',
323
- propName,
324
- elementInfo,
325
- ownerInfo
326
- );
327
-}
328
-
329
-// Inline Object.is polyfill
330
-function is(a, b) {
331
- if (a !== a) {
332
- // NaN
333
- return b !== b;
334
- }
335
- if (a === 0 && b === 0) {
336
- // +-0
337
- return 1 / a === 1 / b;
338
- }
339
- return a === b;
340
-}
341
-
342
-/**
343
- * Given an element, check if its props have been mutated since element
344
- * creation (or the last call to this function). In particular, check if any
345
- * new props have been added, which we can't directly catch by defining warning
346
- * properties on the props object.
347
- *
348
- * @param {ReactElement} element
349
- */
350
-function checkAndWarnForMutatedProps(element) {
351
- if (!element._store) {
352
- // Element was created using `new ReactElement` directly or with
353
- // `ReactElement.createElement`; skip mutation checking
354
- return;
355
- }
356
-
357
- var originalProps = element._store.originalProps;
358
- var props = element.props;
359
-
360
- for (var propName in props) {
361
- if (props.hasOwnProperty(propName)) {
362
- if (!originalProps.hasOwnProperty(propName) ||
363
- !is(originalProps[propName], props[propName])) {
364
- warnForPropsMutation(propName, element);
365
-
366
- // Copy over the new value so that the two props objects match again
367
- originalProps[propName] = props[propName];
368
- }
369
- }
370
- }
371
-}
372
-
289
/**
290
* Given an element, validate that its props follow the propTypes definition,
291
* provided by the type.
@@ -409,8 +325,6 @@ function validatePropTypes(element) {
325
326
var ReactElementValidator = {
327
412
- checkAndWarnForMutatedProps: checkAndWarnForMutatedProps,
413
-
328
createElement: function(type, props, children) {
329
// We warn in this case but don't throw. We expect the element creation to
330
// succeed and there will likely be errors in render.
src/isomorphic/classic/element/__tests__/ReactElement-test.js
+25
-59
@@ -37,7 +37,9 @@ describe('ReactElement', function() {
37
expect(element.type).toBe(ComponentClass);
38
expect(element.key).toBe(null);
39
expect(element.ref).toBe(null);
40
- expect(element.props).toEqual({});
40
+ var expectation = {};
41
+ Object.freeze(expectation);
42
+ expect(element.props).toEqual(expectation);
43
});
44
45
it('allows a string to be passed as the type', function() {
@@ -45,7 +47,9 @@ describe('ReactElement', function() {
47
expect(element.type).toBe('div');
48
expect(element.key).toBe(null);
49
expect(element.ref).toBe(null);
48
- expect(element.props).toEqual({});
50
+ var expectation = {};
51
+ Object.freeze(expectation);
52
+ expect(element.props).toEqual(expectation);
53
});
54
55
it('returns an immutable element', function() {
@@ -70,7 +74,9 @@ describe('ReactElement', function() {
74
expect(element.type).toBe(ComponentClass);
75
expect(element.key).toBe('12');
76
expect(element.ref).toBe('34');
73
- expect(element.props).toEqual({foo:'56'});
77
+ var expectation = {foo:'56'};
78
+ Object.freeze(expectation);
79
+ expect(element.props).toEqual(expectation);
80
});
81
82
it('coerces the key to a string', function() {
@@ -81,7 +87,9 @@ describe('ReactElement', function() {
87
expect(element.type).toBe(ComponentClass);
88
expect(element.key).toBe('12');
89
expect(element.ref).toBe(null);
84
- expect(element.props).toEqual({foo:'56'});
90
+ var expectation = {foo:'56'};
91
+ Object.freeze(expectation);
92
+ expect(element.props).toEqual(expectation);
93
});
94
95
it('preserves the owner on the element', function() {
@@ -243,84 +251,42 @@ describe('ReactElement', function() {
251
expect(inst2.props.prop).toBe(null);
252
});
253
246
- it('warns when changing a prop after element creation', function() {
247
- spyOn(console, 'error');
254
+ it('throws when changing a prop (in dev) after element creation', function() {
255
var Outer = React.createClass({
256
render: function() {
257
var el = <div className="moo" />;
258
252
- // This assignment warns but should still work for now.
253
- el.props.className = 'quack';
254
- expect(el.props.className).toBe('quack');
259
+ expect(function() {
260
+ el.props.className = 'quack';
261
+ }).toThrow();
262
+ expect(el.props.className).toBe('moo');
263
264
return el;
265
},
266
});
267
var outer = ReactTestUtils.renderIntoDocument(<Outer color="orange" />);
260
- expect(React.findDOMNode(outer).className).toBe('quack');
261
-
262
- expect(console.error.argsForCall.length).toBe(1);
263
- expect(console.error.argsForCall[0][0]).toContain(
264
- 'Don\'t set .props.className of the React component <div />.'
265
- );
266
- expect(console.error.argsForCall[0][0]).toContain(
267
- 'The element was created by Outer.'
268
- );
269
-
270
- console.error.reset();
271
-
272
- // This also warns (just once per key/type pair)
273
- outer.props.color = 'green';
274
- outer.forceUpdate();
275
- outer.props.color = 'purple';
276
- outer.forceUpdate();
277
-
278
- expect(console.error.argsForCall.length).toBe(1);
279
- expect(console.error.argsForCall[0][0]).toContain(
280
- 'Don\'t set .props.color of the React component <Outer />.'
281
- );
268
+ expect(React.findDOMNode(outer).className).toBe('moo');
269
});
270
284
- it('warns when adding a prop after element creation', function() {
285
- spyOn(console, 'error');
271
+ it('throws when adding a prop (in dev) after element creation', function() {
272
var container = document.createElement('div');
273
var Outer = React.createClass({
274
getDefaultProps: () => ({sound: 'meow'}),
275
render: function() {
276
var el = <div>{this.props.sound}</div>;
277
292
- // This assignment doesn't warn immediately (because we can't) but it
293
- // warns upon mount.
294
- el.props.className = 'quack';
295
- expect(el.props.className).toBe('quack');
278
+ expect(function() {
279
+ el.props.className = 'quack';
280
+ }).toThrow();
281
+
282
+ expect(el.props.className).toBe(undefined);
283
284
return el;
285
},
286
});
287
var outer = React.render(<Outer />, container);
288
expect(React.findDOMNode(outer).textContent).toBe('meow');
302
- expect(React.findDOMNode(outer).className).toBe('quack');
303
-
304
- expect(console.error.argsForCall.length).toBe(1);
305
- expect(console.error.argsForCall[0][0]).toContain(
306
- 'Don\'t set .props.className of the React component <div />.'
307
- );
308
- expect(console.error.argsForCall[0][0]).toContain(
309
- 'The element was created by Outer.'
310
- );
311
-
312
- console.error.reset();
313
-
314
- var newOuterEl = <Outer />;
315
- newOuterEl.props.sound = 'oink';
316
- outer = React.render(newOuterEl, container);
317
- expect(React.findDOMNode(outer).textContent).toBe('oink');
318
- expect(React.findDOMNode(outer).className).toBe('quack');
319
-
320
- expect(console.error.argsForCall.length).toBe(1);
321
- expect(console.error.argsForCall[0][0]).toContain(
322
- 'Don\'t set .props.sound of the React component <Outer />.'
323
- );
289
+ expect(React.findDOMNode(outer).className).toBe('');
290
});
291
292
it('does not warn for NaN props', function() {
src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
+8
-4
@@ -16,6 +16,10 @@ var React;
16
describe('ReactES6Class', function() {
17
18
var container;
19
+ var freeze = function(expectation) {
20
+ Object.freeze(expectation);
21
+ return expectation;
22
+ };
23
var Inner;
24
var attachedListener = null;
25
var renderedName = null;
@@ -288,10 +292,10 @@ describe('ReactES6Class', function() {
292
lifeCycles = []; // reset
293
test(<Foo value="bar" />, 'SPAN', 'bar');
294
expect(lifeCycles).toEqual([
291
- 'receive-props', {value: 'bar'},
292
- 'should-update', {value: 'bar'}, {},
293
- 'will-update', {value: 'bar'}, {},
294
- 'did-update', {value: 'foo'}, {},
295
+ 'receive-props', freeze({value: 'bar'}),
296
+ 'should-update', freeze({value: 'bar'}), {},
297
+ 'will-update', freeze({value: 'bar'}), {},
298
+ 'did-update', freeze({value: 'foo'}), {},
299
]);
300
lifeCycles = []; // reset
301
React.unmountComponentAtNode(container);
src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js
+15
-5
@@ -34,7 +34,9 @@ describe('ReactJSXElement', function() {
34
expect(element.type).toBe(Component);
35
expect(element.key).toBe(null);
36
expect(element.ref).toBe(null);
37
- expect(element.props).toEqual({});
37
+ var expectation = {};
38
+ Object.freeze(expectation);
39
+ expect(element.props).toEqual(expectation);
40
});
41
42
it('allows a lower-case to be passed as the string type', function() {
@@ -42,7 +44,9 @@ describe('ReactJSXElement', function() {
44
expect(element.type).toBe('div');
45
expect(element.key).toBe(null);
46
expect(element.ref).toBe(null);
45
- expect(element.props).toEqual({});
47
+ var expectation = {};
48
+ Object.freeze(expectation);
49
+ expect(element.props).toEqual(expectation);
50
});
51
52
it('allows a string to be passed as the type', function() {
@@ -51,7 +55,9 @@ describe('ReactJSXElement', function() {
55
expect(element.type).toBe('div');
56
expect(element.key).toBe(null);
57
expect(element.ref).toBe(null);
54
- expect(element.props).toEqual({});
58
+ var expectation = {};
59
+ Object.freeze(expectation);
60
+ expect(element.props).toEqual(expectation);
61
});
62
63
it('returns an immutable element', function() {
@@ -72,7 +78,9 @@ describe('ReactJSXElement', function() {
78
expect(element.type).toBe(Component);
79
expect(element.key).toBe('12');
80
expect(element.ref).toBe('34');
75
- expect(element.props).toEqual({foo:'56'});
81
+ var expectation = {foo:'56'};
82
+ Object.freeze(expectation);
83
+ expect(element.props).toEqual(expectation);
84
});
85
86
it('coerces the key to a string', function() {
@@ -80,7 +88,9 @@ describe('ReactJSXElement', function() {
88
expect(element.type).toBe(Component);
89
expect(element.key).toBe('12');
90
expect(element.ref).toBe(null);
83
- expect(element.props).toEqual({foo:'56'});
91
+ var expectation = {foo:'56'};
92
+ Object.freeze(expectation);
93
+ expect(element.props).toEqual(expectation);
94
});
95
96
it('merges JSX children onto the children prop', function() {
src/renderers/dom/client/ReactMount.js
-5
@@ -15,7 +15,6 @@ var DOMProperty = require('DOMProperty');
15
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
16
var ReactCurrentOwner = require('ReactCurrentOwner');
17
var ReactElement = require('ReactElement');
18
-var ReactElementValidator = require('ReactElementValidator');
18
var ReactEmptyComponent = require('ReactEmptyComponent');
19
var ReactInstanceHandles = require('ReactInstanceHandles');
20
var ReactInstanceMap = require('ReactInstanceMap');
@@ -375,10 +374,6 @@ var ReactMount = {
374
nextElement,
375
container,
376
callback) {
378
- if (__DEV__) {
379
- ReactElementValidator.checkAndWarnForMutatedProps(nextElement);
380
- }
381
-
377
ReactMount.scrollMonitor(container, function() {
378
ReactUpdateQueue.enqueueElementInternal(prevComponent, nextElement);
379
if (callback) {
src/renderers/shared/reconciler/ReactCompositeComponent.js
-7
@@ -14,7 +14,6 @@
14
var ReactComponentEnvironment = require('ReactComponentEnvironment');
15
var ReactCurrentOwner = require('ReactCurrentOwner');
16
var ReactElement = require('ReactElement');
17
-var ReactElementValidator = require('ReactElementValidator');
17
var ReactInstanceMap = require('ReactInstanceMap');
18
var ReactLifeCycle = require('ReactLifeCycle');
19
var ReactNativeComponent = require('ReactNativeComponent');
@@ -503,12 +502,6 @@ var ReactCompositeComponentMixin = {
502
}
503
504
if (this._pendingStateQueue !== null || this._pendingForceUpdate) {
506
- if (__DEV__) {
507
- ReactElementValidator.checkAndWarnForMutatedProps(
508
- this._currentElement
509
- );
510
- }
511
-
505
this.updateComponent(
506
transaction,
507
this._currentElement,
src/renderers/shared/reconciler/ReactReconciler.js
-10
@@ -12,7 +12,6 @@
12
'use strict';
13
14
var ReactRef = require('ReactRef');
15
-var ReactElementValidator = require('ReactElementValidator');
15
16
/**
17
* Helper to call ReactRef.attachRefs with this composite component, split out
@@ -36,11 +35,6 @@ var ReactReconciler = {
35
*/
36
mountComponent: function(internalInstance, rootID, transaction, context) {
37
var markup = internalInstance.mountComponent(rootID, transaction, context);
39
- if (__DEV__) {
40
- ReactElementValidator.checkAndWarnForMutatedProps(
41
- internalInstance._currentElement
42
- );
43
- }
38
if (internalInstance._currentElement.ref != null) {
39
transaction.getReactMountReady().enqueue(attachRefs, internalInstance);
40
}
@@ -83,10 +77,6 @@ var ReactReconciler = {
77
return;
78
}
79
86
- if (__DEV__) {
87
- ReactElementValidator.checkAndWarnForMutatedProps(nextElement);
88
- }
89
-
80
var refsChanged = ReactRef.shouldUpdateRefs(
81
prevElement,
82
nextElement