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

isOfType could accept an array of actions instead of one #85

Closed
paszkowskiDamian opened this issue Oct 10, 2018 · 8 comments
Closed

isOfType could accept an array of actions instead of one #85

paszkowskiDamian opened this issue Oct 10, 2018 · 8 comments

Comments

@paszkowskiDamian
Copy link
Contributor

paszkowskiDamian commented Oct 10, 2018

Is your feature request related to a problem?

Sometimes we have epics that listen for more than one action. It would be really cool if could preserve type-safety of isOfType while being able to filter more than one action.

Describe a solution you'd like

return action$.pipe(
    filter(isOfTypes(RELOAD_ORDERS , ORDERS_FETCH_REQUEST)),
    mergeMap(async action =>  ordersheetRepository.loadOrders(action.payload)) // action is union of RELOAD_ORDERS , ORDERS_FETCH_REQUEST actions
    ...

Who does this impact? Who is this for?

It is for TypeScript users who would like to handle multiple actions using one epic.

Describe alternatives you've considered

We could use ofType operator from redux-observable but it doesn't return proper types.

Additional context

This is my local implementaion:

export function isOfTypes<T extends string[]>(...actions: T):
  <Action extends {type: string}>(action: Action) => action is Action extends {type: Unboxed<T>} ? Action : never {
    return <Action extends {type: string}>(action: Action):
    action is Action extends {type: Unboxed<T>}
      ? Action
      : never => actions.includes(action.type);
}

This could be integrated to already existing ifOsType.

@Carl-Foster
Copy link
Contributor

Any reason you don't use isActionOf?

@piotrwitek
Copy link
Owner

Hey @paszkowskiDamian, thanks for the proposal.
Currently isActionOf supports an array as parameter to handle a parallel scenario.
So IMO it would be just to extend isOfType API to also support an array as alternative parameter and API will stay consistent.

I agree and will integrate this change into the API.

Our test suite will also need to be extended with new API cases.

@paszkowskiDamian
Copy link
Contributor Author

paszkowskiDamian commented Oct 11, 2018

I've opened a new PR #86 implementing new API

@paszkowskiDamian
Copy link
Contributor Author

paszkowskiDamian commented Oct 11, 2018

@Carl-Foster actaully, I didn't try with isActionOf but it seems as it is not narrowing action types properly but I didn't researched it why.

action$.pipe(
    filter(isActionOf([fetchMoreAccountsFailed, fetchAccountsListFailed])),
    mergeMap(async action /*inferes as RootAction*/ => accountsRepository.doSth(action.paylod)),

@IssueHuntBot
Copy link

@issuehuntfest has funded $20.00 to this issue. See it on IssueHunt

@IssueHuntBot
Copy link

@paszkowskiDamian has submitted a pull request. See it on IssueHunt

piotrwitek pushed a commit that referenced this issue Dec 26, 2018
It is PR implementing new API proposed in issue #85. API of `isOfType` has been extended to accept also an array of types as `redux-observable` operator `ofType`. It does not introduce any baking changes.

* [x] Rebase before creating a PR to keep commit history clean
* [x] Clear node_modules and reinstall all the dependencies: `npm run reinstall`
* [x] Run checker npm script: `npm run check`

Extra checklist:
* [x] Always add some description and refer to all the related issues using `#` tag

For bugfixes:
 * [ ] Add at least one unit test to cover the bug that have been fixed

For new feature:
 * [x] Update API docs
 * [x] Add examples to demonstrate new feature
 * [x] Add type unit tests
 * [x] Add runtime unit tests
@IssueHuntBot
Copy link

@piotrwitek has rewarded $14.00 to @paszkowskiDamian. See it on IssueHunt

  • 💰 Total deposit: $20.00
  • 🎉 Repository reward(20%): $4.00
  • 🔧 Service fee(10%): $2.00

@piotrwitek
Copy link
Owner

Resolved by #86

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

No branches or pull requests

4 participants