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

Migrating to react-redux v7 #5588

Closed
vivek1729 opened this issue Jul 27, 2021 · 7 comments · Fixed by #5601
Closed

Migrating to react-redux v7 #5588

vivek1729 opened this issue Jul 27, 2021 · 7 comments · Fixed by #5601

Comments

@vivek1729
Copy link
Contributor

Hello nteract,

I noticed that the mono repo relies on react-redux v6 which was released about 2 years ago. Redux 7 has been out for a bit and has seen good community adoption. Beyond stabilization, v7 also provides some performance improvements and new APIs. Most notably, they introduce a new batch api which could have a significant impact in reducing the number of state updates and render calls for React components.

I wanted to jumpstart a discussion to talk through the challenges and better understand if this is something on the radar of the community

Thanks!

@willingc
Copy link
Member

Hi @vivek1729. This seems like a reasonable thing to do. I'm not sure how much updates need to be done when going from 6 to 7.

@willingc
Copy link
Member

@vivek1729 and others, Perhaps let's start a checklist of things that would need to be updated to go from v6 to 7.

Breaking changes from v6 to 7 that impact nteract

  • Likely widgets

@vivek1729
Copy link
Contributor Author

Thanks for chiming in @willingc. The detailed release notes for v7 can be found here. This update would require bumping react as well as it's min peer dependency version was raised to 16.8.4 in v7 and nteract is on 16.3.2.

I scanned through this repo and it seems like the following packages have react-redux defined as a dependency in their package.json:

  • connected-components
  • myths
  • notebook-app-component
    Beyond these packages, it is also mentioned as a dependency for 2 top level applications - jupyter-extension and web.

For the outputs repo, I think it might only be widgets as you already pointed out.

@vivek1729
Copy link
Contributor Author

I also spent some time attempting this upgrade on the nteract mono repo (draft PR) and the outputs repo (draft PR). In both the PRs, I have tried to detail the issues and learnings. In both these PRs, I managed to get the package built successfully and the nteract desktop app to load as well.

However, looking at the changes detailed in the redux 7 roadmap, it'd be important to validate the integration of these different packages and think through these scenarios.

@vivek1729
Copy link
Contributor Author

vivek1729 commented Sep 1, 2021

Here are some of the added benefits to updating react-redux to v7 (from Barry's notes):

  • Perf improvments: It seems like the v6 released had a regression in performance as compared to v5. In the newer version, the authors have started relying on React.memo to cache rendered result for given props and avoid un-needed re-renders. (source)
  • Additional flexibility supporting react hooks for redux APIs: useSelector, useDispatch, useStore.

@captainsafia, @rgbkrk - here are some of the tasks I foresee for this update to happen if we decide to take a phased approach.

Phase 1: Update react-redux to v7 in the outputs repo

  • Bump react-redux to v7
  • Mark react-redux as a peer dependency for packages that depend on it
  • Update peer dependency version of react to 16.8.4 as this is the min version required for react-redux v7

Phase 2: Update react-redux to v7 in the mono repo

  • Mark react-redux as a peer dependency in relevant packages. This would involve an audit of all the packages in the mono repo. I noticed that there were a few packages like stateful-components that depend upon react-redux but don't claim it as a dependency. This would also involve updating packages where react-redux is marked as a regular dependency. I've identified some of these packages in my earlier comment.
  • Update react, react-dom and associated typings to align with a later version of React. Ensure that react* packages are also managed as peer dependencies wherever necessary and they are updated to reflect this bump.
  • Add nteract/jupyter-widgets and nteract/trasnform-model-debug packages as dependencies for the desktop app. This lead to a build error as documented in my draft PR.
  • Update nteract apps (desktop, jupyter extension, etc.) to incorporate output package updates with redux 7. This is to ensure that all the packages for these apps are on same redux version
  • Validate and test these changes at an integration level in the mono repo
  • Communicate about this upcoming breaking change to the community

Phase 3: Adopt newer APIs from v7 as necessary.

Clearly we'd also want to validate and test out these changes before the release (esp. in Phase 1 and Phase 2). You'd also want to think about how this should affects the package release versions (if this qualifies as a breaking change etc.)
This list might be a good starting point when you decide to tackle this effort.

@willingc
Copy link
Member

willingc commented Sep 14, 2021

Communicate breaking change

  • change log
  • blog post
  • jupyter discourse
  • Slack @nteract #general channel

@markerikson
Copy link

markerikson commented Sep 22, 2021

Yipes, I'm amazed anyone is still using v6 this long after v7 came out! Please do migrate to v7 ASAP - it's vastly better than v6 was. There should be no meaningful changes API-wise, just bump the package version and make sure you're on React 16.8+ (which you should be anyway at this point).

You should also know that we're currently working on React-Redux v8, which is aimed to come out alongside React 18 whenever that goes live (possibly early 2022?), and will have a hard dependency on React 18 the same way v7 depends on 16.8+:

reduxjs/react-redux#1740

Note that we do also now recommend the use of the React-Redux hooks API as the default. connect still works fine, and will continue to be supported even in the upcoming v8 version, but we've found that the hooks API is generally simpler and easier to work with.

Finally, note that we have an official Redux Toolkit package that is now the standard approach for writing Redux logic. If you aren't using yet, we strongly recommend switching to using RTK - it will simplify existing code, catch common mistakes, and includes APIs for a number of common Redux use cases. You can migrate incrementally - switch the store setup for RTK's configureStore, then migrate one reducer at a time:

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 a pull request may close this issue.

3 participants