From cc8c1978dd51b975ebe719716b24df99069acc4b Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Wed, 22 Feb 2017 09:07:34 -0500 Subject: [PATCH 1/2] Duck-type subscription in prop checks Use `PropTypes.shape` instead of instanceOf checks which don't work across multiple instances of react-redux. Granted it's not a problem that _should_ occur since they should be deduped, but dependency management is hard, and sometimes that's not feasible due to symlinks and the like. This is (I think) is more idiomatic JS any way :) thanks! (sorry if something obvious is off i wrote this in the GH editor) --- src/components/connectAdvanced.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 7f403f47e..6b5d51129 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -9,6 +9,13 @@ let hotReloadingVersion = 0 const dummyState = {} function noop() {} +const subscriptionShape = PropTypes.shape({ + trySubscribe: PropTypes.func.isRequired, + tryUnsubscribe: PropTypes.func.isRequired, + notifyNestedSubs: PropTypes.func.isRequired, + isSubscribed: PropTypes.func.isRequired, +}) + function makeSelectorStateful(sourceSelector, store) { // wrap the selector in an object that tracks its results between runs. const selector = { @@ -81,10 +88,10 @@ export default function connectAdvanced( const contextTypes = { [storeKey]: storeShape, - [subscriptionKey]: PropTypes.instanceOf(Subscription), + [subscriptionKey]: subscriptionShape, } const childContextTypes = { - [subscriptionKey]: PropTypes.instanceOf(Subscription) + [subscriptionKey]: subscriptionShape, } return function wrapWithConnect(WrappedComponent) { From c455ac3b861b7b6b2742e0719efa836124a3f53d Mon Sep 17 00:00:00 2001 From: Tim Dorr Date: Wed, 22 Feb 2017 13:00:37 -0500 Subject: [PATCH 2/2] Centralize PropType shapes. --- src/components/Provider.js | 5 ++--- src/components/connectAdvanced.js | 20 ++++++-------------- src/utils/PropTypes.js | 14 ++++++++++++++ src/utils/storeShape.js | 7 ------- 4 files changed, 22 insertions(+), 24 deletions(-) create mode 100644 src/utils/PropTypes.js delete mode 100644 src/utils/storeShape.js diff --git a/src/components/Provider.js b/src/components/Provider.js index 308f4da71..db3918712 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -1,6 +1,5 @@ import { Component, PropTypes, Children } from 'react' -import Subscription from '../utils/Subscription' -import storeShape from '../utils/storeShape' +import { storeShape, subscriptionShape } from '../utils/PropTypes' import warning from '../utils/warning' let didWarnAboutReceivingStore = false @@ -51,6 +50,6 @@ Provider.propTypes = { } Provider.childContextTypes = { store: storeShape.isRequired, - storeSubscription: PropTypes.instanceOf(Subscription) + storeSubscription: subscriptionShape } Provider.displayName = 'Provider' diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 6b5d51129..54ffa477c 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,21 +1,13 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' -import { Component, PropTypes, createElement } from 'react' +import { Component, createElement } from 'react' import Subscription from '../utils/Subscription' -import storeShape from '../utils/storeShape' +import { storeShape, subscriptionShape } from '../utils/PropTypes' let hotReloadingVersion = 0 const dummyState = {} function noop() {} - -const subscriptionShape = PropTypes.shape({ - trySubscribe: PropTypes.func.isRequired, - tryUnsubscribe: PropTypes.func.isRequired, - notifyNestedSubs: PropTypes.func.isRequired, - isSubscribed: PropTypes.func.isRequired, -}) - function makeSelectorStateful(sourceSelector, store) { // wrap the selector in an object that tracks its results between runs. const selector = { @@ -201,16 +193,16 @@ export default function connectAdvanced( initSubscription() { if (!shouldHandleStateChanges) return - + // parentSub's source should match where store came from: props vs. context. A component // connected to the store via props shouldn't use subscription from context, or vice versa. const parentSub = (this.propsMode ? this.props : this.context)[subscriptionKey] this.subscription = new Subscription(this.store, parentSub, this.onStateChange.bind(this)) - + // `notifyNestedSubs` is duplicated to handle the case where the component is unmounted in // the middle of the notification loop, where `this.subscription` will then be null. An // extra null check every change can be avoided by copying the method onto `this` and then - // replacing it with a no-op on unmount. This can probably be avoided if Subscription's + // replacing it with a no-op on unmount. This can probably be avoided if Subscription's // listeners logic is changed to not call listeners that have been unsubscribed in the // middle of the notification loop. this.notifyNestedSubs = this.subscription.notifyNestedSubs.bind(this.subscription) @@ -225,7 +217,7 @@ export default function connectAdvanced( this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate this.setState(dummyState) } - } + } notifyNestedSubsOnComponentDidUpdate() { // `componentDidUpdate` is conditionally implemented when `onStateChange` determines it diff --git a/src/utils/PropTypes.js b/src/utils/PropTypes.js new file mode 100644 index 000000000..ae714d6b8 --- /dev/null +++ b/src/utils/PropTypes.js @@ -0,0 +1,14 @@ +import { PropTypes } from 'react' + +export const subscriptionShape = PropTypes.shape({ + trySubscribe: PropTypes.func.isRequired, + tryUnsubscribe: PropTypes.func.isRequired, + notifyNestedSubs: PropTypes.func.isRequired, + isSubscribed: PropTypes.func.isRequired, +}) + +export const storeShape = PropTypes.shape({ + subscribe: PropTypes.func.isRequired, + dispatch: PropTypes.func.isRequired, + getState: PropTypes.func.isRequired +}) diff --git a/src/utils/storeShape.js b/src/utils/storeShape.js deleted file mode 100644 index 16b1b141a..000000000 --- a/src/utils/storeShape.js +++ /dev/null @@ -1,7 +0,0 @@ -import { PropTypes } from 'react' - -export default PropTypes.shape({ - subscribe: PropTypes.func.isRequired, - dispatch: PropTypes.func.isRequired, - getState: PropTypes.func.isRequired -})