From 351dcfed01d51dd18016d84ded4f86bddecaa7ba Mon Sep 17 00:00:00 2001 From: Marshall Roch Date: Wed, 27 Nov 2013 12:00:30 -0800 Subject: [PATCH] Validate propTypes, contextTypes, childContextTypes who watches the watchmen? --- src/core/ReactCompositeComponent.js | 36 +++++++++++-- src/core/ReactPropTypeLocationNames.js | 31 +++++++++++ src/core/ReactPropTypes.js | 12 +---- .../__tests__/ReactCompositeComponent-test.js | 51 +++++++++++++++++++ 4 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 src/core/ReactPropTypeLocationNames.js diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 5adfae53f4..881d4d9d42 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -26,6 +26,7 @@ var ReactOwner = require('ReactOwner'); var ReactPerf = require('ReactPerf'); var ReactPropTransferer = require('ReactPropTransferer'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactUpdates = require('ReactUpdates'); var invariant = require('invariant'); @@ -319,16 +320,46 @@ var RESERVED_SPEC_KEYS = { } }, childContextTypes: function(Constructor, childContextTypes) { + validateTypeDef( + Constructor, + childContextTypes, + ReactPropTypeLocations.childContext + ); Constructor.childContextTypes = childContextTypes; }, contextTypes: function(Constructor, contextTypes) { + validateTypeDef( + Constructor, + contextTypes, + ReactPropTypeLocations.context + ); Constructor.contextTypes = contextTypes; }, propTypes: function(Constructor, propTypes) { + validateTypeDef( + Constructor, + propTypes, + ReactPropTypeLocations.prop + ); Constructor.propTypes = propTypes; } }; +function validateTypeDef(Constructor, typeDef, location) { + for (var propName in typeDef) { + if (typeDef.hasOwnProperty(propName)) { + invariant( + typeof typeDef[propName] == 'function', + '%s: %s type `%s` is invalid; it must be a function, usually from ' + + 'React.PropTypes.', + Constructor.displayName || 'ReactCompositeComponent', + ReactPropTypeLocationNames[location], + propName + ); + } + } +} + function validateMethodOverride(proto, name) { var specPolicy = ReactCompositeComponentInterface[name]; @@ -790,9 +821,8 @@ var ReactCompositeComponentMixin = { _checkPropTypes: function(propTypes, props, location) { var componentName = this.constructor.displayName; for (var propName in propTypes) { - var checkProp = propTypes[propName]; - if (checkProp) { - checkProp(props, propName, componentName, location); + if (propTypes.hasOwnProperty(propName)) { + propTypes[propName](props, propName, componentName, location); } } }, diff --git a/src/core/ReactPropTypeLocationNames.js b/src/core/ReactPropTypeLocationNames.js new file mode 100644 index 0000000000..a62d63a992 --- /dev/null +++ b/src/core/ReactPropTypeLocationNames.js @@ -0,0 +1,31 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule ReactPropTypeLocationNames + */ + +"use strict"; + +var ReactPropTypeLocationNames = {}; + +if (__DEV__) { + ReactPropTypeLocationNames = { + prop: 'prop', + context: 'context', + childContext: 'child context' + }; +} + +module.exports = ReactPropTypeLocationNames; diff --git a/src/core/ReactPropTypes.js b/src/core/ReactPropTypes.js index cf243edd86..753d9d73b3 100644 --- a/src/core/ReactPropTypes.js +++ b/src/core/ReactPropTypes.js @@ -18,19 +18,11 @@ "use strict"; +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); + var createObjectFrom = require('createObjectFrom'); var invariant = require('invariant'); -var ReactPropTypeLocationNames = {}; - -if (__DEV__) { - ReactPropTypeLocationNames = { - prop: 'prop', - context: 'context', - childContext: 'child context' - }; -} - /** * Collection of methods that allow declaration and validation of props that are * supplied to React components. Example usage: diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 188f618b9e..0afef2dbda 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -289,6 +289,57 @@ describe('ReactCompositeComponent', function() { }).not.toThrow(); }); + it('should throw on invalid prop types', function() { + expect(function() { + React.createClass({ + displayName: 'Component', + propTypes: { + key: null + }, + render: function() { + return {this.props.key}; + } + }); + }).toThrow( + 'Invariant Violation: Component: prop type `key` is invalid; ' + + 'it must be a function, usually from React.PropTypes.' + ); + }); + + it('should throw on invalid context types', function() { + expect(function() { + React.createClass({ + displayName: 'Component', + contextTypes: { + key: null + }, + render: function() { + return {this.props.key}; + } + }); + }).toThrow( + 'Invariant Violation: Component: context type `key` is invalid; ' + + 'it must be a function, usually from React.PropTypes.' + ); + }); + + it('should throw on invalid child context types', function() { + expect(function() { + React.createClass({ + displayName: 'Component', + childContextTypes: { + key: null + }, + render: function() { + return {this.props.key}; + } + }); + }).toThrow( + 'Invariant Violation: Component: child context type `key` is invalid; ' + + 'it must be a function, usually from React.PropTypes.' + ); + }); + it('should not allow `forceUpdate` on unmounted components', function() { var container = document.createElement('div'); document.documentElement.appendChild(container);