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

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

Closed
piotrwitek opened this issue Apr 4, 2018 · 5 comments · Fixed by #33
Closed

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

piotrwitek opened this issue Apr 4, 2018 · 5 comments · Fixed by #33
Assignees
Milestone

Comments

@piotrwitek
Copy link
Owner

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 piotrwitek added this to the v2.0.0 milestone Apr 4, 2018
@piotrwitek piotrwitek self-assigned this Apr 4, 2018
@piotrwitek
Copy link
Owner Author

piotrwitek commented Apr 11, 2018

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
Copy link

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
Copy link

cjol commented Apr 12, 2018

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
Copy link

riq-dfaubion commented Apr 26, 2018

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
Copy link
Owner Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants