piotrwitek / typesafe-actions

Typesafe utilities for "action-creators" in Redux / Flux Architecture
https://codesandbox.io/s/github/piotrwitek/typesafe-actions/tree/master/codesandbox
MIT License
2.41k stars 98 forks source link

`ActionsUnion` not working with `buildAction().async()` #24

Closed piotrwitek closed 6 years ago

piotrwitek commented 6 years ago

Async action from buildAction have some type issues with Discriminated Union Type and currently is not working in reducer switch cases!

I'm trying to work around this problem.

piotrwitek commented 6 years ago

After investigation, it's not possible to use intersections to generate types dynamically for async acitons, because intersection with union cancel each other out and remove their instances from resulting discriminated union (this is why it doesn't work with switch cases).

To work around this issue there is a slight change needed to buildAsyncAction API as below:

const fetchUser = buildAsyncAction(
  'FETCH_USER_REQUEST',
  'FETCH_USER_SUCCESS',
  'FETCH_USER_FAILURE'
).withTypes<string, User, Error>();

const fetchUser = buildAsyncAction(
  'FETCH_USER_REQUEST',
  'FETCH_USER_SUCCESS',
  'FETCH_USER_FAILURE'
).withMappers<string, User, Error>(
  id => ({ id }), // request mapper
  ({ firstName, lastName }) => `${firstName} ${lastName}` // success mapper
  ({ message }) => message // error mapper
);
chasecaleb commented 6 years ago

I like that solution, particularly because withMappers allows action creators to have some logic - which the Redux docs even call out as one of the main reasons to write action creators.

Also, even though having to specify the request/success/failure action type literals isn't as fancy as generating them, that at least has the benefit of being explicit. That should lower the learning curve for developers trying to get up to speed on a project that uses this library.

cjol commented 6 years ago

I agree, this is a better solution. I was a bit uncomfortable with the magic involved in generating custom action type literals. I also didn't like the situation where the value "FOO_REQUEST" had the type "FOO"&"REQUEST" (i.e. a different static and run time type), so I'm glad that has disappeared too!

riq-dfaubion commented 6 years ago

For those who follow in my footsteps of frustration with this issue, fret not! There is an easy way to get around this issue that should be compatible with what the API will produce when it's fixed.

const fetchUser = {
  failure: buildAction("FETCH_USER_FAILURE").payload<Error>(),
  request: buildAction("FETCH_USER_REQUEST").payload<string>(),
  success: buildAction("FETCH_USER_SUCCESS").payload<User>(),
}

When the "new" new-api is published, you can switch to it, for now, this is a way to get your types to resolve without adding code in your reducer that will have to be changed later.

piotrwitek commented 6 years ago

@riq-dfaubion thanks, you're totally right And sorry for delays, I had a lot of work in our main project recently so couldn't find time to finalize the release :/ I will try this weekend to do the final stretch