From d973e32336913b8dd5cbc1fb5d7caaed31ea2cf8 Mon Sep 17 00:00:00 2001 From: Aria Buckles Date: Tue, 10 Mar 2015 15:21:52 -0700 Subject: [PATCH] [PropTypes] Add warnings if PropTypes return functions Summary: Right now, if a component specifies a propType as, for example, `myProp: React.PropTypes.shape`, without an actual shape parameter, any prop type will be accepted, because `React.PropTypes.shape` returns a function (the actual validator), not an Error, currently indicating that propType checking passed. This can create an unfortunate situation where a component looks like it has fully specified `propTypes`, but in fact does not. This commit addresses this by warning if a propType checker returns anything non-falsy that is not an Error (currently all the library PropTypes return null or an Error). Test Plan: Added a unit test; ran `jest` in the root repo directory. Also ran `grunt lint` and `grunt test` --- src/classic/element/ReactElementValidator.js | 12 +++++++++ .../__tests__/ReactElementValidator-test.js | 26 +++++++++++++++++++ .../types/__tests__/ReactPropTypes-test.js | 4 +-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/classic/element/ReactElementValidator.js b/src/classic/element/ReactElementValidator.js index 026bf3ce11..71a339313f 100644 --- a/src/classic/element/ReactElementValidator.js +++ b/src/classic/element/ReactElementValidator.js @@ -247,6 +247,18 @@ function checkPropTypes(componentName, propTypes, props, location) { } catch (ex) { error = ex; } + warning( + !error || error instanceof Error, + '%s: the type of %s `%s` is invalid; the type checker function ' + + 'must return `null` or an `Error`, but returned a %s. ' + + 'You may have forgotten to pass an argument to the type checker ' + + 'creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and ' + + 'shape all require an argument).', + componentName || 'React class', + ReactPropTypeLocationNames[location], + propName, + typeof error + ); if (error instanceof Error && !(error.message in loggedTypeFailures)) { // Only monitor this failure once because there tends to be a lot of the // same error. diff --git a/src/classic/element/__tests__/ReactElementValidator-test.js b/src/classic/element/__tests__/ReactElementValidator-test.js index 98de236f28..757b29e73e 100644 --- a/src/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/classic/element/__tests__/ReactElementValidator-test.js @@ -362,6 +362,32 @@ describe('ReactElementValidator', function() { expect(console.warn.calls.length).toBe(2); }); + it('should warn if a PropType creator is used as a PropType', function() { + spyOn(console, 'warn'); + + var Component = React.createClass({ + propTypes: { + prop: React.PropTypes.shape + }, + render: function() { + return React.createElement('span', null, this.props.prop.value); + } + }); + + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {prop: {value: 'hi'}}) + ); + + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toBe( + 'Warning: Component: the type of prop `prop` is invalid; the type ' + + 'checker function must return `null` or an `Error`, but returned a ' + + 'function. You may have forgotten to pass an argument to the type ' + + 'checker creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, ' + + 'and shape all require an argument).' + ); + }); + it('should warn if a fragment is used without the wrapper', function() { spyOn(console, 'warn'); var child = React.createElement('span'); diff --git a/src/classic/types/__tests__/ReactPropTypes-test.js b/src/classic/types/__tests__/ReactPropTypes-test.js index f87856491a..d1683e9d52 100644 --- a/src/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/classic/types/__tests__/ReactPropTypes-test.js @@ -777,11 +777,11 @@ describe('ReactPropTypes', function() { 'Warning: Failed propType: num must be 5!'); }); - it('should not warn if the validator returned anything else than an error', + it('should not warn if the validator returned null', function() { var spy = jasmine.createSpy().andCallFake( function(props, propName, componentName) { - return 'This message will never reach anyone'; + return null; } ); var Component = React.createClass({