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

Start using modern Redux #1710

Merged
merged 11 commits into from
Oct 12, 2023
Merged

Start using modern Redux #1710

merged 11 commits into from
Oct 12, 2023

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Oct 11, 2023

Description of proposed changes

Mostly following the migration doc. See commit messages.

Related issue(s)

#1711

Checklist

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.
@victorlin victorlin self-assigned this Oct 11, 2023
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-use-m-rcnf1e October 11, 2023 00:29 Inactive
@victorlin victorlin mentioned this pull request Oct 11, 2023
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
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-use-m-rcnf1e October 12, 2023 00:12 Inactive
@victorlin victorlin changed the title Use modern Redux Start using modern Redux Oct 12, 2023
@victorlin victorlin marked this pull request as ready for review October 12, 2023 00:17
src/store/index.js Outdated Show resolved Hide resolved
@jameshadfield jameshadfield mentioned this pull request Oct 12, 2023
8 tasks
@victorlin victorlin merged commit fd760cd into master Oct 12, 2023
20 checks passed
@victorlin victorlin deleted the victorlin/use-modern-redux branch October 12, 2023 22:24
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.

3 participants