From a134d45bf1f9e8dbe4fe21f575776ecbed75abb1 Mon Sep 17 00:00:00 2001 From: Keyan Zhang Date: Fri, 20 May 2016 16:53:38 -0700 Subject: [PATCH 1/2] remove prop types checking in ReactCompositeComponent --- .../types/__tests__/ReactPropTypes-test.js | 6 +- .../reconciler/ReactCompositeComponent.js | 58 ++++--------------- 2 files changed, 14 insertions(+), 50 deletions(-) diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index 1c01aeee8404a..cb778202c334c 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -849,9 +849,8 @@ describe('ReactPropTypes', function() { var instance = ; instance = ReactTestUtils.renderIntoDocument(instance); - expect(spy.argsForCall.length).toBe(2); // temp double validation + expect(spy.argsForCall.length).toBe(1); expect(spy.argsForCall[0][1]).toBe('num'); - expect(spy.argsForCall[0][2]).toBe('Component'); }); it('should have been called even if the prop is not present', function() { @@ -867,7 +866,8 @@ describe('ReactPropTypes', function() { var instance = ; instance = ReactTestUtils.renderIntoDocument(instance); - expect(spy.argsForCall.length).toBe(2); // temp double validation + expect(spy.argsForCall.length).toBe(1); + expect(spy.argsForCall[0][1]).toBe('num'); }); it('should have received the validator\'s return value', function() { diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 8fc797064a7c3..db88cc7d34bde 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -197,7 +197,7 @@ var ReactCompositeComponentMixin = { this._hostParent = hostParent; this._hostContainerInfo = hostContainerInfo; - var publicProps = this._processProps(this._currentElement.props); + var publicProps = this._currentElement.props; var publicContext = this._processContext(context); var Component = this._currentElement.type; @@ -614,7 +614,7 @@ var ReactCompositeComponentMixin = { if (__DEV__) { var Component = this._currentElement.type; if (Component.contextTypes) { - this._checkPropTypes( + this._checkContextTypes( Component.contextTypes, maskedContext, ReactPropTypeLocations.context @@ -647,7 +647,7 @@ var ReactCompositeComponentMixin = { this.getName() || 'ReactCompositeComponent' ); if (__DEV__) { - this._checkPropTypes( + this._checkContextTypes( Component.childContextTypes, childContext, ReactPropTypeLocations.childContext @@ -666,29 +666,6 @@ var ReactCompositeComponentMixin = { return currentContext; }, - /** - * Processes props by setting default values for unspecified props and - * asserting that the props are valid. Does not mutate its argument; returns - * a new props object with defaults merged in. - * - * @param {object} newProps - * @return {object} - * @private - */ - _processProps: function(newProps) { - if (__DEV__) { - var Component = this._currentElement.type; - if (Component.propTypes) { - this._checkPropTypes( - Component.propTypes, - newProps, - ReactPropTypeLocations.prop - ); - } - } - return newProps; - }, - /** * Assert that the props are valid * @@ -697,9 +674,7 @@ var ReactCompositeComponentMixin = { * @param {string} location e.g. "prop", "context", "child context" * @private */ - _checkPropTypes: function(propTypes, props, location) { - // TODO: Stop validating prop types here and only use the element - // validation. + _checkContextTypes: function(propTypes, props, location) { var componentName = this.getName(); for (var propName in propTypes) { if (propTypes.hasOwnProperty(propName)) { @@ -724,23 +699,12 @@ var ReactCompositeComponentMixin = { // top-level render calls, so I'm abstracting it away into // a function to minimize refactoring in the future var addendum = getDeclarationErrorAddendum(this); - - if (location === ReactPropTypeLocations.prop) { - // Preface gives us something to blacklist in warning module - warning( - false, - 'Failed Composite propType: %s%s', - error.message, - addendum - ); - } else { - warning( - false, - 'Failed Context Types: %s%s', - error.message, - addendum - ); - } + warning( + false, + 'Failed Context Types: %s%s', + error.message, + addendum + ); } } } @@ -830,7 +794,7 @@ var ReactCompositeComponentMixin = { // warning for DOM component props in this upgrade nextProps = nextParentElement.props; } else { - nextProps = this._processProps(nextParentElement.props); + nextProps = nextParentElement.props; willReceive = true; } From c78ac53ffb03b6dd351aa5b626e3ed34a2889933 Mon Sep 17 00:00:00 2001 From: Keyan Zhang Date: Mon, 23 May 2016 11:02:01 -0700 Subject: [PATCH 2/2] simplified .updateComponent() --- .../stack/reconciler/ReactCompositeComponent.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index db88cc7d34bde..b9ca072e556d4 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -667,9 +667,9 @@ var ReactCompositeComponentMixin = { }, /** - * Assert that the props are valid + * Assert that the context types are valid * - * @param {object} propTypes Map of prop name to a ReactPropType + * @param {object} propTypes Map of context field to a ReactPropType * @param {object} props * @param {string} location e.g. "prop", "context", "child context" * @private @@ -788,13 +788,10 @@ var ReactCompositeComponentMixin = { willReceive = true; } - // Distinguish between a props update versus a simple state update - if (prevParentElement === nextParentElement) { - // Skip checking prop types again -- we don't read inst.props to avoid - // warning for DOM component props in this upgrade - nextProps = nextParentElement.props; - } else { - nextProps = nextParentElement.props; + nextProps = nextParentElement.props; + + // Not a simple state update but a props update + if (prevParentElement !== nextParentElement) { willReceive = true; }