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

Add support for react-redux 6 and onwards #10

Merged
merged 10 commits into from
Jan 22, 2020

Conversation

elboman
Copy link
Member

@elboman elboman commented Apr 19, 2019

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.

  • Update to use the new one
  • Update a bunch of dependencies
  • Convert ConnectedUIRouter to function component using hooks
  • Update enzyme adapter for React@16
  • Update tests

elboman added 4 commits April 19, 2019 17:29
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.
elboman added 2 commits April 19, 2019 19:01
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.
Copy link

@Gelio Gelio left a 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) {

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, [])

https://reactjs.org/docs/hooks-reference.html#useeffect

Copy link

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

@DamianOsipiuk
Copy link

@elboman Tried to build it locally.

Error: webpack.optimize.UglifyJsPlugin has been removed, please use config.optimization.minimize instead.

You have to update your webpack config 😉

@DamianOsipiuk
Copy link

@elboman Hey, any update on this?

@DamianOsipiuk
Copy link

@elboman Would be nice to use it without forking the package 😉

@DamianOsipiuk
Copy link

@elboman I can provide updated webpack config if you are interested.

@christopherthielen christopherthielen merged commit cd9e393 into master Jan 22, 2020
@christopherthielen christopherthielen deleted the support-react-redux-6 branch January 22, 2020 20:14
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.

4 participants