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

typescript: ofType loses type information #187

Closed
billba opened this issue Feb 3, 2017 · 8 comments
Closed

typescript: ofType loses type information #187

billba opened this issue Feb 3, 2017 · 8 comments

Comments

@billba
Copy link
Contributor

@billba billba commented Feb 3, 2017

What is the current behavior?

Given the following:

import { Observable } from 'rxjs';
import { Epic } from 'redux-observable';

export type MyActions = {
    type: 'First_Action'
} | {
    type: 'Second_Action'
}

interface MyState {
    field: string;
}

const myEpic: Epic<MyActions, MyState> = (action$, store) =>
    action$.ofType('First_Action')
    .map(action => ({ type: 'Second_Action' }));

In TypeScript-aware editor (e.g. VS Code) mouse over the action parameter in the map. It shows action:any.

What is the expected behavior?

Should show action:MyActions.

Mousing over action$ shows the correct type of ActionsObservable<MyActions>.

Weirdy, mousing over ofType itself shows the correct result type of ActionsObservable<MyActions>

Which versions of redux-observable, and which browser and OS are affected by this issue? Did this work in previous versions of redux-observable?

v13.0.0 - this has been with us since at least the previous release

@sebald
Copy link
Contributor

@sebald sebald commented Mar 6, 2017

Hm, that's really a weird behavior. But I am not sure that it's redux-obervable's fault. It seems like TypeScript can not infer types correctly when using ActionObservable.

I tried the following:

  1. Changed the return type of .ofType() to a regular Observable
    -> ofType(...key: any[]): Observable<T>
    -> TypeScript correctly infers action type

  2. Make ofType generic
    -> ofType<R>(...key: any[]): ActionsObservable<R>
    -> action$.ofType<FirstAction>('First_Action').map(action => ({ type: 'Second_Action' }))
    -> action is any

(1) ist not really applicable, because .ofType() does not return an Observable. I guess TypeScript has issues correctly inferring the type. A reason for this could be the way operators are added to Observables. Maybe TypeScript does not like the pattern :-/


As a workaround you can do the following:

import { Epic } from 'redux-observable';

type FirstAction = {
    type: 'First_Action'
};

type SecondAction = {
    type: 'Second_Action'
};

type MyActions =  FirstAction | SecondAction;

interface MyState {
    field: string;
}

export const myEpic: Epic<MyActions, MyState> = (action$, store) =>
  action$.ofType('First_Action')
    .map<FirstAction, SecondAction>(action => ({ type: 'Second_Action' }));

Not the best solution, but the good thing is that it's even more explicit than having MyAction as input variable for the .map().

@billba
Copy link
Contributor Author

@billba billba commented Mar 6, 2017

In my experience, every time I try to blame a type inference problem on TypeScript, it's usually in my code somewhere. I'll do some experimenting and see if I can track this down.

@jayphelps
Copy link
Member

@jayphelps jayphelps commented Mar 6, 2017

@billba whoops! I can confirm. I believe I found the problem and the solution. PR #208

Can you go into your node_modules/redux-observable/index.d.ts file and edit:

-  lift(operator: Operator<any, T>): ActionsObservable<T>;
+  lift<R>(operator: Operator<T, R>): ActionsObservable<R>;

Now sure wtf I was thinking when I set it as any. 🤦‍♂️

@rgbkrk rgbkrk closed this in #208 Mar 6, 2017
rgbkrk added a commit that referenced this issue Mar 6, 2017
@billba
Copy link
Contributor Author

@billba billba commented Mar 7, 2017

I swear I tried that and it didn't work and then you tried it and it did work. WHAT ELDRITCH MAGIC DO YOU WIELD @jayphelps?

@jayphelps
Copy link
Member

@jayphelps jayphelps commented Mar 7, 2017

@billba

I am a god

@billba
Copy link
Contributor Author

@billba billba commented Mar 7, 2017

This explains so much.

@sebald
Copy link
Contributor

@sebald sebald commented Mar 8, 2017

@jayphelps Has this magical creature some time to spare and can make a 0.14.1 release for all those TypeScript users!? 😃

@jayphelps
Copy link
Member

@jayphelps jayphelps commented Mar 8, 2017

@sebald done. 0.14.1

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

Successfully merging a pull request may close this issue.

3 participants