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

ofType cannot infer that return type is of that type #382

Closed
EamonnLaffey opened this issue Dec 7, 2017 · 2 comments

Comments

@EamonnLaffey
Copy link

commented Dec 7, 2017

Suppose you have types for two actions:

const PING = "PING";
interface PING {
  type: typeof PING;
}
const ping: () => PING = () => ({
  type: PING,
});

const PONG = "PONG";
interface PONG {
  type: typeof PONG;
}
const pong: () => PONG = () => ({
  type: PONG,
});

type RootAction = PING | PONG;
type RootState = ...;

We should now be able to write epics and have proper typing. For example:

const epic: Epic<RootAction, RootState> = action$ => 
  action$
    .ofType(PING)
    .map(action => /* The type of action here is still PING | PONG */)

Although we come into an issue that ofType does not return an observable that is of the type we expect.

We now have to cast the returned action into the type that we expect it to be. I have previously seen that done as below. However, with typescript 2.6 introducing strict function types we can no longer do that either.

const epic: Epic<RootAction, RootState> = action$ => 
  action$
    .ofType(PING)
    .map((action: PING)  => /* typescript 2.6 with strict function types doesn't like this. */)

One solution to get around this is to use rxjs filter instead of using ofType. However, by doing this the code becomes a little repetitive and difficult to read. This is especially an issue as the pattern of filtering on action type using redux observable is very common.

const epic: Epic<RootAction, RootState> = action$ => 
  action$
    .filter<RootAction, PING>((action): action is PING => action.type === PING)
    .map(action  => /* action is now of type PING! */)

The function ofType is implemented to accept a variadic parameter of keys. However, typescript does not support variadic generic templates (yet). I propose having a number of overloads for different number of parameters.

For example this is the current type definition for ofType within the ActionsObservable class:

ofType(...key: T['type'][]): ActionsObservable<T>;

This would become something like this to solve the problem:

ofType<T extends Action>(key: T['type']): ActionsObservable<T>;
ofType<T1 extends Action, T2 extends Action>(key1: T1['type'], key2: T2['type']): ActionsObservable<T1 | T2>;
ofType(...key: T['type'][]): ActionsObservable<T>;

We can now use ofType and send through the type of the action and get correct type information

const epic: Epic<RootAction, RootState> = action$ => 
  action$
    .ofType<PING>(PING)
    .map(action => /* action is now of type PING! */)

I would be happy to work on this and add more documentation about using redux observable with typescript but would like some input first. So let me know!

@evertbouw

This comment has been minimized.

Copy link
Member

commented Dec 7, 2017

A change already landed on master that adds a generic to the ofType method:

ofType<R extends Action = T>(...key: R['type'][]): ActionsObservable<R>;

https://github.com/redux-observable/redux-observable/blob/master/index.d.ts#L29

Also the lettable version has this signature:

export declare function ofType<T extends Action>(...keys: T['type'][]): (source: Observable<T>) => Observable<T>;

So your last example already works with the lettable operator and is going to work with the dot operator in the next version.

If you want to use multiple action types, wouldn't you be able to do ofType<PING | PONG>(PING, PONG) and achieve the same result?

@Hotell

This comment has been minimized.

Copy link
Contributor

commented Dec 24, 2017

Typescript: 2.6.2
tsconfig: {strict: true}


UPDATE: lettable ofType related issue ReactiveX/rxjs#3125

workaround via filter

// Following Works -> by explicitly setting type guards 
const epic11 = (action$: ActionsObservable<AllActions>) =>
  action$.pipe(
    filter((action: AllActions): action is ActionOne => action.type === ActionTypes.One),
    map((action) => {})
  )

while https://github.com/redux-observable/redux-observable/blob/master/index.d.ts#L29 fixes the .ofType chain via explicitly setting generic action creator types, lettable ofType is still broken.

const enum ActionTypes {
  One = 'ACTION_ONE',
  Two = 'ACTION_TWO',
}
interface ActionOne {
  type: ActionTypes.One
  myStr: string
}
interface ActionTwo {
  type: ActionTypes.Two
  myBool: boolean
}
type AllActions = ActionOne | ActionTwo

// This is fine
const epic1 = (action$: ActionsObservable<AllActions>) => action$.ofType<ActionOne>(ActionTypes.One).map((action) => {})

// With lettable, doesn't work
const epic11 = (action$: ActionsObservable<AllActions>) =>
  action$.pipe(ofType<ActionOne>(ActionTypes.One), map((action) => {}))

image

When explicit generic is removed the type narrowing doesn't work

// With lettable, without explicit generic, no errors, type narrowing doesn't work though
const epic11 = (action$: ActionsObservable) =>
  action$.pipe(ofType<ActionOne>(ActionTypes.One), map((action) => {}))

image

Isn't there any way how to do this without explicit generics + make the lettable work ?

thanks

Hotell added a commit to Hotell/redux-observable that referenced this issue Dec 25, 2017

Hotell added a commit to Hotell/redux-observable that referenced this issue Jan 16, 2018

fix(typings): make lettable ofType correctly narrow action type
- docs: add typescript ofType troubleshooting solution
- chore(npm-scripts): enable all strict type-checking options for TS tests
- test(typings): cover ofType proper type narrowing
- docs: fix code typos
- fix(typings): make ofType definition non breaking

Closes redux-observable#382

Hotell added a commit to Hotell/redux-observable that referenced this issue Jan 16, 2018

fix(typings): make lettable ofType correctly narrow action type
- docs: add typescript ofType troubleshooting solution
- chore(npm-scripts): enable all strict type-checking options for TS tests
- test(typings): cover ofType proper type narrowing
- docs: fix code typos
- fix(typings): make ofType definition non breaking

Closes redux-observable#382

jayphelps added a commit that referenced this issue Jan 17, 2018

fix(typings): make lettable ofType correctly narrow action type (#385)
- docs: add typescript ofType troubleshooting solution
- chore(npm-scripts): enable all strict type-checking options for TS tests
- test(typings): cover ofType proper type narrowing
- docs: fix code typos
- fix(typings): make ofType definition non breaking

Closes #382
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.