diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js index 42b7e37cab3bb..9e333270e2f5c 100644 --- a/src/isomorphic/classic/element/ReactElement.js +++ b/src/isomorphic/classic/element/ReactElement.js @@ -14,71 +14,12 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var assign = require('Object.assign'); -var warning = require('warning'); var RESERVED_PROPS = { key: true, ref: true, }; -/** - * Warn for mutations. - * - * @internal - * @param {object} object - * @param {string} key - */ -function defineWarningProperty(object, key) { - Object.defineProperty(object, key, { - - configurable: false, - enumerable: true, - - get: function() { - if (!this._store) { - return null; - } - return this._store[key]; - }, - - set: function(value) { - warning( - false, - 'Don\'t set the %s property of the React element. Instead, ' + - 'specify the correct value when initially creating the element.', - key - ); - this._store[key] = value; - }, - - }); -} - -/** - * This is updated to true if the membrane is successfully created. - */ -var useMutationMembrane = false; - -/** - * Warn for mutations. - * - * @internal - * @param {object} prototype - */ -function defineMutationMembrane(prototype) { - try { - var pseudoFrozenProperties = { - props: true, - }; - for (var key in pseudoFrozenProperties) { - defineWarningProperty(prototype, key); - } - useMutationMembrane = true; - } catch (x) { - // IE will fail on defineProperty - } -} - /** * Base constructor for all React elements. This is only used to make this * work with a dynamic instanceof check. Nothing should live on this prototype. @@ -100,11 +41,11 @@ var ReactElement = function(type, key, ref, owner, props) { this._owner = owner; if (__DEV__) { - // The validation flag and props are currently mutative. We put them on + // The validation flag is currently mutative. We put it on // an external backing store so that we can freeze the whole object. // This can be replaced with a WeakMap once they are implemented in // commonly used development environments. - this._store = {props: props, originalProps: assign({}, props)}; + this._store = {}; // To make comparing ReactElements easier for testing purposes, we make // the validation flag non-enumerable (where possible, which should @@ -115,21 +56,15 @@ var ReactElement = function(type, key, ref, owner, props) { configurable: false, enumerable: false, writable: true, + value: false, }); } catch (x) { + this._store.validated = false; } - this._store.validated = false; - - // We're not allowed to set props directly on the object so we early - // return and rely on the prototype membrane to forward to the backing - // store. - if (useMutationMembrane) { - Object.freeze(this); - return; - } + this.props = props; + Object.freeze(this.props); + Object.freeze(this); } - - this.props = props; }; // We intentionally don't expose the function on the constructor property. @@ -138,10 +73,6 @@ ReactElement.prototype = { _isReactElement: true, }; -if (__DEV__) { - defineMutationMembrane(ReactElement.prototype); -} - ReactElement.createElement = function(type, config, children) { var propName; @@ -219,6 +150,7 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) { // If the key on the original is valid, then the clone is valid newElement._store.validated = oldElement._store.validated; } + return newElement; }; diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index a7067ec0457ff..d4c2df2a077df 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -286,90 +286,6 @@ function checkPropTypes(componentName, propTypes, props, location) { } } -var warnedPropsMutations = {}; - -/** - * Warn about mutating props when setting `propName` on `element`. - * - * @param {string} propName The string key within props that was set - * @param {ReactElement} element - */ -function warnForPropsMutation(propName, element) { - var type = element.type; - var elementName = typeof type === 'string' ? type : type.displayName; - var ownerName = element._owner ? - element._owner.getPublicInstance().constructor.displayName : null; - - var warningKey = propName + '|' + elementName + '|' + ownerName; - if (warnedPropsMutations.hasOwnProperty(warningKey)) { - return; - } - warnedPropsMutations[warningKey] = true; - - var elementInfo = ''; - if (elementName) { - elementInfo = ' <' + elementName + ' />'; - } - var ownerInfo = ''; - if (ownerName) { - ownerInfo = ' The element was created by ' + ownerName + '.'; - } - - warning( - false, - 'Don\'t set .props.%s of the React component%s. Instead, specify the ' + - 'correct value when initially creating the element or use ' + - 'React.cloneElement to make a new element with updated props.%s', - propName, - elementInfo, - ownerInfo - ); -} - -// Inline Object.is polyfill -function is(a, b) { - if (a !== a) { - // NaN - return b !== b; - } - if (a === 0 && b === 0) { - // +-0 - return 1 / a === 1 / b; - } - return a === b; -} - -/** - * Given an element, check if its props have been mutated since element - * creation (or the last call to this function). In particular, check if any - * new props have been added, which we can't directly catch by defining warning - * properties on the props object. - * - * @param {ReactElement} element - */ -function checkAndWarnForMutatedProps(element) { - if (!element._store) { - // Element was created using `new ReactElement` directly or with - // `ReactElement.createElement`; skip mutation checking - return; - } - - var originalProps = element._store.originalProps; - var props = element.props; - - for (var propName in props) { - if (props.hasOwnProperty(propName)) { - if (!originalProps.hasOwnProperty(propName) || - !is(originalProps[propName], props[propName])) { - warnForPropsMutation(propName, element); - - // Copy over the new value so that the two props objects match again - originalProps[propName] = props[propName]; - } - } - } -} - /** * Given an element, validate that its props follow the propTypes definition, * provided by the type. @@ -409,8 +325,6 @@ function validatePropTypes(element) { var ReactElementValidator = { - checkAndWarnForMutatedProps: checkAndWarnForMutatedProps, - createElement: function(type, props, children) { // We warn in this case but don't throw. We expect the element creation to // succeed and there will likely be errors in render. diff --git a/src/isomorphic/classic/element/__tests__/ReactElement-test.js b/src/isomorphic/classic/element/__tests__/ReactElement-test.js index 0cb9f82a84d78..1e715e1f45579 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElement-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElement-test.js @@ -37,7 +37,9 @@ describe('ReactElement', function() { expect(element.type).toBe(ComponentClass); expect(element.key).toBe(null); expect(element.ref).toBe(null); - expect(element.props).toEqual({}); + var expectation = {}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('allows a string to be passed as the type', function() { @@ -45,7 +47,9 @@ describe('ReactElement', function() { expect(element.type).toBe('div'); expect(element.key).toBe(null); expect(element.ref).toBe(null); - expect(element.props).toEqual({}); + var expectation = {}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('returns an immutable element', function() { @@ -70,7 +74,9 @@ describe('ReactElement', function() { expect(element.type).toBe(ComponentClass); expect(element.key).toBe('12'); expect(element.ref).toBe('34'); - expect(element.props).toEqual({foo:'56'}); + var expectation = {foo:'56'}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('coerces the key to a string', function() { @@ -81,7 +87,9 @@ describe('ReactElement', function() { expect(element.type).toBe(ComponentClass); expect(element.key).toBe('12'); expect(element.ref).toBe(null); - expect(element.props).toEqual({foo:'56'}); + var expectation = {foo:'56'}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('preserves the owner on the element', function() { @@ -243,84 +251,42 @@ describe('ReactElement', function() { expect(inst2.props.prop).toBe(null); }); - it('warns when changing a prop after element creation', function() { - spyOn(console, 'error'); + it('throws when changing a prop (in dev) after element creation', function() { var Outer = React.createClass({ render: function() { var el =
; - // This assignment warns but should still work for now. - el.props.className = 'quack'; - expect(el.props.className).toBe('quack'); + expect(function() { + el.props.className = 'quack'; + }).toThrow(); + expect(el.props.className).toBe('moo'); return el; }, }); var outer = ReactTestUtils.renderIntoDocument(); - expect(React.findDOMNode(outer).className).toBe('quack'); - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Don\'t set .props.className of the React component
.' - ); - expect(console.error.argsForCall[0][0]).toContain( - 'The element was created by Outer.' - ); - - console.error.reset(); - - // This also warns (just once per key/type pair) - outer.props.color = 'green'; - outer.forceUpdate(); - outer.props.color = 'purple'; - outer.forceUpdate(); - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Don\'t set .props.color of the React component .' - ); + expect(React.findDOMNode(outer).className).toBe('moo'); }); - it('warns when adding a prop after element creation', function() { - spyOn(console, 'error'); + it('throws when adding a prop (in dev) after element creation', function() { var container = document.createElement('div'); var Outer = React.createClass({ getDefaultProps: () => ({sound: 'meow'}), render: function() { var el =
{this.props.sound}
; - // This assignment doesn't warn immediately (because we can't) but it - // warns upon mount. - el.props.className = 'quack'; - expect(el.props.className).toBe('quack'); + expect(function() { + el.props.className = 'quack'; + }).toThrow(); + + expect(el.props.className).toBe(undefined); return el; }, }); var outer = React.render(, container); expect(React.findDOMNode(outer).textContent).toBe('meow'); - expect(React.findDOMNode(outer).className).toBe('quack'); - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Don\'t set .props.className of the React component
.' - ); - expect(console.error.argsForCall[0][0]).toContain( - 'The element was created by Outer.' - ); - - console.error.reset(); - - var newOuterEl = ; - newOuterEl.props.sound = 'oink'; - outer = React.render(newOuterEl, container); - expect(React.findDOMNode(outer).textContent).toBe('oink'); - expect(React.findDOMNode(outer).className).toBe('quack'); - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Don\'t set .props.sound of the React component .' - ); + expect(React.findDOMNode(outer).className).toBe(''); }); it('does not warn for NaN props', function() { diff --git a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js index 9c1179f972b7f..e1c876bc71c7b 100644 --- a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js +++ b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js @@ -16,6 +16,10 @@ var React; describe('ReactES6Class', function() { var container; + var freeze = function(expectation) { + Object.freeze(expectation); + return expectation; + }; var Inner; var attachedListener = null; var renderedName = null; @@ -288,10 +292,10 @@ describe('ReactES6Class', function() { lifeCycles = []; // reset test(, 'SPAN', 'bar'); expect(lifeCycles).toEqual([ - 'receive-props', {value: 'bar'}, - 'should-update', {value: 'bar'}, {}, - 'will-update', {value: 'bar'}, {}, - 'did-update', {value: 'foo'}, {}, + 'receive-props', freeze({value: 'bar'}), + 'should-update', freeze({value: 'bar'}), {}, + 'will-update', freeze({value: 'bar'}), {}, + 'did-update', freeze({value: 'foo'}), {}, ]); lifeCycles = []; // reset React.unmountComponentAtNode(container); diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js index 06ec4fe9f91de..caf4e7f923615 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js @@ -34,7 +34,9 @@ describe('ReactJSXElement', function() { expect(element.type).toBe(Component); expect(element.key).toBe(null); expect(element.ref).toBe(null); - expect(element.props).toEqual({}); + var expectation = {}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('allows a lower-case to be passed as the string type', function() { @@ -42,7 +44,9 @@ describe('ReactJSXElement', function() { expect(element.type).toBe('div'); expect(element.key).toBe(null); expect(element.ref).toBe(null); - expect(element.props).toEqual({}); + var expectation = {}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('allows a string to be passed as the type', function() { @@ -51,7 +55,9 @@ describe('ReactJSXElement', function() { expect(element.type).toBe('div'); expect(element.key).toBe(null); expect(element.ref).toBe(null); - expect(element.props).toEqual({}); + var expectation = {}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('returns an immutable element', function() { @@ -72,7 +78,9 @@ describe('ReactJSXElement', function() { expect(element.type).toBe(Component); expect(element.key).toBe('12'); expect(element.ref).toBe('34'); - expect(element.props).toEqual({foo:'56'}); + var expectation = {foo:'56'}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('coerces the key to a string', function() { @@ -80,7 +88,9 @@ describe('ReactJSXElement', function() { expect(element.type).toBe(Component); expect(element.key).toBe('12'); expect(element.ref).toBe(null); - expect(element.props).toEqual({foo:'56'}); + var expectation = {foo:'56'}; + Object.freeze(expectation); + expect(element.props).toEqual(expectation); }); it('merges JSX children onto the children prop', function() { diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index c4045117be3de..d0a0fdd8886fb 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -15,7 +15,6 @@ var DOMProperty = require('DOMProperty'); var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); -var ReactElementValidator = require('ReactElementValidator'); var ReactEmptyComponent = require('ReactEmptyComponent'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -375,10 +374,6 @@ var ReactMount = { nextElement, container, callback) { - if (__DEV__) { - ReactElementValidator.checkAndWarnForMutatedProps(nextElement); - } - ReactMount.scrollMonitor(container, function() { ReactUpdateQueue.enqueueElementInternal(prevComponent, nextElement); if (callback) { diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index c95a58593d422..d87372c6110a6 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -14,7 +14,6 @@ var ReactComponentEnvironment = require('ReactComponentEnvironment'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); -var ReactElementValidator = require('ReactElementValidator'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactLifeCycle = require('ReactLifeCycle'); var ReactNativeComponent = require('ReactNativeComponent'); @@ -503,12 +502,6 @@ var ReactCompositeComponentMixin = { } if (this._pendingStateQueue !== null || this._pendingForceUpdate) { - if (__DEV__) { - ReactElementValidator.checkAndWarnForMutatedProps( - this._currentElement - ); - } - this.updateComponent( transaction, this._currentElement, diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index a3cefe71a895c..c4067d7d6742d 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -12,7 +12,6 @@ 'use strict'; var ReactRef = require('ReactRef'); -var ReactElementValidator = require('ReactElementValidator'); /** * Helper to call ReactRef.attachRefs with this composite component, split out @@ -36,11 +35,6 @@ var ReactReconciler = { */ mountComponent: function(internalInstance, rootID, transaction, context) { var markup = internalInstance.mountComponent(rootID, transaction, context); - if (__DEV__) { - ReactElementValidator.checkAndWarnForMutatedProps( - internalInstance._currentElement - ); - } if (internalInstance._currentElement.ref != null) { transaction.getReactMountReady().enqueue(attachRefs, internalInstance); } @@ -83,10 +77,6 @@ var ReactReconciler = { return; } - if (__DEV__) { - ReactElementValidator.checkAndWarnForMutatedProps(nextElement); - } - var refsChanged = ReactRef.shouldUpdateRefs( prevElement, nextElement