fix(effects): remove throwing error from action filtering

[frango9000]

moved the filtering for only actions after checking if dispatching is true

removed the thrown error to fix the only actions filtering

PR Checklist

Please check if your PR fulfills the following requirements:

• [ ] The commit message follows our guidelines: https://github.com/ngneat/effects/blob/master/CONTRIBUTING.md#commit
• [ ] Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

closes #42

[stackblitz[bot]]

Run & review this pull request in StackBlitz Codeflow.

[NetanelBasal]

Oh, sorry I saw your PR after merging the one from @EricPoul

[NetanelBasal]

@EricPoul do we need https://github.com/ngneat/effects/pull/43/files#diff-dd27678542de63f13a7d88d8b976b69f8d41e6f5830b35471704841d0eb731bfL71?

[EricPoul]

I'd keep an error as it was before. I was able to properly type createAction with { dispatch: true } and ts will show an error if you do return not an Action but if there's { dispatchByDefault: true } the error is the only way to notify a developer that not an Action is returned to the dispatcher.

[frango9000]

It is ok 🙂 But just wondering, if we receive multiple "maybe" actions as we can now, but only one is actually an action, this filtering will throw and the other "maybe actions" which are actually actions won't get dispatched. So it is not actually filtering. It is checking if any "maybe actions" is not an action to not dispatch anything. Or am I missing something?

[EricPoul]

Yes, it's more like an extra guard that will allow us to know that all "possible" actions are really actions so TS won't complain that we try to dispatch "anything".

But just wondering, if we receive multiple "maybe" actions as we can now, but only one is actually an action, this filtering will throw and the other "maybe actions" which are actually actions won't get dispatched.

Yes. If you made an effect with { dispatch: true } or you have { dispatchByDefault: true } then you need to return an action otherwise it's misleading. With { dispatch: true } TS won't let you emit anything but action. With { dispatchByDefault: true } we can't do the same by TS so we need to keep the consistency and throw an error for the developer to know.

[frango9000]

I get it now, thanks! Fell free to close this PR then🙂