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

typings: make lettable ofType properly narrow action type #385

Merged
merged 1 commit into from Jan 17, 2018

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Dec 25, 2017

Closes #382

@Hotell Hotell changed the title typings: make lettable ofType properly type narrow typings: make lettable ofType properly narrow action type Dec 25, 2017
index.d.ts Outdated
@@ -52,6 +52,6 @@ export declare function createEpicMiddleware<T extends Action, S, D = any>(rootE
export declare function combineEpics<T extends Action, S, D = any>(...epics: Epic<T, S, D>[]): Epic<T, S, D>;
export declare function combineEpics<E>(...epics: E[]): E;

export declare function ofType<T extends Action>(...keys: T['type'][]): (source: Observable<T>) => Observable<T>;
export declare function ofType<T extends Action, R extends T, K extends R['type'] = R['type']>(...key: K[]): (source: Observable<T>) => Observable<R>;
Copy link
Member

@jayphelps jayphelps Jan 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would be a breaking change since it now requires two params instead of one. But if we provide a default param for the second one I think that would make it non-breaking and have the same behavior as before wouldn't it? R extends T = T

ofType<T extends Action, R extends T = T, K extends R['type'] = R['type']>

Copy link
Contributor Author

@Hotell Hotell Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, good catch! my bad thx!

@jayphelps
Copy link
Member

@jayphelps jayphelps commented Jan 4, 2018

Thank you for this! Let's discuss my breaking change comment a bit.

Also, I kicked the build as it failed for unrelated reasons and should pass now, assuming it would otherwise.

- 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
Copy link
Contributor Author

@Hotell Hotell commented Jan 16, 2018

hey @jayphelps sorry it took so long :( too busy lately

  • squashed + rebase + conflicts resolved
  • update def to non breaking

thx!

@jayphelps jayphelps merged commit 45d09a7 into redux-observable:master Jan 17, 2018
1 check passed
@jayphelps
Copy link
Member

@jayphelps jayphelps commented Jan 17, 2018

Thanks again! 🕺 I'm gonna do a release in the next couple days.

@Hotell Hotell deleted the ts-ofType branch Jan 17, 2018
@jayphelps
Copy link
Member

@jayphelps jayphelps commented Feb 7, 2018

@Hotell
Copy link
Contributor Author

@Hotell Hotell commented Feb 7, 2018

🍻

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

Successfully merging this pull request may close these issues.

None yet

2 participants