Skip to content

Commit

Permalink
Fixes reduxjs#525 (cursor bug) by moving notify from setState callbac…
Browse files Browse the repository at this point in the history
…k to componentDidUpdate. (reduxjs#557)

* Fixes reduxjs#525 (cursor bug) by moving notify from setState callback to cDU.

This also eliminates react15CompatibilityMode stopgap setting. See: reduxjs#525 (comment) for discussion.

* Optimizes perf of previous commit.
  • Loading branch information
jimbolla authored and timdorr committed Dec 5, 2016
1 parent 72e9517 commit 6bbcb53
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 31 deletions.
33 changes: 7 additions & 26 deletions src/components/connectAdvanced.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import { Component, PropTypes, createElement } from 'react'
import Subscription from '../utils/Subscription'
import storeShape from '../utils/storeShape'


let defaultReact15CompatibilityMode = true
let hotReloadingVersion = 0
export default function connectAdvanced(
/*
Expand Down Expand Up @@ -37,9 +35,6 @@ export default function connectAdvanced(
// probably overridden by wrapper functions such as connect()
methodName = 'connectAdvanced',

// temporary compatibility setting for React 15. See Connect constructor for details
react15CompatibilityMode = undefined,

// if defined, the name of the property passed to the wrapped element indicating the number of
// calls to render. useful for watching in react devtools for unnecessary re-renders.
renderCountProp = undefined,
Expand All @@ -63,7 +58,6 @@ export default function connectAdvanced(
const contextTypes = {
[storeKey]: storeShape,
[subscriptionKey]: PropTypes.instanceOf(Subscription),
react15CompatibilityMode: PropTypes.bool,
}
const childContextTypes = {
[subscriptionKey]: PropTypes.instanceOf(Subscription)
Expand Down Expand Up @@ -103,18 +97,7 @@ export default function connectAdvanced(
this.state = {}
this.renderCount = 0
this.store = this.props[storeKey] || this.context[storeKey]

// react15CompatibilityMode controls whether the subscription system is used. This is for
// https://github.com/reactjs/react-redux/issues/525 and should be removed completely when
// react-redux's dependency on react is bumped to mimimum v16, which is expected to include
// PR https://github.com/facebook/react/pull/8204 which fixes the issue.
const compatMode = [
react15CompatibilityMode,
props.react15CompatibilityMode,
context.react15CompatibilityMode,
defaultReact15CompatibilityMode
].find(cm => cm !== undefined && cm !== null)
this.parentSub = compatMode ? null : props[subscriptionKey] || context[subscriptionKey]
this.parentSub = props[subscriptionKey] || context[subscriptionKey]

this.setWrappedInstance = this.setWrappedInstance.bind(this)

Expand Down Expand Up @@ -209,7 +192,6 @@ export default function connectAdvanced(
initSubscription() {
if (shouldHandleStateChanges) {
const subscription = this.subscription = new Subscription(this.store, this.parentSub)
const notifyNestedSubs = subscription.notifyNestedSubs.bind(subscription)
const dummyState = {}

subscription.onStateChange = function onStateChange() {
Expand All @@ -218,7 +200,12 @@ export default function connectAdvanced(
if (!this.selector.shouldComponentUpdate) {
subscription.notifyNestedSubs()
} else {
this.setState(dummyState, notifyNestedSubs)
this.componentDidUpdate = function componentDidUpdate() {
this.componentDidUpdate = undefined
subscription.notifyNestedSubs()
}

this.setState(dummyState)
}
}.bind(this)
}
Expand Down Expand Up @@ -275,9 +262,3 @@ export default function connectAdvanced(
return hoistStatics(Connect, WrappedComponent)
}
}

connectAdvanced.setDefaultReact15CompatibilityMode =
function setDefaultReact15CompatibilityMode(compat) {
defaultReact15CompatibilityMode = compat
}

4 changes: 1 addition & 3 deletions src/connect/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,4 @@ export function createConnect({
}
}

const connect = createConnect()
connect.setDefaultReact15CompatibilityMode = connectAdvanced.setDefaultReact15CompatibilityMode
export default connect
export default createConnect()
2 changes: 1 addition & 1 deletion test/components/Provider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('React', () => {
// The state from parent props should always be consistent with the current state
expect(state).toEqual(parentProps.parentState)
return {}
}, null, null, { react15CompatibilityMode: false })
})
class ChildContainer extends Component {
render() {
return <div />
Expand Down
2 changes: 1 addition & 1 deletion test/components/connect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ describe('React', () => {
// The state from parent props should always be consistent with the current state
expect(state).toEqual(parentProps.parentState)
return {}
}, null, null, { react15CompatibilityMode: false })
})
class ChildContainer extends Component {
render() {
return <Passthrough {...this.props}/>
Expand Down

0 comments on commit 6bbcb53

Please sign in to comment.