From 82d15c8fd52811dfca22609d73ac9dcb3871adcd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 29 Dec 2014 12:43:40 -0500 Subject: [PATCH] Add late class validation warnings for statics ES6 classes won't have an early validation step. Therefore I added some extra validation later on in the process. These would've normally have been caught by createClass in classic React. --- src/classic/class/ReactClass.js | 6 ++ src/classic/element/ReactElementValidator.js | 20 +++++++ src/core/ReactCompositeComponent.js | 19 +++++- .../ReactJSXElementValidator-test.js | 58 +++++++++++++++++++ 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/classic/class/ReactClass.js b/src/classic/class/ReactClass.js index b38bc4c72c..c9dc4203f7 100644 --- a/src/classic/class/ReactClass.js +++ b/src/classic/class/ReactClass.js @@ -868,6 +868,12 @@ var ReactClass = { // Initialize the defaultProps property after all mixins have been merged if (Constructor.getDefaultProps) { Constructor.defaultProps = Constructor.getDefaultProps(); + if (__DEV__) { + // This is a tag to indicate that this use of getDefaultProps is ok, + // since it's used with createClass. If it's not, then it's likely a + // mistake so we'll warn you to use the static property instead. + Constructor.getDefaultProps._isReactClassApproved = true; + } } invariant( diff --git a/src/classic/element/ReactElementValidator.js b/src/classic/element/ReactElementValidator.js index 7c262243c8..1aedd8cc98 100644 --- a/src/classic/element/ReactElementValidator.js +++ b/src/classic/element/ReactElementValidator.js @@ -20,10 +20,12 @@ var ReactElement = require('ReactElement'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactCurrentOwner = require('ReactCurrentOwner'); var getIteratorFn = require('getIteratorFn'); var monitorCodeUse = require('monitorCodeUse'); +var invariant = require('invariant'); var warning = require('warning'); /** @@ -236,6 +238,16 @@ function checkPropTypes(componentName, propTypes, props, location) { // 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 { + // This is intentionally an invariant that gets caught. It's the same + // behavior as without this statement except with a better message. + invariant( + typeof propTypes[propName] === 'function', + '%s: %s type `%s` is invalid; it must be a function, usually from ' + + 'React.PropTypes.', + componentName || 'React class', + ReactPropTypeLocationNames[location], + propName + ); error = propTypes[propName](props, propName, componentName, location); } catch (ex) { error = ex; @@ -296,7 +308,15 @@ var ReactElementValidator = { ReactPropTypeLocations.context ); } + if (typeof type.getDefaultProps === 'function') { + warning( + type.getDefaultProps._isReactClassApproved, + 'getDefaultProps is only used on classic React.createClass ' + + 'definitions. Use a static property named `defaultProps` instead.' + ); + } } + return element; }, diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 7875c3bdbe..ab78a4d345 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -19,6 +19,7 @@ var ReactElement = require('ReactElement'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactPerf = require('ReactPerf'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactUpdates = require('ReactUpdates'); var assign = require('Object.assign'); @@ -515,8 +516,22 @@ var ReactCompositeComponentMixin = assign({}, this._instance.constructor.name; for (var propName in propTypes) { if (propTypes.hasOwnProperty(propName)) { - var error = - propTypes[propName](props, propName, componentName, location); + var error; + try { + // This is intentionally an invariant that gets caught. It's the same + // behavior as without this statement except with a better message. + invariant( + typeof propTypes[propName] === 'function', + '%s: %s type `%s` is invalid; it must be a function, usually ' + + 'from React.PropTypes.', + componentName || 'React class', + ReactPropTypeLocationNames[location], + propName + ); + error = propTypes[propName](props, propName, componentName, location); + } catch (ex) { + error = ex; + } if (error instanceof Error) { // We may want to extend this logic for similar errors in // React.render calls, so I'm abstracting it away into diff --git a/src/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/modern/element/__tests__/ReactJSXElementValidator-test.js index 779c72fbfc..76abef884a 100644 --- a/src/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -290,4 +290,62 @@ describe('ReactJSXElementValidator', function() { expect(console.warn.calls.length).toBe(2); }); + it('should warn on invalid prop types', function() { + // Since there is no prevalidation step for ES6 classes, there is no hook + // for us to issue a warning earlier than element creation when the error + // actually occurs. Since this step is skipped in production, we should just + // warn instead of throwing for this case. + spyOn(console, 'warn'); + class Component { + render() { + return {this.props.prop}; + } + } + Component.propTypes = { + prop: null + }; + ReactTestUtils.renderIntoDocument(); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'Invariant Violation: Component: prop type `prop` is invalid; ' + + 'it must be a function, usually from React.PropTypes.' + ); + }); + + it('should warn on invalid context types', function() { + spyOn(console, 'warn'); + class Component { + render() { + return {this.props.prop}; + } + } + Component.contextTypes = { + prop: null + }; + ReactTestUtils.renderIntoDocument(); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'Invariant Violation: Component: context type `prop` is invalid; ' + + 'it must be a function, usually from React.PropTypes.' + ); + }); + + it('should warn if getDefaultProps is specificed on the class', function() { + spyOn(console, 'warn'); + class Component { + render() { + return {this.props.prop}; + } + } + Component.getDefaultProps = () => ({ + prop: 'foo' + }); + ReactTestUtils.renderIntoDocument(); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'getDefaultProps is only used on classic React.createClass definitions.' + + ' Use a static property named `defaultProps` instead.' + ); + }); + });