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

Async actions should return the same type differentiated by an error object #38

Open
casr opened this issue Aug 10, 2017 · 5 comments
Open

Comments

@casr
Copy link

casr commented Aug 10, 2017

From the flux-standard-action repo:

Flux actions can be thought of as an asynchronous sequence of values. It is important for asynchronous sequences to deal with errors. Currently, many Flux implementations don't do this, and instead define separate action types like LOAD_SUCCESS and LOAD_FAILURE. This is less than ideal, because it overloads two separate concerns: disambiguating actions of a certain type from the "global" action sequence, and indicating whether or not an action represents an error. FSA treats errors as a first class concept.

Was there a particular reasoning to do it with *_DONE/*_FAILED?

@aikoven
Copy link
Owner

aikoven commented Aug 17, 2017

To be honest, I just didn't know that.

One reason to have separate action types is that you can distinguish between success and failure in DevTools without having to look inside the action.

On the other hand, I kinda like this:

// somewhere in reducer
if (isType(action, myAction.finished)) {
  if (action.error) {
    // error handling
  } else {
    // success handling
  }
}

instead of this:

if (isType(action, myAction.done)) {
  // success handling
}
if (isType(action, myAction.failed)) {
  // error handling
}

because in the former case the type of payload is unknown unless you guard the block with action.error. So it is somewhat more explicit and you are forced to handle errors.

Still, inability to see whether an action succeeded in DevTools is a major drawback for me. There's probably a way to have separate action types via single action creator asyncAction.finished(paylod, isError) and somehow make it work with isType.

What do you think?

@casr
Copy link
Author

casr commented Oct 12, 2017

The DevTools are a valid concern however I think that is small patch away from highlighting actions with certain properties, i.e. if error is true then display action as red.

By making errors tied to the same action you will force your reducers to handle it in every instance rather than letting them slip through unhandled. I would rather my code crash hard instead of letting a few unhandled errors pile up and cause a much harder to debug issue.

Another benefit is that actions may share cleanup style operations regardless of whether they are successes or errors.

And finally, by having a standard action we allow other middleware to operate well. As an example, you may have a generic middleware that captures errors and displays a message to the user so that they may be able to help themselves.

@aikoven
Copy link
Owner

aikoven commented Nov 10, 2017

I started experimenting with discriminating between success and error actions by error field. Unfortunately, there seems to be a bug in TypeScript compiler which doesn't let us do this inside isType check: microsoft/TypeScript#19904

@elyalvarado
Copy link

Hi @aikoven, is there any update on this issue? Maybe typescript 3 supports it. I have a javascript app I'm migrating to typescript and uses FSA as defined in the repo, and would love to use typescript-fsa

@aikoven
Copy link
Owner

aikoven commented Nov 22, 2018

Unfortunately not. I've implemented this feature but the compiler still behaves the same way.

https://github.com/aikoven/typescript-fsa/compare/error-discriminator-experiment

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