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

Dispatching an action creator should result in an error #3299

Closed
thSoft opened this issue Mar 27, 2023 · 4 comments
Closed

Dispatching an action creator should result in an error #3299

thSoft opened this issue Mar 27, 2023 · 4 comments

Comments

@thSoft
Copy link

thSoft commented Mar 27, 2023

This example contains a subtle bug: in line 15, the action creator is dispatched instead of the action itself.

If you click on the increment button, nothing happens and the programmer might be confused why, so I think this should be an error either at compile-time (I'm aware of #2896 but it doesn't explicitly mention this case) or at runtime (maybe a warning should be printed when an action is not handled by any reducer?).

@markerikson
Copy link
Collaborator

It is subtle, but I don't think there's much we we can do about it.

Passing a function to dispatch is legal, if you have the thunk middleware installed (and RTK's configureStore sets that up by default). It's just that in this case, the function is an action creator, and not a thunk.

Note that this example will not result in an action being dispatched. Instead, the thunk middleware will see "hey, it's a function", call it, and not pass anything onwards:

so what happens is "nothing at all", rather than "some action gets dispatched".

Also, it's actually entirely legal and fine to have an action dispatched that does not result in any state updates, which goes along with our recommendation to model actions as "events".

The only potential improvement we could make here would be to somehow improve the TS types to indicate that functions resembling action creators are not allowed even when you have the thunk middleware added to the store... but that wouldn't help in this specific case because the example is plain JS.

@thSoft
Copy link
Author

thSoft commented Mar 27, 2023

Thanks for the quick reply and the extensive explanation! However, it's not entirely clear for me whether either a type error or a runtime error is feasible in this case, can you please clarify this?

(One side-note: the above CodeSandbox is definitely TypeScript, not plain JS, I even added a type argument to useDispatch to make sure the action is properly typed.)

@EskiMojo14
Copy link
Collaborator

EskiMojo14 commented Mar 27, 2023

if you're using a thunk middleware, it's valid to dispatch functions and you wouldn't get a runtime error (the thunk middleware would just call it with dispatch)

dispatch(increment) === increment(dispatch, getState)

if you're not, then the final dispatch in the redux store would throw a runtime error.

type-wise, this is actually unrelated to the thunk middleware - RTK action creators have a static .type property, meaning they're assignable to Action and therefore AnyAction. (if this wasn't the case, only ActionCreatorWithoutPayload would be able to be dispatched as above, as any sort of first parameter that wasn't Dispatch would immediately error)

@markerikson
Copy link
Collaborator

markerikson commented Aug 16, 2023

Resolved by adding the actionCreatorMiddleware in #3414 and adding it by default, available in the 2.0 beta.

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