-
Notifications
You must be signed in to change notification settings - Fork 1
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
Make expectBoundFunction
declarations a little more fluid
#58
Conversation
withReducer( | ||
reducer: Reducer<State, any>, | ||
initialState?: DeepPartial<State>, | ||
): CallStage<State, Args, Return> & WithMocksStage<State, Args, Return>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the CallStage<State, Args, Return> &
here? The extends
in the WithMocks
stage should take care of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, didn't expect that extends
there.
I think it'd rather remove the extends
though and keep this here, since that'd just affect a single case and not be the default (that mock could always be skipped).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but the same pattern is used in the AssertionState
. Then I'll leave it as is for now and change the return value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, botched the merge in between, but then cherry-picked the docs changes again. Didn't know they were happening in here.
Resorted to non-inheriting interfaces, and only have the return type be unions. The test implementation didn't need updated annotations, as all interfaces are now explicitly implement on it, and it always returns this
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HenriBeck Ok for you? From my side we could merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tp looks good
…test helper implements all of them
Fixes #53