-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Utilise tuples to fix function types #82
Conversation
@piotrwitek Any thoughts on this? |
This works for me. Thank you @Carl-Foster. |
@piotrwitek Don't mean to bother, but any update on this? Currently my best workaround is type MyActionUnion = ActionType<typeof actionOne> | ActionType<typeof actionOne>;
const isMyActionUnion = (action: Action): action is MyActionUnion => {
return isActionOf(actionOne, action) || isActionOf(actionTwo, action);
};
// In epic
action$.pipe(
filter(isMyActionUnion),
// ...
); |
For the moment this is blocked, first I need to finish rewrite of type-testing suite to I already rewritten my other library EDIT: Maybe I can do it tomorrow if I stay super motivated |
@piotrwitek do you want me to update this in some way? |
Thanks for a quick response @Carl-Foster, there'll be quite a lot :) So please wait a bit more as for the moment I cannot predict exactly what I'll end up with. I'll see tomorrow. |
Hey, after the analysis the only useful part here is the simplification of overloads in The simplification using tuples looks good from code maintenance perspective and shouldn't change any behaviour of the library type-wise. We need to rebase it and discard all other changes except is-action-of.ts file refactoring. I have just added new type tests cases with type-tests snapshots using new test suite just to be sure that all previous use-cases are still covered and it is not breaking anything. |
Closing as implemented: bab3528 |
I set this up to see if I could use tuple types to fix some of the lingering issues. I think this has fixed it although I'm not 100% sure.
I think this fixes #44 and #63 but does require a bump in TypeScript version