aikoven / typescript-fsa-redux-saga

TypeScript FSA utilities for redux-saga
MIT License
61 stars 4 forks source link

Add mustDispatchStartedAction option #4

Closed asilgag closed 6 years ago

asilgag commented 6 years ago

Having an action created by AsyncActionCreators generates 3 different actions:

But when using takeLatest/takeEvery, the only way to "start" a saga is by dispatching a fourth action (i.e.: MY_ACTION_TRIGGER) used as a trigger. This fake trigger action must mimic the same payload of MY_ACTION_STARTED, and must pass it to that started action (MY_ACTION_STARTED).

This way of triggering the saga is redundant from a Redux point of view, because:

I have added a mustDispatchStartedAction option (true by default), to make the dispatch of the started action optional.

This way, saga listens to MY_ACTION_STARTED action, and will only dispatch done/failed upon finish. Hence, no trigger action is necessary.

MJLHThomassen-Eurocom commented 6 years ago

I was also wondering about this. "Triggering" async actions without inputs can be done with dispatch(myAsyncAction), but triggering async actions with parameters requires a 4th non-async action that can be dispatched with a parameter which redux-saga can takeEvery/takeLatest and then "call" the bindAsyncAction result passing it the parameters of the "trigger action".

I'd love for this change to fix that need for a 4th "dummy" aciton aswell.

aikoven commented 6 years ago

The started action wasn't originally meant to act as a trigger. In my projects, I use actions as events, not commands. This way, an async process is triggered by another event, for example, a click on some button. That clicked action doesn't duplicate the started action, they may have different payloads: clicked action may have no payload at all; the saga that caught it may collect some data from the state before passing it to the bound async process. There may be many instances of that process running in parallel and we can use the payload in started action to distinguish between them and correlate the done/failed actions.

That said, I have no objections to using this library in a way different from my own.

Regarding the PR: let's move the option to the options object, it will make it easier to extend it in the future. Also, let's drop the must and make it just dispatchStartedAction. What do you think?

MJLHThomassen-Eurocom commented 6 years ago

I use actions as events, not commands. This way, an async process is triggered by another event, for example, a click on some button. That clicked action doesn't duplicate the started action, they may have different payloads: clicked action may have no payload at all; the saga that caught it may collect some data from the state before passing it to the bound async process.

Thats basically what I am doing now:

unregisterConsumer: actionCreator<string>(ConsumersPageActionTypes.UNREGISTER_CONSUMER),
    unregisterConsumerAsync: actionCreator.async<string, void, ApiError>(ConsumersPageActionTypes.UNREGISTER_CONSUMER)

The unregisterConsumer "event" is fired when the user clicks the button "Unregister Consumer" in my ui, then I have a Saga "takeEvery" ConsumersPageActionTypes.UNREGISTER_CONSUMER which then uses your plugin to start the async process of dispatching the right webapi call etc.

In prace though, this always leads to having to have 2 actions for async processes. Namely the async version and another action (which may or may not have the same name and may or may not have additional side effects) which has to be fired to start the async process somehow.

In my app I have many situations in which i just want to start the async process without other side effects for which I require other actions (that could then call the async action processes).

Perhaps this should not be fixed here but in typescript-fsa, expanding the .async method to optionally also create a "trigger" action. Though the proposed solution here also works.

As for some of my input: I agree with the naming you propose

aikoven commented 6 years ago

@MJLHThomassen-Eurocom An action is really an event is when it's not concerned with any of its effects. It only gives knowledge of what happened and not what's going to happen. So if an action is explicitly known as a trigger, it's not an event anymore.

Moreover, a single action may be handled in multiple places — reducers, sagas, even multiple reducers/sagas. In that case, it becomes ambiguous of what this action a trigger of.

I always try to make my actions strict events so they are completely decoupled from any logic that's executed as the result of dispatching them. I also put action definitions close to the source that dispatches them — e.g. a React component or a saga. This way a component only manifests what can happen inside it — kind of an interface of a component for the rest of an app. IMO it makes it much easier to design actions — you don't have to think of it as a method, you don't have to think of its outcome, and finally, you don't have to think of how to name it :), it's just "user clicked that button".

MJLHThomassen-Eurocom commented 6 years ago

@aikoven Maybe a bit off-topic, but thanks for that description. I hadn't looked at it that way. I'm currently trying the approach you outline here for one of my larger container components and so far I'm liking it. Thanks for the suggestion!

aikoven commented 6 years ago

@asilgag I am very sorry, I've forgotten about this issue.

Merging it in, will release soon with a bunch of other stuff.

aikoven commented 6 years ago

Released in v1.1.0.

I've moved the flag to options argument, see README.