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

DRAFT (Do not merge) Attempt redux7 update #5594

Merged
merged 1 commit into from Sep 27, 2021

Conversation

vivek1729
Copy link
Contributor

To think through some of the challenges in upgrading react-redux to v7, I decided to scaffold a PR and see how far I get in this effort. Detailed info on the release notes and breaking changes for v7 can be found here. Since, v7 also requires a react update to atleast 16.8, I bumped up react/react-dom to latest 16.

Associated package updates:
It's awesome that react* dependencies are listed as peer dependencies in individual packages which makes upgrades and management simpler. Following packages had react-redux 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.

Learnings

I wanted to focus my spike on getting the desktop app to load. I ran into the following Type annotation error that required a minor code change:
image

For some reason, I also consistently ran into the following error where the desktop app couldn't resolve these pcakges:
image
These 2 packages aren't listed as dependencies for the desktop app. As a temporary mitigation, I worked around it by just installing them at the top level.

With these changes I was able to get the desktop app to load and execute cells.

Future work

  • Bump react versions in the peer dependencies sections of packages to account for the react-redux update.
  • Validation: I also created a draft PR for a similar upgrade in the outputs repo and it'd be important to validate how these packages interact after upgrades. We'd also want to run through automated tests and think about integration scenarios that might break.

I hope this serves as a good starting point for the effort.

@vercel
Copy link

vercel bot commented Aug 26, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nteract/site/F8wqCgyF2XadCZfg4sFZrgw43bgy
✅ Preview: https://site-git-fork-vivek1729-vipradha-redux7-nteract.vercel.app

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.

None yet

2 participants