kentcdodds / ama

Ask me anything!
https://github.com/kentcdodds/ama/issues?q=is%3Aissue+is%3Aclosed
685 stars 75 forks source link

Moving from redux to hooks #663

Closed serene closed 5 years ago

serene commented 5 years ago

We are currently using redux to maintain state, but global state feels like a code smell to me.

Do you have any suggestions on how we can move towards using more local state using hooks?

kentcdodds commented 5 years ago

Hey @serene 👋

I need to update https://kentcdodds.com/blog/application-state-management with hooks in mind. It'll basically be a rewrite where I talk about how to accomplish a really awesome state management situation with only what React provides. So I'll leave this issue open to remind me to do that eventually.

As for migrating from redux to more local state, here's how I would approach that problem:

  1. Determine what parts of your global state are used in only one place. I think most redux apps will have many of these.
  2. Move those single-use elements of state from redux to local state where they're used.
  3. Determine what parts of your global state are used on one page of your app.
  4. Move those page-level elements of state from redux to a context provider that you render that page of your app in. Using useReducer and putting your existing reducer logic in there should be a pretty straightforward refactor (though you may have to do some fancy stuff to deal with middleware you depend on). Expose state and dispatch as your context value, then make a custom hook to consume that context and expose all your "action creators" to dispatch the existing actions you've got.

Once you're done with that, you'll be left with a handful of state elements that are truly global. You can repeat step 4 with those.

The biggest trick will be handling the middleware that you used to have. I have several ideas about this, and I need to play around with it a bit, but basically you could handle lots of that in the custom hook you write for the context consumer.

I hope that's helpful! I'll probably have more on this in the future. Also considering making a workshop or maybe a course for this.

DobrinGanev commented 5 years ago

@kentcdodds I did a similar to your 4th suggestion example of reusing the Redux reducers and the action creators. In the example, there is something like redux-thunk to handle dispatching of async actions https://github.com/DobrinGanev/react-hooks-examples

Yakimych commented 5 years ago

@serene Just curious - why does global state feel like a code smell?

serene commented 5 years ago

@Yakimych IMHO, components should know as little as possible - only the information that it needs to do its job. Otherwise, you can end up with feature envy and problems with implicit coupling.

These problems can be made more visible when your components have a large amount of information required for test setup. It's better to keep things modular, and only a few things in global state that really need to be there.

Yakimych commented 5 years ago

@serene

components should know as little as possible - only the information that it needs to do its job

Good point! My understanding is that it's exactly what you achieve with the whole mapStateToProps/mapDispatchToProps ceremony - pick out only the things the component needs to know about.

kentcdodds commented 5 years ago

Agreed @serene. Global state is something that most apps do actually need, but most apps I've seen have a very loose definition of what actually needs to be global and what could be local :)

DobrinGanev commented 5 years ago

@serene

components should know as little as possible - only the information that it needs to do its job

Good point! My understanding is that it's exactly what you achieve with the whole mapStateToProps/mapDispatchToProps ceremony - pick out only the things the component needs to know about.

That's correct, all child components of Redux Provider can connect to the store and access the state, is not like their props are polluted with stuff that they don't need. Now with the hooks, we can do that with the Context API and the useContext hook. The result is the same. What needs to be global and what local depends on the component. In many cases, we need slices of the state that was rendered in other components and global store is a clean solution. The local component state is volatile and it can be lost/destroyed on re-render. The global store is like a disk, which provides you with a single source of truth, without race conditions, transactional/replayable state.

jeremymarc commented 5 years ago

would you guys still recommend using redux with hook?

kentcdodds commented 5 years ago

If you're going to use redux, then I recommend using it the way it's documented.

jeremymarc commented 5 years ago

my question is more about redux vs context API for a new project (with React hook).

kentcdodds commented 5 years ago

Oh, yeah, I pretty much never recommend using Redux unless you have some sort of plugin system that you need third party developers to be able to hook into (like HyperTerm for example).

jeremymarc commented 5 years ago

what about the performance problems due the unnecessary re-rendering with the React Context API? I know we can use observedBits/calculateChangedBits but at the end, it seems we are developing our own kind of redux?

kentcdodds commented 5 years ago

unnecessary re-rendering

Why are you unnecessarily re-rendering? Is it because your context value is updating and your consumer doesn't need the update? You can solve this problem by splitting up your contexts into logical separate contexts. Only update the ones that need updating. Only subscribe to the ones that you need to know about :)

serene commented 5 years ago
export const Movies = () => {
  const [loading, setLoading] = useState(true)
  const [movies, setMovies] = useState([])

  useEffect(() => {
    async function loadMovies() {
      const movies = await getRequest({}, 'get_movies')
      setMovies(movies)
      setLoading(false)
    }

    loadMovies()
    return () => {
      setLoading(false)
    }
  }, [])

  return (
    loading ? (
      <Loader/>
    ) : (
      <ul>{movies.map((item, index) => (<li key={index}>{item}</li>))}</ul>
    )
  )
}

@kentcdodds Go easy on me here, I'm pretty new to react :)

In the example I've created above, doesn't this now make the data retrieval really tightly coupled to the view component and difficult to test?

kentcdodds commented 5 years ago

doesn't this now make the data retrieval really tightly coupled to the view component

Yes it does

and difficult to test

Depends on the tools you're using :)

So first of all I'd say that if you had a lot of complex logic in there then you could extract the hooks there to a custom hook and test that hook in isolation (with react-hooks-testing-library). If there was a bunch of regular JS type logic in there you could even extract that stuff to pure functions and test those in isolation.

But what you have there doesn't seem complex at all, so I would recommend testing that with react-testing-library as-is and I have several examples of how to accomplish that in a way that:

  1. Is pretty easy to test
  2. Is pretty easy to maintain
  3. Gives you a huge amount of confidence // <-- most important

This is the part where I tell you to get yourself a license to testingjavascript.com and it'll all become clear :)

I hope that's helpful!

BruceL33t commented 5 years ago

@kentcdodds any reason you put the "actionCreator" in a custom hook, instead of just putting it on the context directly? Having to fetch the data inside the custom hook, and dispatch an action with that payload from the hook, adds quite a lot of boilerplate code, compared to just doing it all in an attached function on the context.

  1. Move those page-level elements of state from redux to a context provider that you render that page of your app in. Using useReducer and putting your existing reducer logic in there should be a pretty straightforward refactor (though you may have to do some fancy stuff to deal with middleware you depend on). Expose state and dispatch as your context value, then make a custom hook to consume that context and expose all your "action creators" to dispatch the existing actions you've got.

I'm imagining the hook you mention, to look something like:

const useTodos = (id) => {
  const { dispatch } = useContext(Context);
  const [actionCreators, setActionCreators] = useState(null);

  useEffect(() => {
    setActionCreators({
      fetchTodos: async (id) => {
        const todos = await getTodos(id);
        dispatch({ type: 'GET_TODOS', payload: todos });
      },
    });
  }, []);

  return actionCreators;
}

Whereas you could just put this on in the Context.Provider value instead:

  fetchTodos: async (id) => {
    const todos = await getTodos(id);
    dispatch({ type: 'GET_TODOS', payload: todos });
  },
kentcdodds commented 5 years ago

Why would actionCreators need to be in state?

function useTodos(id) {
  const [state, dispatch] = React.useContext(TodosContext)
  // naturally, this would also need to handle error/loading states as well.
  const fetchTodos = async id =>
    dispatch({ type: 'GET_TODOS', payload: await getTodos(id) })
  return { state, fetchTodos }
}

Seems pretty straightforward to me 🤷‍♂️

BruceL33t commented 5 years ago

@kentcdodds to avoid unnecessary api requests (yes, just tried yours, without state, and it hit the api for each render, without the state and useEffect of course) :-)

I also don't see the benefit of putting the actionCreator in a hook, instead of directly on the context. But curious to know what your thoughts were with this. Maybe error handling in the hook would be cleaner, but is that the only reason? :-)

For those interrested I made a question here with running code example of both versions: https://stackoverflow.com/questions/55786752/how-to-manipulate-context-attach-function-to-context-or-wrap-dispatch-in-hook

kentcdodds commented 5 years ago

Feel free to do it however you want 🤷‍♂️ Looking at both implementations, I think I like putting it in a custom hook better, but it has no substantive difference because everyone will be consuming context via the custom hook anyway (I don't recommend exposing the context object itself).

kentcdodds commented 5 years ago

I did write about this https://kentcdodds.com/blog/application-state-management-with-react