Skip to content

Update Redux example to use "Modern Redux" techniques? #4

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

Closed
markerikson opened this issue Jun 11, 2021 · 2 comments
Closed

Update Redux example to use "Modern Redux" techniques? #4

markerikson opened this issue Jun 11, 2021 · 2 comments

Comments

@markerikson
Copy link

Hi, I'm a Redux maintainer. I was just pointed at this repo asking about some of its performance characteristics, and I note that it's actually using some very outdated Redux practices.

Some observations:

  • It's using hand-written Redux logic in reducers, and separate action constants. There's actually no separate action creators - those are all being defined inside of mapDispatch functions. Instead, it should be using our official Redux Toolkit package, which simplifies reducer logic, generates action creators for you, and eliminates the need to write action constants separately: https://redux-toolkit.js.org
  • It's using connected-react-router. We would generally recommend not using a Redux-based router at this point. It's not necessary and doesn't provide enough benefit.
  • The components are class components using connect, not function components using the React-Redux hooks API
  • The codebase is written in plain JS, not TypeScript
  • It doesn't appear to be following some of the other recommendations in the Redux Style guide ( https://redux.js.org/style-guide/style-guide ), such as using a "feature folder" structure for the logic.
  • There's a number of additional dependencies that appear to be unnecessary or at least could be replaced with lighter equivalents. For example, marked is adding 15K min+gz to the bundle, whereas other RealWorld examples are using snarked or nano-markdown which are only about 1K min+gz. There's also a bunch of other deps, like _.isEqualWith, object-inspect, etc. Some are directly used, others are transitive dependencies. The bundle analysis looks like this:

image

I'd love to see this example app updated with two different variations:

  • One that uses our official Redux Toolkit package for the logic, React-Redux hooks, and function components, with TypeScript, and uses RTK's createAsyncThunk to help define the data fetching logic. Rather than superagent, perhaps use something like redaxios if you want a wrapper around fetch.
  • A second one that's also RTK + hooks + TS, but uses our new RTK Query addon to Redux Toolkit to do all the data fetching: https://redux-toolkit.js.org/rtk-query/overview
@khaledosman
Copy link
Owner

khaledosman commented Jun 13, 2021

Hi @markerikson , Thanks for the suggestions, so far the repo has only been updated to use the newest versions of the dependencies, but I never really bothered to refactor the actual codebase into the newer way of doing things too much, so while the project does potentially support hooks, typescript and such, I tried to be as minimally invasive as possible to the original codebase, although I do agree now would be a good time to update it.

If anyone would like to take any of the points feel free to, looks like @leosuncin already started working on a PR

@khaledosman
Copy link
Owner

Most of the changes should now be handled in #11

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

No branches or pull requests

2 participants