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

Legacy Context removed on react-redux@^6 #101

Closed
asermax opened this issue Dec 2, 2018 · 7 comments · Fixed by #107
Closed

Legacy Context removed on react-redux@^6 #101

asermax opened this issue Dec 2, 2018 · 7 comments · Fixed by #107

Comments

@asermax
Copy link

asermax commented Dec 2, 2018

Hi there!
Just wanted to give a heads up regarding the usage of the legacy context to access redux's store on Link and NavLink.
redux-react is planning on removing the legacy context to move to the new context API provided by react (https://github.com/reduxjs/react-redux/releases/tag/v6.0.0-beta.1).

In any case, thanks for the great work so far!

@raRaRa
Copy link

raRaRa commented Dec 5, 2018

Yup, this is definitely an issue. I just upgraded to react-redux 6 and I get this error:

TypeError: Cannot read property 'getState' of undefined
/redux-first-router-link/dist/Link.js:94
> 94 | var location = (0, _reduxFirstRouter.selectLocationState)(store.getState());

@ScriptedAlchemy
Copy link
Collaborator

Anyone feel up to creating a checker to maintain compatibility? Or can you downgrade to a lower version will accept PRs or documentation regarding the issue

@raRaRa
Copy link

raRaRa commented Dec 5, 2018

Here are some good links on what changed and how to fix it:
https://blog.isquaredsoftware.com/2018/11/react-redux-history-implementation/

connected-react-router are working on a fix for their lib.
supasate/connected-react-router#191

@cdoublev
Copy link
Contributor

@ScriptedAlchemy, I managed to get this package work with react-redux@v6.0.0 and older versions without some special checker. I simply removed all context usages and switched to using props mapped from state for all props required by Link/NavLink (location and its properties routesMap, pathname). Is there something wrong with this solution?

@ScriptedAlchemy
Copy link
Collaborator

@cdoublev, open a PR with whatever you have done. paste in an example and it will help me look at abstracting it.

@raRaRa - thanks for pointing me in the direction, I'm in the middle of watching and adding a major fix to react static. It is moving out of beta very soon to the next major version. I've been tied up with that project for this week.

I should be able to invest in an implementation at end of this week.

As always - the more debug info any user who reads this issue, the faster ill fix it with less risk :P

Really appreciate you fellas taking the time to look over the problem and provide some ideas. This will be my next priority as soon as react-static is stabilized.

I apologize that i am bottlenecking us at the moment. Its 12am and it has been a few days of late nights on RS.

I hope to be on standby tomorrow, allowing me to devote time to RFR/RFRL

@ScriptedAlchemy
Copy link
Collaborator

🎉 This issue has been resolved in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ScriptedAlchemy
Copy link
Collaborator

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants