From e8a6121a6bcd4a776fe88048d86a552e4ae8103f Mon Sep 17 00:00:00 2001 From: cpojer Date: Thu, 16 Jan 2014 11:10:57 -0800 Subject: [PATCH 1/2] Change ReactPropTypes invariant's to console.warn. --- src/core/ReactCompositeComponent.js | 32 +- src/core/ReactPropTypes.js | 20 +- .../__tests__/ReactCompositeComponent-test.js | 129 +++-- src/core/__tests__/ReactPropTypes-test.js | 547 ++++++++++++------ src/vendor/core/warning.js | 42 ++ 5 files changed, 495 insertions(+), 275 deletions(-) create mode 100644 src/vendor/core/warning.js diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index f730955e125..54f3a2b0f85 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -837,11 +837,13 @@ var ReactCompositeComponentMixin = { for (var contextName in contextTypes) { maskedContext[contextName] = context[contextName]; } - this._checkPropTypes( - contextTypes, - maskedContext, - ReactPropTypeLocations.context - ); + if (__DEV__) { + this._checkPropTypes( + contextTypes, + maskedContext, + ReactPropTypeLocations.context + ); + } } return maskedContext; }, @@ -861,11 +863,13 @@ var ReactCompositeComponentMixin = { 'use getChildContext().', displayName ); - this._checkPropTypes( - this.constructor.childContextTypes, - childContext, - ReactPropTypeLocations.childContext - ); + if (__DEV__) { + this._checkPropTypes( + this.constructor.childContextTypes, + childContext, + ReactPropTypeLocations.childContext + ); + } for (var name in childContext) { invariant( name in this.constructor.childContextTypes, @@ -896,9 +900,11 @@ var ReactCompositeComponentMixin = { props[propName] = defaultProps[propName]; } } - var propTypes = this.constructor.propTypes; - if (propTypes) { - this._checkPropTypes(propTypes, props, ReactPropTypeLocations.prop); + if (__DEV__) { + var propTypes = this.constructor.propTypes; + if (propTypes) { + this._checkPropTypes(propTypes, props, ReactPropTypeLocations.prop); + } } return props; }, diff --git a/src/core/ReactPropTypes.js b/src/core/ReactPropTypes.js index 10fe5fa590b..9c9fe2b647d 100644 --- a/src/core/ReactPropTypes.js +++ b/src/core/ReactPropTypes.js @@ -21,8 +21,8 @@ var ReactComponent = require('ReactComponent'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); +var warning = require('warning'); var createObjectFrom = require('createObjectFrom'); -var invariant = require('invariant'); /** * Collection of methods that allow declaration and validation of props that are @@ -57,7 +57,7 @@ var invariant = require('invariant'); * // An optional string or URI prop named "href". * href: function(props, propName, componentName) { * var propValue = props[propName]; - * invariant( + * warning( * propValue == null || * typeof propValue === 'string' || * propValue instanceof URI, @@ -147,7 +147,7 @@ function createPrimitiveTypeChecker(expectedType) { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` of type `%s` supplied to `%s`, expected `%s`.', ReactPropTypeLocationNames[location], @@ -169,7 +169,7 @@ function createEnumTypeChecker(expectedValues) { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` supplied to `%s`, expected one of %s.', ReactPropTypeLocationNames[location], @@ -199,7 +199,7 @@ function createShapeTypeChecker(shapeTypes) { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` of type `%s` supplied to `%s`, expected `object`.', ReactPropTypeLocationNames[location], @@ -219,7 +219,7 @@ function createInstanceTypeChecker(expectedClass) { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` supplied to `%s`, expected instance of `%s`.', ReactPropTypeLocationNames[location], @@ -239,7 +239,7 @@ function createRenderableTypeChecker() { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` supplied to `%s`, expected a renderable prop.', ReactPropTypeLocationNames[location], @@ -258,7 +258,7 @@ function createComponentTypeChecker() { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Invalid %s `%s` supplied to `%s`, expected a React component.', ReactPropTypeLocationNames[location], @@ -288,7 +288,7 @@ function createChainableTypeChecker(validate) { if (!shouldThrow) { return isValid; } - invariant( + warning( isValid, 'Required %s `%s` was not specified in `%s`.', ReactPropTypeLocationNames[location], @@ -320,7 +320,7 @@ function createUnionTypeChecker(arrayOfValidators) { break; } } - invariant( + warning( isValid, 'Invalid %s `%s` supplied to `%s`.', ReactPropTypeLocationNames[location], diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index a0695f1eed1..f1e67d8eeed 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -32,6 +32,7 @@ var ReactDoNotBindDeprecated; var cx; var reactComponentExpect; var mocks; +var warn; describe('ReactCompositeComponent', function() { @@ -80,6 +81,13 @@ describe('ReactCompositeComponent', function() { ; } }); + + warn = console.warn; + console.warn = mocks.getMockFunction(); + }); + + afterEach(function() { + console.warn = warn; }); it('should support rendering to different child types over time', function() { @@ -275,11 +283,11 @@ describe('ReactCompositeComponent', function() { reactComponentExpect(instance).scalarPropsEqual({key: 'testKey'}); reactComponentExpect(instance).scalarStateEqual({key: 'testKeyState'}); - expect(function() { - ReactTestUtils.renderIntoDocument(); - }).toThrow( - 'Invariant Violation: Required prop `key` was not specified in ' + - '`Component`.' + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `key` was not specified in `Component`.' ); }); @@ -294,12 +302,11 @@ describe('ReactCompositeComponent', function() { } }); - var instance = ; - expect(function() { - ReactTestUtils.renderIntoDocument(instance); - }).toThrow( - 'Invariant Violation: Required prop `key` was not specified in ' + - '`Component`.' + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `key` was not specified in `Component`.' ); }); @@ -313,23 +320,23 @@ describe('ReactCompositeComponent', function() { } }); - expect(function() { - ReactTestUtils.renderIntoDocument(); - }).toThrow( - 'Invariant Violation: Required prop `key` was not specified in ' + - '`Component`.' + ReactTestUtils.renderIntoDocument(); + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(2); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `key` was not specified in `Component`.' ); - expect(function() { - ReactTestUtils.renderIntoDocument(); - }).toThrow( - 'Invariant Violation: Invalid prop `key` of type `number` supplied to ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `key` of type `number` supplied to ' + '`Component`, expected `string`.' ); - expect(function() { - ReactTestUtils.renderIntoDocument(); - }).not.toThrow(); + ReactTestUtils.renderIntoDocument(); + + // Should not error for strings + expect(console.warn.mock.calls.length).toBe(2); }); it('should throw on invalid prop types', function() { @@ -853,25 +860,27 @@ describe('ReactCompositeComponent', function() { } }); - expect(function() { - ReactTestUtils.renderIntoDocument(); - }).toThrow( - 'Invariant Violation: Required context `foo` was not specified in ' + - '`Component`.' + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required context `foo` was not specified in `Component`.' ); - expect(function() { - React.withContext({foo: 'bar'}, function() { - ReactTestUtils.renderIntoDocument(); - }); - }).not.toThrow(); + React.withContext({foo: 'bar'}, function() { + ReactTestUtils.renderIntoDocument(); + }); - expect(function() { - React.withContext({foo: 123}, function() { - ReactTestUtils.renderIntoDocument(); - }); - }).toThrow( - 'Invariant Violation: Invalid context `foo` of type `number` supplied ' + + // Previous call should not error + expect(console.warn.mock.calls.length).toBe(1); + + React.withContext({foo: 123}, function() { + ReactTestUtils.renderIntoDocument(); + }); + + expect(console.warn.mock.calls.length).toBe(2); + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid context `foo` of type `number` supplied ' + 'to `Component`, expected `string`.' ); }); @@ -892,35 +901,31 @@ describe('ReactCompositeComponent', function() { } }); - expect(function() { - ReactTestUtils.renderIntoDocument( - - ); - }).toThrow( - 'Invariant Violation: Required child context `foo` was not specified ' + - 'in `Component`.' + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required child context `foo` was not specified in `Component`.' ); - expect(function() { - ReactTestUtils.renderIntoDocument( - - ); - }).toThrow( - 'Invariant Violation: Invalid child context `foo` of type `number` ' + + ReactTestUtils.renderIntoDocument(); + + expect(console.warn.mock.calls.length).toBe(2); + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid child context `foo` of type `number` ' + 'supplied to `Component`, expected `string`.' ); - expect(function() { - ReactTestUtils.renderIntoDocument( - - ); - }).not.toThrow(); + ReactTestUtils.renderIntoDocument( + + ); - expect(function() { - ReactTestUtils.renderIntoDocument( - - ); - }).not.toThrow(); + ReactTestUtils.renderIntoDocument( + + ); + + // Previous calls should not log errors + expect(console.warn.mock.calls.length).toBe(2); }); it('should filter out context not in contextTypes', function() { diff --git a/src/core/__tests__/ReactPropTypes-test.js b/src/core/__tests__/ReactPropTypes-test.js index 8c2db8606dd..38ab4a8eb9a 100644 --- a/src/core/__tests__/ReactPropTypes-test.js +++ b/src/core/__tests__/ReactPropTypes-test.js @@ -25,17 +25,16 @@ var Props = require('ReactPropTypes'); var React = require('React'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); +var warn; +var mocks; + function typeCheck(declaration, value) { var props = {}; if (arguments.length > 1) { props.testProp = value; } - return declaration.bind( - null, - props, - 'testProp', - 'testComponent', - ReactPropTypeLocations.prop + return declaration( + props, 'testProp', 'testComponent', ReactPropTypeLocations.prop ); } @@ -49,156 +48,240 @@ describe('Primitive Types', function() { beforeEach(function() { require('mock-modules').dumpCache(); + mocks = require('mocks'); ReactTestUtils = require('ReactTestUtils'); + + warn = console.warn; + console.warn = mocks.getMockFunction(); }); - it("should throw for invalid strings", function() { - expect(typeCheck(Props.string, [])).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `array` ' + + afterEach(function() { + console.warn = warn; + }); + + it("should warn for invalid strings", function() { + typeCheck(Props.string, []); + typeCheck(Props.string, false); + typeCheck(Props.string, 1); + typeCheck(Props.string, {}); + + expect(console.warn.mock.calls.length).toBe(4); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` of type `array` ' + 'supplied to `testComponent`, expected `string`.' ); - expect(typeCheck(Props.string, false)).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `boolean` ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` of type `boolean` ' + 'supplied to `testComponent`, expected `string`.' ); - expect(typeCheck(Props.string, 1)).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `number` ' + + expect(console.warn.mock.calls[2][0]).toBe( + 'Warning: Invalid prop `testProp` of type `number` ' + 'supplied to `testComponent`, expected `string`.' ); - expect(typeCheck(Props.string, {})).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `object` ' + + expect(console.warn.mock.calls[3][0]).toBe( + 'Warning: Invalid prop `testProp` of type `object` ' + 'supplied to `testComponent`, expected `string`.' ); }); - it("should not throw for valid values", function() { - expect(typeCheck(Props.array, [])).not.toThrow(); - expect(typeCheck(Props.bool, false)).not.toThrow(); - expect(typeCheck(Props.func, function() {})).not.toThrow(); - expect(typeCheck(Props.number, 0)).not.toThrow(); - expect(typeCheck(Props.object, {})).not.toThrow(); - expect(typeCheck(Props.string, '')).not.toThrow(); + it("should not warn for valid values", function() { + typeCheck(Props.array, []); + typeCheck(Props.bool, false); + typeCheck(Props.func, function() {}); + typeCheck(Props.number, 0); + typeCheck(Props.object, {}); + typeCheck(Props.string, ''); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should be implicitly optional and not throw without values", function() { - expect(typeCheck(Props.string, null)).not.toThrow(); - expect(typeCheck(Props.string, undefined)).not.toThrow(); + it("should be implicitly optional and not warn without values", function() { + typeCheck(Props.string, null); + typeCheck(Props.string, undefined); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should throw for missing required values", function() { - expect(typeCheck(Props.string.isRequired, null)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + it("should warn for missing required values", function() { + typeCheck(Props.string.isRequired, null); + typeCheck(Props.string.isRequired, undefined); + + expect(console.warn.mock.calls.length).toBe(2); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); - expect(typeCheck(Props.string.isRequired, undefined)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); }); it("should have a weak version that returns true/false", function() { - expect(typeCheck(Props.string.weak, null)()).toEqual(true); - expect(typeCheck(Props.string.weak.isRequired, null)()).toEqual(false); - expect(typeCheck(Props.string.isRequired.weak, null)()).toEqual(false); + expect(typeCheck(Props.string.weak, null)).toEqual(true); + expect(typeCheck(Props.string.weak.isRequired, null)).toEqual(false); + expect(typeCheck(Props.string.isRequired.weak, null)).toEqual(false); }); }); describe('Enum Types', function() { beforeEach(function() { require('mock-modules').dumpCache(); + mocks = require('mocks'); + warn = console.warn; + console.warn = mocks.getMockFunction(); }); - it("should throw for invalid strings", function() { - expect(typeCheck(Props.oneOf(['red', 'blue']), true)).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + afterEach(function() { + console.warn = warn; + }); + + it("should warn for invalid strings", function() { + typeCheck(Props.oneOf(['red', 'blue']), true); + typeCheck(Props.oneOf(['red', 'blue']), []); + typeCheck(Props.oneOf(['red', 'blue']), ''); + + expect(console.warn.mock.calls.length).toBe(3); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected one of ["blue","red"].' ); - expect(typeCheck(Props.oneOf(['red', 'blue']), [])).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected one of ["blue","red"].' ); - expect(typeCheck(Props.oneOf(['red', 'blue']), '')).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[2][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected one of ["blue","red"].' ); }); - it("should not throw for valid values", function() { - expect(typeCheck(Props.oneOf(['red', 'blue']), 'red')).not.toThrow(); - expect(typeCheck(Props.oneOf(['red', 'blue']), 'blue')).not.toThrow(); + it("should not warn for valid values", function() { + typeCheck(Props.oneOf(['red', 'blue']), 'red'); + typeCheck(Props.oneOf(['red', 'blue']), 'blue'); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should be implicitly optional and not throw without values", function() { - expect(typeCheck(Props.oneOf(['red', 'blue']), null)).not.toThrow(); - expect(typeCheck(Props.oneOf(['red', 'blue']), undefined)).not.toThrow(); + it("should be implicitly optional and not warn without values", function() { + typeCheck(Props.oneOf(['red', 'blue']), null); + typeCheck(Props.oneOf(['red', 'blue']), undefined); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); it("should have a weak version that returns true/false", function() { var checker = Props.oneOf(['red', 'blue']); - expect(typeCheck(checker.weak, null)()).toEqual(true); - expect(typeCheck(checker.weak.isRequired, null)()).toEqual(false); - expect(typeCheck(checker.isRequired.weak, null)()).toEqual(false); + expect(typeCheck(checker.weak, null)).toEqual(true); + expect(typeCheck(checker.weak.isRequired, null)).toEqual(false); + expect(typeCheck(checker.isRequired.weak, null)).toEqual(false); }); }); describe('Shape Types', function() { beforeEach(function() { require('mock-modules').dumpCache(); + mocks = require('mocks'); + warn = console.warn; + console.warn = mocks.getMockFunction(); + }); + + afterEach(function() { + console.warn = warn; }); - it("should throw for non objects", function() { - expect(typeCheck(Props.shape({}), 'some string')).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `string` ' + + it("should warn for non objects", function() { + typeCheck(Props.shape({}), 'some string'); + typeCheck(Props.shape({}), ['array']); + + expect(console.warn.mock.calls.length).toBe(2); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` of type `string` ' + 'supplied to `testComponent`, expected `object`.' ); - expect(typeCheck(Props.shape({}), ['array'])).toThrow( - 'Invariant Violation: Invalid prop `testProp` of type `array` ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` of type `array` ' + 'supplied to `testComponent`, expected `object`.' ); }); - it("should not throw for empty values", function() { - expect(typeCheck(Props.shape({}), undefined)).not.toThrow(); - expect(typeCheck(Props.shape({}), null)).not.toThrow(); - expect(typeCheck(Props.shape({}), {})).not.toThrow(); + it("should not warn for empty values", function() { + typeCheck(Props.shape({}), undefined); + typeCheck(Props.shape({}), null); + typeCheck(Props.shape({}), {}); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should throw for empty required value", function() { - expect(typeCheck(Props.shape({}).isRequired, undefined)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + it("should warn for empty required value", function() { + typeCheck(Props.shape({}).isRequired, undefined); + typeCheck(Props.shape({}).isRequired, null); + + expect(console.warn.mock.calls.length).toBe(2); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); - expect(typeCheck(Props.shape({}).isRequired, null)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); - expect(typeCheck(Props.shape({}).isRequired, {})).not.toThrow(); + + // Should not warn + typeCheck(Props.shape({}).isRequired, {}); + expect(console.warn.mock.calls.length).toBe(2); }); - it("should not throw for non specified types", function() { - expect(typeCheck(Props.shape({}), {key: 1})).not.toThrow(); + it("should not warn for non specified types", function() { + typeCheck(Props.shape({}), {key: 1}); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should not throw for valid types", function() { - expect(typeCheck(Props.shape({ + it("should not warn for valid types", function() { + typeCheck(Props.shape({ key: Props.number - }), {key: 1})).not.toThrow(); + }), {key: 1}); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it("should throw for required valid types", function() { - expect(typeCheck(Props.shape({ + it("should warn for required valid types", function() { + typeCheck(Props.shape({ key: Props.number.isRequired - }), {})).toThrow( - 'Invariant Violation: Required prop `key` was not specified in ' + + }), {}); + + expect(console.warn.mock.calls.length).toBe(1); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `key` was not specified in ' + '`testComponent`.' ); }); - it("should throw for invalid key types", function() { - expect(typeCheck(Props.shape({ + it("should warn for invalid key types", function() { + typeCheck(Props.shape({ key: Props.number - }), {key: 'abc'})).toThrow( - 'Invariant Violation: Invalid prop `key` of type `string` supplied to ' + + }), {key: 'abc'}); + + expect(console.warn.mock.calls.length).toBe(1); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `key` of type `string` supplied to ' + '`testComponent`, expected `number`.' ); }); @@ -207,45 +290,76 @@ describe('Shape Types', function() { describe('Instance Types', function() { beforeEach(function() { require('mock-modules').dumpCache(); + mocks = require('mocks'); + warn = console.warn; + console.warn = mocks.getMockFunction(); + }); + + afterEach(function() { + console.warn = warn; }); - it("should throw for invalid instances", function() { + it("should warn for invalid instances", function() { function Person() {} var name = Person.name || '<>'; + typeCheck(Props.instanceOf(Person), false); + typeCheck(Props.instanceOf(Person), {}); + typeCheck(Props.instanceOf(Person), ''); - expect(typeCheck(Props.instanceOf(Person), false)).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + expect(console.warn.mock.calls.length).toBe(3); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected instance of `' + name + '`.' ); - expect(typeCheck(Props.instanceOf(Person), {})).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected instance of `' + name + '`.' ); - expect(typeCheck(Props.instanceOf(Person), '')).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[2][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected instance of `' + name + '`.' ); }); - it("should not throw for valid values", function() { + it("should not warn for valid values", function() { function Person() {} function Engineer() {} Engineer.prototype = new Person(); - expect(typeCheck(Props.instanceOf(Person), new Person())).not.toThrow(); - expect(typeCheck(Props.instanceOf(Person), new Engineer())).not.toThrow(); + typeCheck(Props.instanceOf(Person), new Person()); + typeCheck(Props.instanceOf(Person), new Engineer()); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); }); describe('Component Type', function() { + beforeEach(function() { + require('mock-modules').dumpCache(); + mocks = require('mocks'); + warn = console.warn; + console.warn = mocks.getMockFunction(); + }); + + afterEach(function() { + console.warn = warn; + }); + it('should support components', () => { - expect(typeCheck(Props.component,
)).not.toThrow(); + typeCheck(Props.component,
); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); it('should not support multiple components or scalar values', () => { - [[
,
], 123, 'foo', false].forEach((value) => { - expect(typeCheck(Props.component, value)).toThrow(); - }); + var list = [[
,
], 123, 'foo', false]; + list.forEach((value) => typeCheck(Props.component, value)); + expect(console.warn.mock.calls.length).toBe(list.length); }); var Component = React.createClass({ @@ -259,75 +373,93 @@ describe('Component Type', function() { }); it('should be able to define a single child as children', () => { - expect(() => { - var instance = - -
- ; - ReactTestUtils.renderIntoDocument(instance); - }).not.toThrow(); - }); - - it('should throw when passing more than one child', () => { - expect(() => { - var instance = - -
-
- ; - ReactTestUtils.renderIntoDocument(instance); - }).toThrow(); - }); - - it('should throw when passing no children and isRequired is set', () => { - expect(() => { - var instance = ; - ReactTestUtils.renderIntoDocument(instance); - }).toThrow(); + var instance = + +
+ ; + ReactTestUtils.renderIntoDocument(instance); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); + }); + + it('should warn when passing more than one child', () => { + var instance = + +
+
+ ; + ReactTestUtils.renderIntoDocument(instance); + expect(console.warn.mock.calls.length).toBe(1); + }); + + it('should warn when passing no children and isRequired is set', () => { + var instance = ; + ReactTestUtils.renderIntoDocument(instance); + + expect(console.warn.mock.calls.length).toBe(1); }); }); describe('Union Types', function() { beforeEach(function() { require('mock-modules').dumpCache(); + mocks = require('mocks'); + warn = console.warn; + console.warn = mocks.getMockFunction(); }); - it('should throw if none of the types are valid', function() { + afterEach(function() { + console.warn = warn; + }); + + it('should warn if none of the types are valid', function() { var checker = Props.oneOfType([ Props.string, Props.number ]); - expect(typeCheck(checker, [])).toThrow( - 'Invariant Violation: Invalid prop `testProp` ' + - 'supplied to `testComponent`.' - ); - + typeCheck(checker, []); checker = Props.oneOfType([ Props.string.isRequired, Props.number.isRequired ]); - expect(typeCheck(checker, null)).toThrow( - 'Invariant Violation: Invalid prop `testProp` ' + + typeCheck(checker, null); + + expect(console.warn.mock.calls.length).toBe(2); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` ' + + 'supplied to `testComponent`.' + ); + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` ' + 'supplied to `testComponent`.' ); }); - it('should not throw if one of the types are valid', function() { + it('should not warn if one of the types are valid', function() { var checker = Props.oneOfType([ Props.string, Props.number ]); - expect(typeCheck(checker, null)).not.toThrow(); - expect(typeCheck(checker, 'foo')).not.toThrow(); - expect(typeCheck(checker, 123)).not.toThrow(); + typeCheck(checker, null); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(checker, 'foo'); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(checker, 123); + expect(console.warn.mock.calls.length).toBe(0); checker = Props.oneOfType([ Props.string, Props.number.isRequired ]); - expect(typeCheck(checker, null)).not.toThrow(); - expect(typeCheck(checker, 'foo')).not.toThrow(); - expect(typeCheck(checker, 123)).not.toThrow(); + typeCheck(checker, null); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(checker, 'foo'); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(checker, 123); + expect(console.warn.mock.calls.length).toBe(0); }); describe('React Component Types', function() { @@ -337,103 +469,138 @@ describe('Union Types', function() { var myFunc = function() {}; - it('should throw for invalid values', function() { - expect(typeCheck(Props.renderable, false)).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + it('should warn for invalid values', function() { + typeCheck(Props.renderable, false); + typeCheck(Props.renderable, myFunc); + typeCheck(Props.renderable, {key: myFunc}); + + expect(console.warn.mock.calls.length).toBe(3); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected a renderable prop.' ); - expect(typeCheck(Props.renderable, myFunc)).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected a renderable prop.' ); - expect(typeCheck(Props.renderable, {key: myFunc})).toThrow( - 'Invariant Violation: Invalid prop `testProp` supplied to ' + + + expect(console.warn.mock.calls[2][0]).toBe( + 'Warning: Invalid prop `testProp` supplied to ' + '`testComponent`, expected a renderable prop.' ); }); - it('should not throw for valid values', function() { + it('should not warn for valid values', function() { // DOM component - expect(typeCheck(Props.renderable,
)).not.toThrow(); + typeCheck(Props.renderable,
); + expect(console.warn.mock.calls.length).toBe(0); // Custom component - expect(typeCheck(Props.renderable, )).not.toThrow(); + typeCheck(Props.renderable, ); + expect(console.warn.mock.calls.length).toBe(0); // String - expect(typeCheck(Props.renderable, 'Some string')).not.toThrow(); + typeCheck(Props.renderable, 'Some string'); + expect(console.warn.mock.calls.length).toBe(0); // Empty array - expect(typeCheck(Props.renderable, [])).not.toThrow(); + typeCheck(Props.renderable, []); + expect(console.warn.mock.calls.length).toBe(0); // Empty object - expect(typeCheck(Props.renderable, {})).not.toThrow(); + typeCheck(Props.renderable, {}); + expect(console.warn.mock.calls.length).toBe(0); // Array of renderable things - expect( - typeCheck(Props.renderable, [ - 123, - 'Some string', -
, - ['Another string', [456], , ], - - ]) - ).not.toThrow(); + + typeCheck(Props.renderable, [ + 123, + 'Some string', +
, + ['Another string', [456], , ], + + ]); + expect(console.warn.mock.calls.length).toBe(0); + // Object of rendereable things - expect( - typeCheck(Props.renderable, { - k0: 123, - k1: 'Some string', - k2:
, - k3: { - k30: , - k31: {k310: }, - k32: 'Another string' - } - }) - ).not.toThrow(); + typeCheck(Props.renderable, { + k0: 123, + k1: 'Some string', + k2:
, + k3: { + k30: , + k31: {k310: }, + k32: 'Another string' + } + }); + expect(console.warn.mock.calls.length).toBe(0); }); - it('should not throw for null/undefined if not required', function() { - expect(typeCheck(Props.renderable, null)).not.toThrow(); - expect(typeCheck(Props.renderable, undefined)).not.toThrow(); + it('should not warn for null/undefined if not required', function() { + typeCheck(Props.renderable, null); + typeCheck(Props.renderable, undefined); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); - it('should throw for missing required values', function() { - expect(typeCheck(Props.renderable.isRequired, null)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + it('should warn for missing required values', function() { + typeCheck(Props.renderable.isRequired, null); + typeCheck(Props.renderable.isRequired, undefined); + + expect(console.warn.mock.calls.length).toBe(2); + + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); - expect(typeCheck(Props.renderable.isRequired, undefined)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); }); it('should accept empty array & object for required props', function() { - expect(typeCheck(Props.renderable.isRequired, [])).not.toThrow(); - expect(typeCheck(Props.renderable.isRequired, {})).not.toThrow(); + typeCheck(Props.renderable.isRequired, []); + typeCheck(Props.renderable.isRequired, {}); + + // No warnings should have been logged. + expect(console.warn.mock.calls.length).toBe(0); }); }); describe('Any type', function() { it('should should accept any value', function() { - expect(typeCheck(Props.any, 1)).not.toThrow(); - expect(typeCheck(Props.any, 'str')).not.toThrow(); - expect(typeCheck(Props.any.isRequired, 1)).not.toThrow(); - expect(typeCheck(Props.any.isRequired, 'str')).not.toThrow(); - - expect(typeCheck(Props.any, null)).not.toThrow(); - expect(typeCheck(Props.any, undefined)).not.toThrow(); - - expect(typeCheck(Props.any.isRequired, null)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + typeCheck(Props.any, 1); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(Props.any, 'str'); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(Props.any.isRequired, 1); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(Props.any.isRequired, 'str'); + expect(console.warn.mock.calls.length).toBe(0); + + typeCheck(Props.any, null); + expect(console.warn.mock.calls.length).toBe(0); + typeCheck(Props.any, undefined); + expect(console.warn.mock.calls.length).toBe(0); + + typeCheck(Props.any.isRequired, null); + typeCheck(Props.any.isRequired, undefined); + + expect(console.warn.mock.calls.length).toBe(2); + expect(console.warn.mock.calls[0][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); - expect(typeCheck(Props.any.isRequired, undefined)).toThrow( - 'Invariant Violation: Required prop `testProp` was not specified in ' + + + expect(console.warn.mock.calls[1][0]).toBe( + 'Warning: Required prop `testProp` was not specified in ' + '`testComponent`.' ); }); it('should have a weak version that returns true/false', function() { - expect(typeCheck(Props.any.weak, null)()).toEqual(true); - expect(typeCheck(Props.any.weak.isRequired, null)()).toEqual(false); - expect(typeCheck(Props.any.isRequired.weak, null)()).toEqual(false); + expect(typeCheck(Props.any.weak, null)).toEqual(true); + expect(typeCheck(Props.any.weak.isRequired, null)).toEqual(false); + expect(typeCheck(Props.any.isRequired.weak, null)).toEqual(false); }); }); }); diff --git a/src/vendor/core/warning.js b/src/vendor/core/warning.js new file mode 100644 index 00000000000..2b5b6524124 --- /dev/null +++ b/src/vendor/core/warning.js @@ -0,0 +1,42 @@ +/** + * 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 warning + */ + + +/** + * Similar to invariant but only logs a warning if the condition is not met. + * This can be used to log issues in development environments in critical + * paths. Removing the logging code for production environments will keep the + * same logic and follow the same code paths. + */ +function warning(condition, format, ...args) { + if (format === undefined) { + throw new Error( + '`warning(condition, format, ...args)` requires an error message argument' + ); + } + + if (!condition) { + var argIndex = 0; + console.warn( + 'Warning: ' + + format.replace(/%s/g, () => args[argIndex++]) + ); + } +} + +module.exports = warning; From 39cbd7e0d5e2798e508906bcd8d948d7c6ef24fa Mon Sep 17 00:00:00 2001 From: cpojer Date: Fri, 17 Jan 2014 10:28:26 -0800 Subject: [PATCH 2/2] Perf Test for the propTypes change. --- perf/index.html | 1 + perf/tests/propTypes.js | 85 ++++++++++++++++++++++++++++++++++++++ src/vendor/core/warning.js | 8 ++-- 3 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 perf/tests/propTypes.js diff --git a/perf/index.html b/perf/index.html index 4b91dda47cb..71d99c0a8d6 100644 --- a/perf/index.html +++ b/perf/index.html @@ -29,6 +29,7 @@ "setState-callback.js", "basic-div.js", "basic-unmount.js", + "propTypes.js", "renderComponent-basic.js", "shouldComponentUpdate.js", ]; diff --git a/perf/tests/propTypes.js b/perf/tests/propTypes.js new file mode 100644 index 00000000000..6c2a0f8346e --- /dev/null +++ b/perf/tests/propTypes.js @@ -0,0 +1,85 @@ +if (typeof exports == 'undefined') exports = {}; + +/*http://benchmarkjs.com/docs#options*/ + +exports.name = 'propTypes'; + +var Thing = function() {}; +var List; +var ListItem; +var MyReactComponent; +var _rootNode; + +exports.setup = function(){ + List = React.createClass({ + propTypes: { + array: React.PropTypes.array, + bool: React.PropTypes.bool.isRequired, + number: React.PropTypes.number, + string: React.PropTypes.string.isRequired, + func: React.PropTypes.func.isRequired, + renderable: React.PropTypes.renderable.isRequired, + instanceOf: React.PropTypes.instanceOf(Thing).isRequired + }, + render: function() { + return React.DOM.ul(null, this.props.children); + } + }); + + ListItem = React.createClass({ + propTypes: { + array: React.PropTypes.array, + bool: React.PropTypes.bool.isRequired, + number: React.PropTypes.number, + string: React.PropTypes.string.isRequired, + func: React.PropTypes.func.isRequired, + renderable: React.PropTypes.renderable.isRequired, + renderable2: React.PropTypes.renderable.isRequired, + instanceOf: React.PropTypes.instanceOf(Thing).isRequired, + component: React.PropTypes.component.isRequired + }, + render: function(){ + return React.DOM.li(null, + this.props.number + this.props.string + this.props.renderable + ); + } + }); + + MyReactComponent = React.createClass({ + render: function() { + return React.DOM.span(); + } + }); + + _rootNode = document.createElement('div'); + document.body.appendChild(_rootNode); +}; +exports.fn = function(){ + var items = []; + for (var i = 0; i < 1000; i++) { + items.push(ListItem({ + array: [11, 12, 13, 14, 15, 16, 17, 18, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], + bool: false, + number: Math.random(), + string: 'banana banana banana', + func: function() { return 'this is a function'; }, + renderable: 'renderable string', + renderable2: [MyReactComponent(), 'a string'], + instanceOf: new Thing, + component: MyReactComponent() + })); + }; + + React.renderComponent(List({ + array: [11, 12, 13, 14, 15, 16, 17, 18, 19, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], + bool: false, + number: Math.random(), + string: 'banana banana banana', + func: function() { return 'this is a function'; }, + renderable: 'renderable string', + instanceOf: new Thing + }, items), _rootNode); +}; +exports.teardown = function(){ + React.unmountComponentAtNode(_rootNode); +}; diff --git a/src/vendor/core/warning.js b/src/vendor/core/warning.js index 2b5b6524124..bb0be956ce3 100644 --- a/src/vendor/core/warning.js +++ b/src/vendor/core/warning.js @@ -26,16 +26,14 @@ function warning(condition, format, ...args) { if (format === undefined) { throw new Error( - '`warning(condition, format, ...args)` requires an error message argument' + '`warning(condition, format, ...args)` requires a warning ' + + 'message argument' ); } if (!condition) { var argIndex = 0; - console.warn( - 'Warning: ' + - format.replace(/%s/g, () => args[argIndex++]) - ); + console.warn('Warning: ' + format.replace(/%s/g, () => args[argIndex++])); } }