Skip to content

Fixes #525 (cursor bug) by moving notify from setState callback to componentDidUpdate. #557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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