@samitouri / QOS-React-2 / commits / 82d15c8fd5

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.

Sebastian Markbage committed Dec 29, 2014 at 12:43 UTC 82d15c8fd52811dfca22609d73ac9dcb3871adcd
4 files changed +101 -2
src/classic/class/ReactClass.js
+6
@@ -868,6 +868,12 @@ var ReactClass = {
868 // Initialize the defaultProps property after all mixins have been merged
869 if (Constructor.getDefaultProps) {
870 Constructor.defaultProps = Constructor.getDefaultProps();
871 + if (__DEV__) {
872 + // This is a tag to indicate that this use of getDefaultProps is ok,
873 + // since it's used with createClass. If it's not, then it's likely a
874 + // mistake so we'll warn you to use the static property instead.
875 + Constructor.getDefaultProps._isReactClassApproved = true;
876 + }
877 }
878
879 invariant(
src/classic/element/ReactElementValidator.js
+20
@@ -20,10 +20,12 @@
20
21 var ReactElement = require('ReactElement');
22 var ReactPropTypeLocations = require('ReactPropTypeLocations');
23 +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
24 var ReactCurrentOwner = require('ReactCurrentOwner');
25
26 var getIteratorFn = require('getIteratorFn');
27 var monitorCodeUse = require('monitorCodeUse');
28 +var invariant = require('invariant');
29 var warning = require('warning');
30
31 /**
@@ -236,6 +238,16 @@ function checkPropTypes(componentName, propTypes, props, location) {
238 // fail the render phase where it didn't fail before. So we log it.
239 // After these have been cleaned up, we'll let them throw.
240 try {
241 + // This is intentionally an invariant that gets caught. It's the same
242 + // behavior as without this statement except with a better message.
243 + invariant(
244 + typeof propTypes[propName] === 'function',
245 + '%s: %s type `%s` is invalid; it must be a function, usually from ' +
246 + 'React.PropTypes.',
247 + componentName || 'React class',
248 + ReactPropTypeLocationNames[location],
249 + propName
250 + );
251 error = propTypes[propName](props, propName, componentName, location);
252 } catch (ex) {
253 error = ex;
@@ -296,7 +308,15 @@ var ReactElementValidator = {
308 ReactPropTypeLocations.context
309 );
310 }
311 + if (typeof type.getDefaultProps === 'function') {
312 + warning(
313 + type.getDefaultProps._isReactClassApproved,
314 + 'getDefaultProps is only used on classic React.createClass ' +
315 + 'definitions. Use a static property named `defaultProps` instead.'
316 + );
317 + }
318 }
319 +
320 return element;
321 },
322
src/core/ReactCompositeComponent.js
+17 -2
@@ -19,6 +19,7 @@ var ReactElement = require('ReactElement');
19 var ReactInstanceMap = require('ReactInstanceMap');
20 var ReactPerf = require('ReactPerf');
21 var ReactPropTypeLocations = require('ReactPropTypeLocations');
22 +var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
23 var ReactUpdates = require('ReactUpdates');
24
25 var assign = require('Object.assign');
@@ -515,8 +516,22 @@ var ReactCompositeComponentMixin = assign({},
516 this._instance.constructor.name;
517 for (var propName in propTypes) {
518 if (propTypes.hasOwnProperty(propName)) {
518 - var error =
519 - propTypes[propName](props, propName, componentName, location);
519 + var error;
520 + try {
521 + // This is intentionally an invariant that gets caught. It's the same
522 + // behavior as without this statement except with a better message.
523 + invariant(
524 + typeof propTypes[propName] === 'function',
525 + '%s: %s type `%s` is invalid; it must be a function, usually ' +
526 + 'from React.PropTypes.',
527 + componentName || 'React class',
528 + ReactPropTypeLocationNames[location],
529 + propName
530 + );
531 + error = propTypes[propName](props, propName, componentName, location);
532 + } catch (ex) {
533 + error = ex;
534 + }
535 if (error instanceof Error) {
536 // We may want to extend this logic for similar errors in
537 // React.render calls, so I'm abstracting it away into
src/modern/element/__tests__/ReactJSXElementValidator-test.js
+58
@@ -290,4 +290,62 @@ describe('ReactJSXElementValidator', function() {
290 expect(console.warn.calls.length).toBe(2);
291 });
292
293 + it('should warn on invalid prop types', function() {
294 + // Since there is no prevalidation step for ES6 classes, there is no hook
295 + // for us to issue a warning earlier than element creation when the error
296 + // actually occurs. Since this step is skipped in production, we should just
297 + // warn instead of throwing for this case.
298 + spyOn(console, 'warn');
299 + class Component {
300 + render() {
301 + return <span>{this.props.prop}</span>;
302 + }
303 + }
304 + Component.propTypes = {
305 + prop: null
306 + };
307 + ReactTestUtils.renderIntoDocument(<Component />);
308 + expect(console.warn.calls.length).toBe(1);
309 + expect(console.warn.calls[0].args[0]).toContain(
310 + 'Invariant Violation: Component: prop type `prop` is invalid; ' +
311 + 'it must be a function, usually from React.PropTypes.'
312 + );
313 + });
314 +
315 + it('should warn on invalid context types', function() {
316 + spyOn(console, 'warn');
317 + class Component {
318 + render() {
319 + return <span>{this.props.prop}</span>;
320 + }
321 + }
322 + Component.contextTypes = {
323 + prop: null
324 + };
325 + ReactTestUtils.renderIntoDocument(<Component />);
326 + expect(console.warn.calls.length).toBe(1);
327 + expect(console.warn.calls[0].args[0]).toContain(
328 + 'Invariant Violation: Component: context type `prop` is invalid; ' +
329 + 'it must be a function, usually from React.PropTypes.'
330 + );
331 + });
332 +
333 + it('should warn if getDefaultProps is specificed on the class', function() {
334 + spyOn(console, 'warn');
335 + class Component {
336 + render() {
337 + return <span>{this.props.prop}</span>;
338 + }
339 + }
340 + Component.getDefaultProps = () => ({
341 + prop: 'foo'
342 + });
343 + ReactTestUtils.renderIntoDocument(<Component />);
344 + expect(console.warn.calls.length).toBe(1);
345 + expect(console.warn.calls[0].args[0]).toContain(
346 + 'getDefaultProps is only used on classic React.createClass definitions.' +
347 + ' Use a static property named `defaultProps` instead.'
348 + );
349 + });
350 +
351 });