Skip to content

Commit

Permalink
Duck-type subscription in prop checks (reduxjs#628)
Browse files Browse the repository at this point in the history
* 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)

* Centralize PropType shapes.
  • Loading branch information
jquense authored and timdorr committed Feb 23, 2017
1 parent 9f38554 commit 9a4cda1
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 19 deletions.
5 changes: 2 additions & 3 deletions src/components/Provider.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -51,6 +50,6 @@ Provider.propTypes = {
}
Provider.childContextTypes = {
store: storeShape.isRequired,
storeSubscription: PropTypes.instanceOf(Subscription)
storeSubscription: subscriptionShape
}
Provider.displayName = 'Provider'
17 changes: 8 additions & 9 deletions src/components/connectAdvanced.js
Original file line number Diff line number Diff line change
@@ -1,14 +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() {}

function makeSelectorStateful(sourceSelector, store) {
// wrap the selector in an object that tracks its results between runs.
const selector = {
Expand Down Expand Up @@ -81,10 +80,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) {
Expand Down Expand Up @@ -194,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)
Expand All @@ -218,7 +217,7 @@ export default function connectAdvanced(
this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate
this.setState(dummyState)
}
}
}

notifyNestedSubsOnComponentDidUpdate() {
// `componentDidUpdate` is conditionally implemented when `onStateChange` determines it
Expand Down
14 changes: 14 additions & 0 deletions src/utils/PropTypes.js
Original file line number Diff line number Diff line change
@@ -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
})
7 changes: 0 additions & 7 deletions src/utils/storeShape.js

This file was deleted.

0 comments on commit 9a4cda1

Please sign in to comment.