pelotom / unionize

Boilerplate-free functional sum types in TypeScript
MIT License
402 stars 14 forks source link

Documentation(?): Way to filter multiple types #37

Open mischkl opened 6 years ago

mischkl commented 6 years ago

For libraries such as redux-observable or @ngrx/effects it's a regular requirement to filter a stream of actions, which can be done with unionize like so:

this.actions$.pipe(
    filter(DomainActions.is.MY_DOMAIN_ACTION),
    ...
  );

and most of the time this is enough. However, sometimes it's necessary to filter according to multiple action types. At the moment this could be done with something like the following:

/** Example 1 **/

this.actions$.pipe(
    filter(action => 
      DomainActions.is.MY_DOMAIN_ACTION(action as DomainActions._Union) ||
      AnotherDomainActions.is.ANOTHER_DOMAIN_ACTION(action as AnotherDomainActions._Union))
    ...
  );

or perhaps:

/** Example 2 **/

this.actions$.pipe(
    filter(testMultiple(
      DomainActions.is.MY_DOMAIN_ACTION
      AnotherDomainActions.is.ANOTHER_DOMAIN_ACTION
    )),
    ...
  );

function testMultiple(...predicates: ((a: any) => boolean)[]) {
  return (a: any) => predicates.some(predicate => predicate(a));
}

Using something like the ngrx ofType operator this could be written as:

/** Example 3 **/

this.actions$.pipe(
    ofType(
      DomainActions.MY_DOMAIN_ACTION.name,
      AnotherDomainActions.ANOTHER_DOMAIN_ACTION.name
    ),
    ...
  );

but unfortunately the name property is undefined so I can't do that. 😦

So my question is, @pelotom or anyone else who might have an idea, what is the recommendation for such a case? Should I just create my own testMultiple() utility function à la example 2? Or is there some way to obtain the tag property as a string so I can do something like example 3? Or might there be some other way?

darrenmothersele commented 6 years ago

I use something like your example 2.

I'm not sure how (or if) you can use strings with ofType from ngrx without losing type information.

sledorze commented 6 years ago

@mischkl Here's a quick n dirty external solution.

The code:

const oneOf = <UT extends uz.UnionTypes<any, any>>(u: UT) => <
  TagProp extends keyof typeof u['_Record'][keyof typeof u['_Record']]
>(
  tag: TagProp
) => <K extends keyof typeof u['_Record']>(
  ...keys: K[]
): (<X extends typeof u['_Record'][keyof typeof u['_Record']]>(
  x: X
) => x is Extract<(typeof u['_Record'])[K], X>) => ((x: any) => keys.indexOf(x[tag]) !== -1) as any

usage:

const actions = unionize(..., { tag: 'type' })

...

const isOneOf = oneOf(actions)('type') // Sorry for having to define this

...
// define the possible predicates
const isAOrB = isOneOf('ia', 'ib')
const isA = isOneOf('ia')

...
// use them
    if (isAOrB(c)) {
...
    }

// or inline
    if (isOneOf('ib', 'ic')(c)) {
...
    }

@pelotom we maybe can expose that as part of unionize?

pelotom commented 6 years ago

Hm, this would be natural to include if we used functions of strings instead of properties; just make the is function accept a variable number of tag arguments.

sledorze commented 6 years ago

@pelotom I'm not understanding where you want to go actually.

pelotom commented 6 years ago

@sledorze see #28.

mischkl commented 6 years ago

I've been making due with the testMultiple script I wrote above. I even created an RxJS operator for use with ngrx or redux-observable:

export function filterMultiple<T>(...predicates: ((a: any) => boolean)[]): OperatorFunction<T, T> {
  return filter(testMultiple(...predicates));
}

// Usage:
this.actions$.pipe(
    filterMultiple(
      DomainActions.is.MY_DOMAIN_ACTION,
      AnotherDomainActions.is.ANOTHER_DOMAIN_ACTION
    ),
    ...
  );

IMHO this solution is "good enough", the only downside is that it's not built in and might (or might not) perform better if it was based on string comparison...

@sledorze @pelotom Changing the .is property to be a method would be a breaking change, so I would treat that option with caution. Additionally, the semantics would be kind of odd for cross-domain filtering, since you could end up with something like:

TodoActions.is(TodoAction.ResetTodos.name, AppAction.ClearAppState.name);

Which begs the question, which of the filtered actions do you use for calling "is"? For the purpose of such a string comparison methinks it might make more sense to offer a top-level utility function. Usage could look something like:

import {isAny} from 'unionize';

actions.filter(isAny(TodoAction.ResetTodos.name, AppAction.ClearAppState.name));
// (note that this assumes the existence of a 'name' property.)
pelotom commented 6 years ago

Additionally, the semantics would be kind of odd for cross-domain filtering

Presumably if you’re combining different action types at the top level you’d want a composed action type of them all, no? Perhaps created using something like https://github.com/pelotom/unionize/issues/29#issuecomment-382520408

mischkl commented 6 years ago

@pelotom I was thinking that too. However the syntax would be kind of lengthy in comparison to a global (albeit type-unsafe) function:

actions.filter(TodoAction.add(AppAction).add(AuthAction).is(TodoAction.ResetTodos.name, AppAction.ClearAppState.name, AuthAction.LogoutSuccess.name))
// gets longer for each action type filtered
pelotom commented 6 years ago

I was thinking more like:

const AnyAction = unionize()
  .add(TodoAction)
  .add(AppAction)
  .add(AuthAction);

actions.filter(AnyAction.is('ResetTodos', 'ClearAppState', 'LogoutSuccess'));
sledorze commented 6 years ago

@pelotom that would indeed be a breaking change for 'is'. Can we use another name?