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 type definition should use local generic type in parameter. #375

Closed
Fitzpasd opened this issue Dec 1, 2017 · 0 comments
Closed

ofType type definition should use local generic type in parameter. #375

Fitzpasd opened this issue Dec 1, 2017 · 0 comments

Comments

@Fitzpasd
Copy link
Contributor

@Fitzpasd Fitzpasd commented Dec 1, 2017

What is the current behavior?

The current (second) type definition of ofType is:

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

This is incorrect, since it allows invalid parameters to be passed. Consider this simple example:

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 => {});

// This is also fine, but should give a compiler error
const epic2 = (action$: ActionsObservable<AllActions>) => 
  action$.ofType<ActionOne>(ActionTypes.Two)
    .map(action => {});

What is the expected behavior?

The type definition should be:

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

And this would give a compiler error in epic2 above, which is desirable.

Fitzpasd pushed a commit to Fitzpasd/redux-observable that referenced this issue Dec 1, 2017
jayphelps added a commit that referenced this issue Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant