From d0719a5ea4843cca40b0c1a73ad9acffd9639b50 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 26 Jun 2014 15:36:20 -0700 Subject: [PATCH] Preparing to move defaultProps resolution and type validation to the descriptor This copies the propType and contextType validation to a wrapper around the descriptor factory. By doing the validation early, we make it easier to track down bugs. It also prepares for static type checking which should be done at the usage site. This validation is not yet active and is just logged using monitorCodeUse. This will allow us to clean up callsites which would fail this new type of validation. I chose to copy the validation of abstracting it out since this is just an intermediate step to avoid spamming consoles. This makes more a much cleaner diff review/history. The original validation in the instance will be deleted as soon as we can turn on the warnings. Additionally, getDefaultProps are moved to become a static function which is only executed once. It should be moved to statics but we don't have a convenient way to merge mixins in statics right now. Deferring to ES6 classes. This is still a breaking change since you can return an object or array from getDefaultProps, which later gets mutated and now the shared instance is mutated. Mutating an object that is passed into you from props is highly discouraged and likely to lead to subtle bugs anyway. So I'm not too worried. The defaultProps will later be resolved in the descriptor factory. This will enable a perf optimizations where we don't create an unnecessary object allocation when you use default props. It also means that ReactChildren.map has access to resolved properties which gives them consistent behavior whether or not the default prop is specified. --- src/browser/ReactDOM.js | 7 + src/core/ReactCompositeComponent.js | 99 +++--- src/core/ReactDescriptor.js | 204 +++---------- src/core/ReactDescriptorValidator.js | 283 ++++++++++++++++++ .../__tests__/ReactCompositeComponent-test.js | 20 +- .../__tests__/ReactPropTransferer-test.js | 30 ++ src/core/__tests__/ReactPropTypes-test.js | 4 +- 7 files changed, 425 insertions(+), 222 deletions(-) create mode 100644 src/core/ReactDescriptorValidator.js diff --git a/src/browser/ReactDOM.js b/src/browser/ReactDOM.js index fed034f5e0..35797bb135 100644 --- a/src/browser/ReactDOM.js +++ b/src/browser/ReactDOM.js @@ -20,6 +20,7 @@ "use strict"; var ReactDescriptor = require('ReactDescriptor'); +var ReactDescriptorValidator = require('ReactDescriptorValidator'); var ReactDOMComponent = require('ReactDOMComponent'); var mergeInto = require('mergeInto'); @@ -50,6 +51,12 @@ function createDOMComponentClass(omitClose, tag) { var ConvenienceConstructor = ReactDescriptor.createFactory(Constructor); + if (__DEV__) { + return ReactDescriptorValidator.createFactory( + ConvenienceConstructor + ); + } + return ConvenienceConstructor; } diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 872b53ee33..26c31d060b 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -22,6 +22,7 @@ var ReactComponent = require('ReactComponent'); var ReactContext = require('ReactContext'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactDescriptor = require('ReactDescriptor'); +var ReactDescriptorValidator = require('ReactDescriptorValidator'); var ReactEmptyComponent = require('ReactEmptyComponent'); var ReactErrorUtils = require('ReactErrorUtils'); var ReactOwner = require('ReactOwner'); @@ -328,18 +329,17 @@ var ReactCompositeComponentInterface = { * which all other static methods are defined. */ var RESERVED_SPEC_KEYS = { - displayName: function(ConvenienceConstructor, displayName) { - ConvenienceConstructor.type.displayName = displayName; + displayName: function(Constructor, displayName) { + Constructor.displayName = displayName; }, - mixins: function(ConvenienceConstructor, mixins) { + mixins: function(Constructor, mixins) { if (mixins) { for (var i = 0; i < mixins.length; i++) { - mixSpecIntoComponent(ConvenienceConstructor, mixins[i]); + mixSpecIntoComponent(Constructor, mixins[i]); } } }, - childContextTypes: function(ConvenienceConstructor, childContextTypes) { - var Constructor = ConvenienceConstructor.type; + childContextTypes: function(Constructor, childContextTypes) { validateTypeDef( Constructor, childContextTypes, @@ -350,8 +350,7 @@ var RESERVED_SPEC_KEYS = { childContextTypes ); }, - contextTypes: function(ConvenienceConstructor, contextTypes) { - var Constructor = ConvenienceConstructor.type; + contextTypes: function(Constructor, contextTypes) { validateTypeDef( Constructor, contextTypes, @@ -359,8 +358,21 @@ var RESERVED_SPEC_KEYS = { ); Constructor.contextTypes = merge(Constructor.contextTypes, contextTypes); }, - propTypes: function(ConvenienceConstructor, propTypes) { - var Constructor = ConvenienceConstructor.type; + /** + * Special case getDefaultProps which should move into statics but requires + * automatic merging. + */ + getDefaultProps: function(Constructor, getDefaultProps) { + if (Constructor.getDefaultProps) { + Constructor.getDefaultProps = createMergedResultFunction( + Constructor.getDefaultProps, + getDefaultProps + ); + } else { + Constructor.getDefaultProps = getDefaultProps; + } + }, + propTypes: function(Constructor, propTypes) { validateTypeDef( Constructor, propTypes, @@ -368,8 +380,8 @@ var RESERVED_SPEC_KEYS = { ); Constructor.propTypes = merge(Constructor.propTypes, propTypes); }, - statics: function(ConvenienceConstructor, statics) { - mixStaticSpecIntoComponent(ConvenienceConstructor, statics); + statics: function(Constructor, statics) { + mixStaticSpecIntoComponent(Constructor, statics); } }; @@ -439,7 +451,7 @@ function validateLifeCycleOnReplaceState(instance) { * Custom version of `mixInto` which handles policy validation and reserved * specification keys when building `ReactCompositeComponent` classses. */ -function mixSpecIntoComponent(ConvenienceConstructor, spec) { +function mixSpecIntoComponent(Constructor, spec) { invariant( !ReactDescriptor.isValidFactory(spec), 'ReactCompositeComponent: You\'re attempting to ' + @@ -451,7 +463,6 @@ function mixSpecIntoComponent(ConvenienceConstructor, spec) { 'use a component as a mixin. Instead, just use a regular object.' ); - var Constructor = ConvenienceConstructor.type; var proto = Constructor.prototype; for (var name in spec) { var property = spec[name]; @@ -462,7 +473,7 @@ function mixSpecIntoComponent(ConvenienceConstructor, spec) { validateMethodOverride(proto, name); if (RESERVED_SPEC_KEYS.hasOwnProperty(name)) { - RESERVED_SPEC_KEYS[name](ConvenienceConstructor, property); + RESERVED_SPEC_KEYS[name](Constructor, property); } else { // Setup methods on prototype: // The following member methods should not be automatically bound: @@ -523,7 +534,7 @@ function mixSpecIntoComponent(ConvenienceConstructor, spec) { } } -function mixStaticSpecIntoComponent(ConvenienceConstructor, statics) { +function mixStaticSpecIntoComponent(Constructor, statics) { if (!statics) { return; } @@ -533,10 +544,10 @@ function mixStaticSpecIntoComponent(ConvenienceConstructor, statics) { continue; } - var isInherited = name in ConvenienceConstructor; + var isInherited = name in Constructor; var result = property; if (isInherited) { - var existingProperty = ConvenienceConstructor[name]; + var existingProperty = Constructor[name]; var existingType = typeof existingProperty; var propertyType = typeof property; invariant( @@ -549,10 +560,7 @@ function mixStaticSpecIntoComponent(ConvenienceConstructor, statics) { ); result = createChainedFunction(existingProperty, property); } - ConvenienceConstructor[name] = typeof result === 'function' ? - result.bind(ConvenienceConstructor.type) : - result; - ConvenienceConstructor.type[name] = result; + Constructor[name] = result; } } @@ -731,7 +739,6 @@ var ReactCompositeComponentMixin = { } this.context = this._processContext(this._descriptor._context); - this._defaultProps = this.getDefaultProps ? this.getDefaultProps() : null; this.props = this._processProps(this.props); this.state = this.getInitialState ? this.getInitialState() : null; @@ -785,8 +792,6 @@ var ReactCompositeComponentMixin = { } this._compositeLifeCycleState = null; - this._defaultProps = null; - this._renderedComponent.unmountComponent(); this._renderedComponent = null; @@ -924,13 +929,17 @@ var ReactCompositeComponentMixin = { * @private */ _processProps: function(newProps) { - var props = merge(newProps); - var defaultProps = this._defaultProps; - for (var propName in defaultProps) { - if (!props.hasOwnProperty(propName) || - typeof props[propName] === 'undefined') { - props[propName] = defaultProps[propName]; + var defaultProps = this.constructor.defaultProps; + var props; + if (defaultProps) { + props = merge(newProps); + for (var propName in defaultProps) { + if (typeof props[propName] === 'undefined') { + props[propName] = defaultProps[propName]; + } } + } else { + props = newProps; } if (__DEV__) { var propTypes = this.constructor.propTypes; @@ -950,6 +959,8 @@ var ReactCompositeComponentMixin = { * @private */ _checkPropTypes: function(propTypes, props, location) { + // TODO: Stop validating prop types here and only use the descriptor + // validation. var componentName = this.constructor.displayName; for (var propName in propTypes) { if (propTypes.hasOwnProperty(propName)) { @@ -1325,16 +1336,16 @@ var ReactCompositeComponent = { Constructor.prototype = new ReactCompositeComponentBase(); Constructor.prototype.constructor = Constructor; - var ConvenienceConstructor = ReactDescriptor.createFactory(Constructor); - - // TODO: Move statics off of the convenience constructor. That way the - // factory can be created independently from the main class. - injectedMixins.forEach( - mixSpecIntoComponent.bind(null, ConvenienceConstructor) + mixSpecIntoComponent.bind(null, Constructor) ); - mixSpecIntoComponent(ConvenienceConstructor, spec); + mixSpecIntoComponent(Constructor, spec); + + // Initialize the defaultProps property after all mixins have been merged + if (Constructor.getDefaultProps) { + Constructor.defaultProps = Constructor.getDefaultProps(); + } invariant( Constructor.prototype.render, @@ -1363,7 +1374,17 @@ var ReactCompositeComponent = { } } - return ConvenienceConstructor; + var descriptorFactory = ReactDescriptor.createFactory(Constructor); + + if (__DEV__) { + return ReactDescriptorValidator.createFactory( + descriptorFactory, + Constructor.propTypes, + Constructor.contextTypes + ); + } + + return descriptorFactory; }, injection: { diff --git a/src/core/ReactDescriptor.js b/src/core/ReactDescriptor.js index 0c3cdbf617..1850d05118 100644 --- a/src/core/ReactDescriptor.js +++ b/src/core/ReactDescriptor.js @@ -21,165 +21,9 @@ var ReactContext = require('ReactContext'); var ReactCurrentOwner = require('ReactCurrentOwner'); -var monitorCodeUse = require('monitorCodeUse'); +var merge = require('merge'); var warning = require('warning'); -/** - * Warn if there's no key explicitly set on dynamic arrays of children or - * object keys are not valid. This allows us to keep track of children between - * updates. - */ -var ownerHasKeyUseWarning = { - 'react_key_warning': {}, - 'react_numeric_key_warning': {} -}; -var ownerHasMonitoredObjectMap = {}; - -var NUMERIC_PROPERTY_REGEX = /^\d+$/; - -/** - * Gets the current owner's displayName for use in warnings. - * - * @internal - * @return {?string} Display name or undefined - */ -function getCurrentOwnerDisplayName() { - var current = ReactCurrentOwner.current; - return current && current.constructor.displayName || undefined; -} - -/** - * Warn if the component doesn't have an explicit key assigned to it. - * This component is in an array. The array could grow and shrink or be - * reordered. All children that haven't already been validated are required to - * have a "key" property assigned to it. - * - * @internal - * @param {ReactComponent} component Component that requires a key. - * @param {*} parentType component's parent's type. - */ -function validateExplicitKey(component, parentType) { - if (component._store.validated || component.props.key != null) { - return; - } - component._store.validated = true; - - warnAndMonitorForKeyUse( - 'react_key_warning', - 'Each child in an array should have a unique "key" prop.', - component, - parentType - ); -} - -/** - * Warn if the key is being defined as an object property but has an incorrect - * value. - * - * @internal - * @param {string} name Property name of the key. - * @param {ReactComponent} component Component that requires a key. - * @param {*} parentType component's parent's type. - */ -function validatePropertyKey(name, component, parentType) { - if (!NUMERIC_PROPERTY_REGEX.test(name)) { - return; - } - warnAndMonitorForKeyUse( - 'react_numeric_key_warning', - 'Child objects should have non-numeric keys so ordering is preserved.', - component, - parentType - ); -} - -/** - * Shared warning and monitoring code for the key warnings. - * - * @internal - * @param {string} warningID The id used when logging. - * @param {string} message The base warning that gets output. - * @param {ReactComponent} component Component that requires a key. - * @param {*} parentType component's parent's type. - */ -function warnAndMonitorForKeyUse(warningID, message, component, parentType) { - var ownerName = getCurrentOwnerDisplayName(); - var parentName = parentType.displayName; - - var useName = ownerName || parentName; - var memoizer = ownerHasKeyUseWarning[warningID]; - if (memoizer.hasOwnProperty(useName)) { - return; - } - memoizer[useName] = true; - - message += ownerName ? - ` Check the render method of ${ownerName}.` : - ` Check the renderComponent call using <${parentName}>.`; - - // Usually the current owner is the offender, but if it accepts children as a - // property, it may be the creator of the child that's responsible for - // assigning it a key. - var childOwnerName = null; - if (component._owner && component._owner !== ReactCurrentOwner.current) { - // Name of the component that originally created this child. - childOwnerName = component._owner.constructor.displayName; - - message += ` It was passed a child from ${childOwnerName}.`; - } - - message += ' See http://fb.me/react-warning-keys for more information.'; - monitorCodeUse(warningID, { - component: useName, - componentOwner: childOwnerName - }); - console.warn(message); -} - -/** - * Log that we're using an object map. We're considering deprecating this - * feature and replace it with proper Map and ImmutableMap data structures. - * - * @internal - */ -function monitorUseOfObjectMap() { - var currentName = getCurrentOwnerDisplayName() || ''; - if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) { - return; - } - ownerHasMonitoredObjectMap[currentName] = true; - monitorCodeUse('react_object_map_children'); -} - -/** - * Ensure that every component either is passed in a static location, in an - * array with an explicit keys property defined, or in an object literal - * with valid key property. - * - * @internal - * @param {*} component Statically passed child of any type. - * @param {*} parentType component's parent's type. - * @return {boolean} - */ -function validateChildKeys(component, parentType) { - if (Array.isArray(component)) { - for (var i = 0; i < component.length; i++) { - var child = component[i]; - if (ReactDescriptor.isValidDescriptor(child)) { - validateExplicitKey(child, parentType); - } - } - } else if (ReactDescriptor.isValidDescriptor(component)) { - // This component was passed in a valid location. - component._store.validated = true; - } else if (component && typeof component === 'object') { - monitorUseOfObjectMap(); - for (var name in component) { - validatePropertyKey(name, component[name], parentType); - } - } -} - /** * Warn for mutations. * @@ -237,6 +81,26 @@ function defineMutationMembrane(prototype) { } } +/** + * Transfer static properties from the source to the target. Functions are + * rebound to have this reflect the original source. + */ +function proxyStaticMethods(target, source) { + if (typeof source !== 'function') { + return; + } + for (var key in source) { + if (source.hasOwnProperty(key)) { + var value = source[key]; + if (typeof value === 'function') { + target[key] = value.bind(source); + } else { + target[key] = value; + } + } + } +} + /** * Base constructor for all React descriptors. This is only used to make this * work with a dynamic instanceof check. Nothing should live on this prototype. @@ -255,28 +119,32 @@ ReactDescriptor.createFactory = function(type) { var descriptorPrototype = Object.create(ReactDescriptor.prototype); var factory = function(props, children) { + // For consistency we currently allocate a new object for every descriptor. + // This protects the descriptor from being mutated by the original props + // object being mutated. It also protects the original props object from + // being mutated by children arguments and default props. This behavior + // comes with a performance cost and could be deprecated in the future. + // It could also be optimized with a smarter JSX transform. if (props == null) { props = {}; + } else if (typeof props === 'object') { + props = merge(props); } - // Children can be more than one argument + // Children can be more than one argument, and those are transferred onto + // the newly allocated props object. var childrenLength = arguments.length - 1; if (childrenLength === 1) { - if (__DEV__) { - validateChildKeys(children, type); - } props.children = children; } else if (childrenLength > 1) { var childArray = Array(childrenLength); for (var i = 0; i < childrenLength; i++) { - if (__DEV__) { - validateChildKeys(arguments[i + 1], type); - } childArray[i] = arguments[i + 1]; } props.children = childArray; } + // Initialize the descriptor object var descriptor = Object.create(descriptorPrototype); // Record the component responsible for creating this descriptor. @@ -289,10 +157,13 @@ ReactDescriptor.createFactory = function(type) { if (__DEV__) { // The validation flag and props are currently mutative. We put them 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. descriptor._store = { validated: false, props: props }; // We're not allowed to set props directly on the object so we early - // return and rely on the prototype membrane toward to the backing store. + // return and rely on the prototype membrane to forward to the backing + // store. if (useMutationMembrane) { Object.freeze(descriptor); return descriptor; @@ -301,7 +172,6 @@ ReactDescriptor.createFactory = function(type) { descriptor.props = props; return descriptor; - }; // Currently we expose the prototype of the descriptor so that @@ -316,6 +186,8 @@ ReactDescriptor.createFactory = function(type) { factory.type = type; descriptorPrototype.type = type; + proxyStaticMethods(factory, type); + // Expose a unique constructor on the prototype is that this works with type // systems that compare constructor properties: .constructor === Foo // This may be controversial since it requires a known factory function. diff --git a/src/core/ReactDescriptorValidator.js b/src/core/ReactDescriptorValidator.js new file mode 100644 index 0000000000..e6e23966aa --- /dev/null +++ b/src/core/ReactDescriptorValidator.js @@ -0,0 +1,283 @@ +/** + * Copyright 2014 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 ReactDescriptorValidator + */ + +/** + * ReactDescriptorValidator provides a wrapper around a descriptor factory + * which validates the props passed to the descriptor. This is intended to be + * used only in DEV and could be replaced by a static type checker for languages + * that support it. + */ + +"use strict"; + +var ReactDescriptor = require('ReactDescriptor'); +var ReactPropTypeLocations = require('ReactPropTypeLocations'); +var ReactCurrentOwner = require('ReactCurrentOwner'); + +var monitorCodeUse = require('monitorCodeUse'); + +/** + * Warn if there's no key explicitly set on dynamic arrays of children or + * object keys are not valid. This allows us to keep track of children between + * updates. + */ +var ownerHasKeyUseWarning = { + 'react_key_warning': {}, + 'react_numeric_key_warning': {} +}; +var ownerHasMonitoredObjectMap = {}; + +var loggedTypeFailures = {}; + +var NUMERIC_PROPERTY_REGEX = /^\d+$/; + +/** + * Gets the current owner's displayName for use in warnings. + * + * @internal + * @return {?string} Display name or undefined + */ +function getCurrentOwnerDisplayName() { + var current = ReactCurrentOwner.current; + return current && current.constructor.displayName || undefined; +} + +/** + * Warn if the component doesn't have an explicit key assigned to it. + * This component is in an array. The array could grow and shrink or be + * reordered. All children that haven't already been validated are required to + * have a "key" property assigned to it. + * + * @internal + * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. + */ +function validateExplicitKey(component, parentType) { + if (component._store.validated || component.props.key != null) { + return; + } + component._store.validated = true; + + warnAndMonitorForKeyUse( + 'react_key_warning', + 'Each child in an array should have a unique "key" prop.', + component, + parentType + ); +} + +/** + * Warn if the key is being defined as an object property but has an incorrect + * value. + * + * @internal + * @param {string} name Property name of the key. + * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. + */ +function validatePropertyKey(name, component, parentType) { + if (!NUMERIC_PROPERTY_REGEX.test(name)) { + return; + } + warnAndMonitorForKeyUse( + 'react_numeric_key_warning', + 'Child objects should have non-numeric keys so ordering is preserved.', + component, + parentType + ); +} + +/** + * Shared warning and monitoring code for the key warnings. + * + * @internal + * @param {string} warningID The id used when logging. + * @param {string} message The base warning that gets output. + * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. + */ +function warnAndMonitorForKeyUse(warningID, message, component, parentType) { + var ownerName = getCurrentOwnerDisplayName(); + var parentName = parentType.displayName; + + var useName = ownerName || parentName; + var memoizer = ownerHasKeyUseWarning[warningID]; + if (memoizer.hasOwnProperty(useName)) { + return; + } + memoizer[useName] = true; + + message += ownerName ? + ` Check the render method of ${ownerName}.` : + ` Check the renderComponent call using <${parentName}>.`; + + // Usually the current owner is the offender, but if it accepts children as a + // property, it may be the creator of the child that's responsible for + // assigning it a key. + var childOwnerName = null; + if (component._owner && component._owner !== ReactCurrentOwner.current) { + // Name of the component that originally created this child. + childOwnerName = component._owner.constructor.displayName; + + message += ` It was passed a child from ${childOwnerName}.`; + } + + message += ' See http://fb.me/react-warning-keys for more information.'; + monitorCodeUse(warningID, { + component: useName, + componentOwner: childOwnerName + }); + console.warn(message); +} + +/** + * Log that we're using an object map. We're considering deprecating this + * feature and replace it with proper Map and ImmutableMap data structures. + * + * @internal + */ +function monitorUseOfObjectMap() { + var currentName = getCurrentOwnerDisplayName() || ''; + if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) { + return; + } + ownerHasMonitoredObjectMap[currentName] = true; + monitorCodeUse('react_object_map_children'); +} + +/** + * Ensure that every component either is passed in a static location, in an + * array with an explicit keys property defined, or in an object literal + * with valid key property. + * + * @internal + * @param {*} component Statically passed child of any type. + * @param {*} parentType component's parent's type. + * @return {boolean} + */ +function validateChildKeys(component, parentType) { + if (Array.isArray(component)) { + for (var i = 0; i < component.length; i++) { + var child = component[i]; + if (ReactDescriptor.isValidDescriptor(child)) { + validateExplicitKey(child, parentType); + } + } + } else if (ReactDescriptor.isValidDescriptor(component)) { + // This component was passed in a valid location. + component._store.validated = true; + } else if (component && typeof component === 'object') { + monitorUseOfObjectMap(); + for (var name in component) { + validatePropertyKey(name, component[name], parentType); + } + } +} + +/** + * Assert that the props are valid + * + * @param {string} componentName Name of the component for error messages. + * @param {object} propTypes Map of prop name to a ReactPropType + * @param {object} props + * @param {string} location e.g. "prop", "context", "child context" + * @private + */ +function checkPropTypes(componentName, propTypes, props, location) { + for (var propName in propTypes) { + if (propTypes.hasOwnProperty(propName)) { + var error; + // Prop type validation may throw. In case they do, we don't want to + // fail the render phase where it didn't fail before. So we log it. + // After these have been cleaned up, we'll let them throw. + try { + error = propTypes[propName](props, propName, componentName, location); + } catch (ex) { + error = ex; + } + if (error instanceof Error && !(error.message in loggedTypeFailures)) { + // Only monitor this failure once because there tends to be a lot of the + // same error. + loggedTypeFailures[error.message] = true; + // This will soon use the warning module + monitorCodeUse( + 'react_failed_descriptor_type_check', + { message: error.message } + ); + } + } + } +} + +var ReactDescriptorValidator = { + + /** + * Wraps a descriptor factory function in another function which validates + * the props and context of the descriptor and warns about any failed type + * checks. + * + * @param {function} factory The original descriptor factory + * @param {object?} propTypes A prop type definition set + * @param {object?} contextTypes A context type definition set + * @return {object} The component descriptor, which may be invalid. + * @private + */ + createFactory: function(factory, propTypes, contextTypes) { + var validatedFactory = function(props, children) { + var descriptor = factory.apply(this, arguments); + + for (var i = 1; i < arguments.length; i++) { + validateChildKeys(arguments[i], descriptor.type); + } + + var name = descriptor.type.displayName; + if (propTypes) { + checkPropTypes( + name, + propTypes, + descriptor.props, + ReactPropTypeLocations.prop + ); + } + if (contextTypes) { + checkPropTypes( + name, + contextTypes, + descriptor._context, + ReactPropTypeLocations.context + ); + } + return descriptor; + }; + + validatedFactory.prototype = factory.prototype; + validatedFactory.type = factory.type; + + // Copy static properties + for (var key in factory) { + if (factory.hasOwnProperty(key)) { + validatedFactory[key] = factory[key]; + } + } + + return validatedFactory; + } + +}; + +module.exports = ReactDescriptorValidator; diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 66e735ae26..4fe1053799 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -376,27 +376,17 @@ describe('ReactCompositeComponent', function() { }); - it('should auto bind before getDefaultProps', function() { - var calls = 0; + it('should not pass this to getDefaultProps', function() { var Component = React.createClass({ getDefaultProps: function() { - return { - onClick: this.defaultClickHandler - }; - }, - defaultClickHandler: function() { - expect(this).toBe(instance); - calls++; + expect(this.render).not.toBeDefined(); + return {}; }, render: function() { - return
; + return
; } }); - var instance = ReactTestUtils.renderIntoDocument(); - var handler = instance.props.onClick; - // Call handler with no context - handler(); - expect(calls).toBe(1); + ReactTestUtils.renderIntoDocument(); }); it('should use default values for undefined props', function() { diff --git a/src/core/__tests__/ReactPropTransferer-test.js b/src/core/__tests__/ReactPropTransferer-test.js index 92f0a05771..940e85c9bd 100644 --- a/src/core/__tests__/ReactPropTransferer-test.js +++ b/src/core/__tests__/ReactPropTransferer-test.js @@ -156,4 +156,34 @@ describe('ReactPropTransferer', function() { 'passed in as props or children.' ); }); + + it('should not use the default when a prop is transfered', function() { + + var Child = React.createClass({ + + getDefaultProps: function() { + return { + x: 2 + }; + }, + + render: function() { + expect(this.props.x).toBe(5); + return
; + } + + }); + + var Parent = React.createClass({ + + render: function() { + return this.transferPropsTo(); + } + + }); + + ReactTestUtils.renderIntoDocument(); + + }); + }); diff --git a/src/core/__tests__/ReactPropTypes-test.js b/src/core/__tests__/ReactPropTypes-test.js index e2cabd2b8c..dd6032359d 100644 --- a/src/core/__tests__/ReactPropTypes-test.js +++ b/src/core/__tests__/ReactPropTypes-test.js @@ -712,7 +712,7 @@ describe('Custom validator', function() { var instance = ; instance = ReactTestUtils.renderIntoDocument(instance); - expect(spy.argsForCall.length).toBe(1); + expect(spy.argsForCall.length).toBe(2); // temp double validation expect(spy.argsForCall[0][1]).toBe('num'); expect(spy.argsForCall[0][2]).toBe('Component'); }); @@ -730,7 +730,7 @@ describe('Custom validator', function() { var instance = ; instance = ReactTestUtils.renderIntoDocument(instance); - expect(spy.argsForCall.length).toBe(1); + expect(spy.argsForCall.length).toBe(2); // temp double validation }); it('should have received the validator\'s return value', function() {