aikoven / typescript-fsa

Type-safe action creator utilities
MIT License
607 stars 28 forks source link

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

Open casr opened 7 years ago

casr commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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: https://github.com/Microsoft/TypeScript/issues/19904

elyalvarado commented 6 years ago

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 commented 6 years ago

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