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

Add useDispatch() variant for dispatching async thunks #1694

Closed
neelkamath opened this issue Mar 16, 2021 · 4 comments
Closed

Add useDispatch() variant for dispatching async thunks #1694

neelkamath opened this issue Mar 16, 2021 · 4 comments

Comments

@neelkamath
Copy link

New Features

I propose either adding a variant of useDispatch() (which I'll refer to as useThunkDispatch() throughout this issue), or updating useDispatch() to handle both sync and async actions.

What is the new or updated feature that you are suggesting?

Currently, I've created the following React Hook to use in my codebase. Note that I'm using Redux Toolkit, React Redux, and TypeScript in my codebase.

/**
 * @example
 * When using React Redux, the following boilerplate is required to dispatch an action for an async thunk:
 *
 * ```typescript
 * function ProfilePic(): ReactElement {
 *   const dispatch = useDispatch();
 *   useEffect(() => {
 *     dispatch(ChatsSlice.fetchChat(3));
 *   }, [dispatch]);
 * }
 * ```
 *
 * This function removes the boilerplate as shown below:
 *
 * ```typescript
 * function ProfilePic(): ReactElement {
 *   useThunkDispatch(ChatsSlice.fetchChat(3));
 * }
 * ```
 */
export function useThunkDispatch(actionCreator: AsyncThunkAction<any, any, any>): void {
  const dispatch = useDispatch();
  useEffect(() => {
    dispatch(actionCreator);
  }, [dispatch, actionCreator]);
}

Why should this feature be included?

Currently, we use useDispatch() to dispatch actions but it doesn't work out of the box for async thunks. If you use useDispatch() directly, you get the following error (I've replaced the actual component names with <ComponentName>):

Warning: Cannot update a component (`<ComponentName>`) while rendering a different component
(`<ComponentName>`). To locate the bad setState() call inside `<ComponentName>`, follow the
stack trace as described in https://reactjs.org/link/setstate-in-render

This error can be fixed by using useEffect() to ensure the action is only dispatched once (StackOverflow reference). With either the addition of useThunkDispatch() or the updation of useDispatch() to handle this case, we needn't worry about this anymore.

What docs changes are needed to explain this?

https://react-redux.js.org/api/hooks must document the new/updated hook.

@markerikson
Copy link
Contributor

I'm not quite sure what you're proposing here.

useDispatch works fine for dispatching thunks in general, assuming that your Redux store has been configured with the thunk middleware (which Redux Toolkit's configureStore API does by default). So, the small example you've got listed there is definitely not necessary to dispatch thunks.

If you're getting some kind of a "can't update component A while rendering component B" error, you have some other bug in your code.

There is a complication with useDispatch and thunks at the TypeScript level, because the default Dispatch type doesn't recognize thunks as an acceptable thing to dispatch, but we have documented info on how to extract the type of the real store.dispatch and use that:

https://redux-toolkit.js.org/tutorials/typescript#define-root-state-and-dispatch-types

@neelkamath
Copy link
Author

I agree it's probably a bug in my code. However, I still feel the need to use this custom hook because of this discussion, and the StackOverflow reference I mentioned in my original comment. As per my understanding, dispatching an async thunk (or perhaps any action) before a functional React component has first rendered causes this bug to occur. So, the developer must ensure the dispatch gets delayed using setTimeout(), useEffect(), etc. The other possibility is that useDispatch() isn't to blame but useSelector() because the selector is asking the component to re-render before its first render since it got new data before the first render. If this assumption of mine is wrong, please let me know, and I'll ask on StackOverflow instead.

@markerikson
Copy link
Contributor

markerikson commented Mar 16, 2021

Yeah, I'm really not sure what the problem statement is here that you're describing. It would help if you could show an example of the actual code you're concerned with

You definitely should not be dispatching actions directly while rendering, if that's what you're asking:

function MyComponent() {
  const dispatch = useDispatch();
  dispatch(anything()) // DON'T DO THIS IN THE RENDER LOGIC ITSELF!!!

  return <div />
}

This applies for any Redux action (plain action or thunk). Or, for that matter, React state setting. Rendering logic should be pure - just props+state -> UI, no side effects.

@timdorr
Copy link
Member

timdorr commented Mar 16, 2021

Actually, that code is also incorrect for the library. It will immediately dispatch when that hook is used, regardless of your intent of what to do with dispatch. That's not what our API should do. We provide the functions, you figure out when to call them.

@timdorr timdorr closed this as completed Mar 16, 2021
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

3 participants