-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add support for react-redux 6 and onwards #10
Conversation
BREAKING CHANGE: `@uirouter/redux` now only supports `react-redux` 6.x onwards, with the new React Context api. Internally the library uses `ReactReduxContext` for consuming the redux store, which means it currently does not support custom context for Redux. Support for this might be added in the future.
New component implementation requires at least react@16 and the new enzyme adapter for it. Updated the tests to use the new Redux Context api, as well as rely on public uirouter APIs instead of component internals to be more reliable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also encountered a problem with using the latest react-redux and this seems to fix the issue. The changes look good to me 👍
const reduxPlugin = useRef(createReduxPlugin(store)); | ||
|
||
// let's initialise the plugins and set up the router | ||
if (init.current !== true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are moving to Hooks
anyway, maybe you should convert this to useEffect
to be compliant with the React docs.
Docs of useEffect
suggest using it for side effects etc. So it would be a good use-case for this code.
It would look like below, with empty array as second parameter to ensure that it fires only once.
useEffect(setupRouter, [])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think that is possible as the router would be initialized only after the initial render. As for me, the router should be set up synchronously, so the current implementation looks reasonable
@elboman Tried to build it locally.
You have to update your webpack config 😉 |
@elboman Hey, any update on this? |
@elboman Would be nice to use it without forking the package 😉 |
@elboman I can provide updated webpack config if you are interested. |
As per title, new React Redux uses the "new" context API, while this library was still using the old class-based (and now deprecated) API.
ConnectedUIRouter
to function component using hooks