Skip to content
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

Support react-redux@7 #303

Closed
wants to merge 1 commit into from
Closed

Conversation

cmfcmf
Copy link
Contributor

@cmfcmf cmfcmf commented May 4, 2019

This adds support for react-redux@7 and will close #287.

Two tests are failing right now, namely the ones that use a custom context. This happens due to a behavioral change (I'm unsure if its a bug) in react-redux, which I reported in reduxjs/react-redux#1278

it('supports custom context', () => {
const context = React.createContext(null)
mount(
<Provider store={store} context={context}>
<ConnectedRouter {...props} context={context}>
<Route path="/" render={() => <div>Home</div>} />
</ConnectedRouter>
</Provider>
)
expect(onLocationChangedSpy.mock.calls).toHaveLength(1)
history.push('/new-location')
history.push('/new-location-2')
expect(onLocationChangedSpy.mock.calls).toHaveLength(3)
})

it('supports custom context', () => {
const context = React.createContext(null)
mount(
<Provider store={store} context={context}>
<ConnectedRouter {...props} context={context}>
<Route path="/" render={() => <div>Home</div>} />
</ConnectedRouter>
</Provider>
)
expect(onLocationChangedSpy.mock.calls).toHaveLength(1)
history.push('/new-location')
history.push('/new-location-2')
expect(onLocationChangedSpy.mock.calls).toHaveLength(3)
})

@thisissami
Copy link

thisissami commented May 13, 2019

Looks like the react-redux team has merged in the proposed changes that @cmfcmf submitted in a PR to the react-redux project. That should theoretically have the 2 tests here now passing (with the most up-to-date react-redux on their master branch). reduxjs/react-redux#1278

@cmfcmf
Copy link
Contributor Author

cmfcmf commented May 15, 2019

I'm going to update this PR and mark it as ready once a new version of react-redux is released. The peer depedency would then be ^6.0.0 || ^7.0.4 or ^6.0.0 || ^7.1.0, depending on the next release.

@mrfelton
Copy link

Looks like new version of react-redux was released a couple of days ago. Would be great to get this in.

@cmfcmf
Copy link
Contributor Author

cmfcmf commented May 22, 2019

Looks like new version of react-redux was released a couple of days ago. Would be great to get this in.

They only released alpha versions. I'd like to wait for a non-alpha release.

@genexu
Copy link

genexu commented May 27, 2019

I just meet this issue today, I use lerna to controll all packages deps, and create new react app, And face connectedRouter context missing, maybe we should add Notice on Readme before compatible version has been release.

@ee0pdt
Copy link

ee0pdt commented May 28, 2019

@cmfcmf Thanks for all your time making this PR. Hopefully react-redux will not be long in being updated

@quarties
Copy link
Contributor

Hi folks! There is a new version of react-redux available (v7.1.0) :-) cc @cmfcmf

@ee0pdt
Copy link

ee0pdt commented Jun 12, 2019

Yes, @cmfcmf the 7.1 release is live so I think you can make your PR non-draft now

@quarties
Copy link
Contributor

As @cmfcmf seems to be not available right now, I decided to make my own PR: #321

@cmfcmf
Copy link
Contributor Author

cmfcmf commented Jun 16, 2019

As @cmfcmf seems to be not available right now, I decided to make my own PR: #321

Thank you @quarties and sorry for not responding quicker :) Closing this PR then.

@cmfcmf cmfcmf closed this Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support react-redux v7
6 participants