Start using modern Redux #1710
Merged
Start using modern Redux #1710
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
I assume this is in place for easy uncommenting to enable the logging middleware. In this case, leave the import commented out as well so that ESLint/TypeScript rule exceptions aren't necessary.
initialState (now commonly named preloadedState in modern Redux docs) was unused. This means there is no need for the intermediate configureStore function (whose name was misleading since that is the name of a newer Redux Toolkit function).
Follow the guide¹. In short, configureStore automatically handles most of the store setup, including adding the thunk middleware and setting up the Redux DevTools Extension connection (now conditional on NODE_ENV). ¹ https://redux.js.org/usage/migrating-to-modern-redux#store-setup-with-configurestore
From the migration guide: > [configureStore] automatically added more middleware to check for > common mistakes like accidentally mutating the state It turns out that these "common mistakes" are currently used in practice, and only noticeable now with console errors after the Redux migration. Disable the new checks for now.
nextstrain-bot
temporarily deployed
to
auspice-victorlin-use-m-rcnf1e
15 tasks
This was disabled in 80835fc, however there are issues with a few type declaration files in @reduxjs/toolkit that I don't want to bother tracking down if an earlier version can be used. It was originally disabled with an idea that this codebase might have some of its own declaration files, however I don't see that being necessary ever, or at least in the near future.
There aren't and shouldn't be any other files under src/store. This new naming is what's used in latest Redux documentation.
Export types for the store, inferred from the store setup process. These are unused for now, but will be useful for future TypeScript adoption of anything that accesses the store.
Previously, the controls state could not be implicitly typed. This starts things off for further adoption.
This uses the newly defined types in the controls reducer. The migration doc recommends replacing connect with the hooks API¹. ¹ https://redux.js.org/usage/migrating-to-modern-redux#migrating-connect-to-hooks
Recommended in migration guide¹. ¹ https://redux.js.org/usage/migrating-to-modern-redux#migrating-typescript-for-components
victorlin
force-pushed
the
victorlin/use-modern-redux
branch
from
c33630d to
3e77a0a
Compare
nextstrain-bot
temporarily deployed
to
auspice-victorlin-use-m-rcnf1e
jameshadfield
approved these changes
Oct 12, 2023
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 of proposed changes
Mostly following the migration doc. See commit messages.
Related issue(s)
#1711
Checklist
(to be done by a Nextstrain team member) Create preview PRs on downstream repositories.Not necessary - no functional changes.