-
Notifications
You must be signed in to change notification settings - Fork 375
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
[wallet] upgrade react-redux #812
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is so we can use the new hooks API Also removed duplicated redux / react-redux versions
- Use bindActionsCreator instead of the shorthand syntax for some mapDispatchToProps that were using redux-thunk actions to workaround a new typescript error with the upgraded react-redux definitions - Migrated some tests to react-native-testing-library as they were raising 'TypeError: ShallowWrapper::dive() can only be called on components'. This is because the enzyme version we're using is not compatible with the new react context API used by the new react-redux
…or down to the component
jeanregisser
changed the title
[wallet] upgrade react redux
[wallet] upgrade react-redux
Sep 3, 2019
jmrossy
approved these changes
Sep 3, 2019
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.
LGTM!
|
||
useEffect( | ||
() => { | ||
// Generic error banner | ||
if (asyncResult.error) { | ||
Logger.error('CalculateFee', 'Error calculating fee', asyncResult.error) | ||
showErrorFunction(ErrorMessages.CALCULATE_FEE_FAILED) | ||
dispatch(showError(ErrorMessages.CALCULATE_FEE_FAILED)) |
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.
nice!
cmcewen
approved these changes
Sep 3, 2019
m-chrzan
pushed a commit
that referenced
this pull request
Sep 4, 2019
* Upgrade react-redux to latest stable version This is so we can use the new hooks API Also removed duplicated redux / react-redux versions * Upgrade failing tests with the react-redux upgrade - Use bindActionsCreator instead of the shorthand syntax for some mapDispatchToProps that were using redux-thunk actions to workaround a new typescript error with the upgraded react-redux definitions - Migrated some tests to react-native-testing-library as they were raising 'TypeError: ShallowWrapper::dive() can only be called on components'. This is because the enzyme version we're using is not compatible with the new react context API used by the new react-redux * Ignore a new typescript error * Simplify CalculateFee by using useDispatch instead of passing showError down to the component
aaronmgdr
added a commit
that referenced
this pull request
Sep 5, 2019
* master: Export geth metrics from StatefulSet tx-nodes and validators (#827) Adjust governance end-to-end gold total supply block number (#853) [Blockchain API] Add currency conversion endpoint to blockchain api (#855) [Wallet] Fix Payment Requests and DRY up confirmation card code (#831) Use HTTPS instead of SSH for ethereumjs-vm (#850) Kill end-to-end celo-blockchain instances with SIGINT (#849) [CLI]Minor code improvement in CLI, remove ts-ignore-next-line (#796) [wallet] Always allow users to view backup key (#825) [contractkit] Complete Exchange Wrapper (#801) [Wallet] Fix web3 syncing sagas (#809) Aaronmgdr/social links connect (#792) Deploy integration (#806) [wallet] upgrade react-redux (#812)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Upgrade
react-redux
so we can use the new hooks API which is nicer to use with functional component than wrapping the component with connect.I’m working on the new wallet home (#759) and since I’m writing new functional components I felt the need for this.
Tested
Upgraded and ran tests. Also did a quick test run of the app.
Other changes
redux
(patch version)redux
andreact-redux
versions (yarn.lock)hoist-non-react-statics
to fix a new error about propTypes@types/hoist-non-react-statics
toreact-component
. They are only used there.react-native-testing-library
(our enzyme version is incompatible with the new react context API used by the upgraded react-redux)CalculateFee
a little by using the newuseDispatch
hookRelated issues
Backwards compatibility
Yes.