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

Upgrade to react-redux-7, fix store instance missing issue #59 #60

Merged
merged 2 commits into from
Oct 30, 2019

Conversation

Firenze11
Copy link
Contributor

Changes

In v6 react-redux migrated from legacy react context to createContext. And in order for connect to access the store object passed from Provider, connect and Provider must come from a single react-redux dependency. Previously manifold depends on react-redux, and the embedded app also depends on react-redux, which results in connect and Provider coming from different places and caused the error in #59

  1. Upgrade react-redux dependency to 7
  2. react-redux in example and demo app both resolve to root level dependency
  3. Moved dependency for react-redux in manifold package to peerDependency

Test Plan

Both example and demo app can render; other app that are using Manifold won't crash (after the change is published)

Checklist

  • I have made this PR atomic.
  • I have provided enough context for others to review.
  • [ ] I have added tests to cover my changes (for features and bug fixes).
  • [ ] I have updated the documentation and changelogs accordingly.
  • All new and existing tests passed.

@Firenze11
Copy link
Contributor Author

There's a similar issue in kepler keplergl/kepler.gl#351 which needs to be fixed separately.

Copy link
Contributor

@javidhsueh javidhsueh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Firenze11 Firenze11 merged commit 5e5ba36 into master Oct 30, 2019
@Firenze11 Firenze11 deleted the react-redux-7 branch October 30, 2019 23:49
@kenns29 kenns29 mentioned this pull request Nov 25, 2019
2 tasks
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

3 participants