-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Switch to Redux Toolkit and ducks pattern #2812
base: dev
Are you sure you want to change the base?
Conversation
- Merged App and HomePage slices - RepoListItem becomes a component since we determine ownership of repo in the saga - Delete all useless actions/constants/reducer files - Update all broken tests - Add ESLint exception for reducer files
- Add tests to improve coverage
* Update generators for switch to Redux Toolkit * Remove useless comments from tests * Turn multiline comment into many single line comments ;)
# Conflicts: # app/containers/App/index.js # app/containers/LanguageProvider/reducer.js # app/containers/LocaleToggle/index.js # internals/templates/containers/LanguageProvider/reducer.js # package.json
# Conflicts: # app/containers/App/selectors.js # package.json
Here I don't understand why there is a |
}, | ||
reposLoaded(state, action) { | ||
const { repos } = action.payload; | ||
state.repositories = repos; |
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.
Mutating state directly is an antipattern. You should return { ...state, repos }
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.
RTK uses immer
inside. So you can use it this way
@all-contributors please add Tommoore96 for code |
I've put up a pull request to add @Tommoore96! 🎉 |
Edit: Sorry I opening this back as a result of not merging the #2935 if that means anything;) |
@@ -60,6 +60,7 @@ | |||
}, | |||
"dependencies": { | |||
"@hot-loader/react-dom": "16.11.0", | |||
"@reduxjs/toolkit": "1.0.4", |
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.
@reduxjs/toolkit
includes redux-thunk by default. Not sure if webpack shakes-out this dependency if unused. IMO, toolkit is too opinionated (and hiding immer
is too much "magic" for my taste). The ducks pattern can be implemented just as easily, and with a similar reduction in boilerplate, using a single-purpose library like redux-actions
. Using redux-actions
also allows easier transition to TypeScript, if desired (see typesafe-actions
)
Is there a chance when this will be release? |
React Boilerplate
This is a draft PR which aggregates all the changes as we work towards switching to Redux Toolkit and the ducks pattern.
Related issue:
Changes so far
To Do
reselect
and/orimmer
as dependencies.reselect
recommendations in this comment.