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

refactor: upgrade + optimise Redux #1981

Merged
merged 4 commits into from
May 17, 2023
Merged

refactor: upgrade + optimise Redux #1981

merged 4 commits into from
May 17, 2023

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented May 11, 2023

What it solves

Upgrades Redux to the latest version and optimises/improves type safety of middlewares.

How this PR fixes it

  • react-redux and @reduxjs/toolkit have been upgraded to the latest versions.
  • All midleware (apart from persistence-related ones) have been migrated to createListenerMiddleware

How to test it

No visible changes should be noticed:

  • Added Safes in the sidebar should remain up-to-date (balance).
  • Pending transaction signatures/executions should clear successfully.
  • Pending messages should clear when signed successfully.

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@iamacook iamacook requested a review from usame-algan May 11, 2023 14:41
@github-actions
Copy link

github-actions bot commented May 11, 2023

Branch preview

✅ Deploy successful!

https://upgrade_redux--walletweb.review-wallet-web.5afe.dev

@iamacook iamacook self-assigned this May 11, 2023
@github-actions
Copy link

github-actions bot commented May 11, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@iamacook iamacook marked this pull request as draft May 11, 2023 14:52
Copy link
Member

@usame-algan usame-algan left a comment

Choose a reason for hiding this comment

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

Looks good 👍
I like the better type support and reduced nesting with the new middleware.

@@ -90,7 +88,10 @@ export const _hydrationReducer: typeof rootReducer = (state, action) => {
const makeStore = (initialState?: Record<string, any>) => {
return configureStore({
reducer: _hydrationReducer,
middleware: (getDefaultMiddleware) => getDefaultMiddleware({ serializableCheck: false }).concat(middleware),
middleware: (getDefaultMiddleware) => {
listeners.forEach((listener) => listener(listenerMiddlewareInstance))
Copy link
Member

Choose a reason for hiding this comment

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

Is there a difference in registering the listeners in this function or on the top level? In the examples from the documentation they are registered on the top level right after the middleware is created.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so.

I moved it here as this is just before store creation/allowed for better testing. What are your thoughts?

@iamacook iamacook marked this pull request as ready for review May 16, 2023 13:19
@francovenica
Copy link
Contributor

LGTM

Checked that sidebar updates with the safes, new safes created, added safes, owned safes in new networks
Checked the lists queue history and message. Tx update just fine after creating/executing them
Tried with messages on and off chain, they update fine as well
Tried pagination, pagination with filter, tx updating after switching safes

@katspaugh
Copy link
Member

@iamacook please don't merge just yet, let's create an RC w/o it first.

@iamacook iamacook merged commit 80dd872 into dev May 17, 2023
@iamacook iamacook deleted the upgrade-redux branch May 17, 2023 08:07
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants