Skip to content
This repository has been archived by the owner on Jul 14, 2020. It is now read-only.

Add types for public API #5

Merged
merged 1 commit into from
Apr 27, 2018
Merged

Add types for public API #5

merged 1 commit into from
Apr 27, 2018

Conversation

pittst3r
Copy link
Contributor

So that BigTest can be used in TS projects. The tsconfig.json is for IntelliSense which aids in the creation/maintenance of the type declaration files.

This exports the IStats interface and Assertion and SideEffect function types from the index.d.ts file since those are public APIs. SideEffect is the type signature for the callback passed to Convergence#do() and I thought it was odd that #do(), although intended for side-effects, can perform side-effects and passes on the return value in the new Convergence it returns—unless I misinterpreted what's going on. I would have expected it to only perform side effects and for anything returned from its callback to be swallowed. If both behaviors are desired maybe that indicates an additional method like #map()?

@Robdel12 Robdel12 requested a review from wwilsman April 27, 2018 14:31
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

This awesome! 🎉 Thanks for the PR.

I would have expected it to only perform side effects and for anything returned from its callback to be swallowed. If both behaviors are desired maybe that indicates an additional method like #map()?

I think that sounds right to me. I'm going to wait for a @cowboyd or @wwilsman review to figure out what we should do going forward. I don't think it has to hold this PR up either. But we should figure out what our follow up is

@pittst3r
Copy link
Contributor Author

Confirm, not a blocker.

@cowboyd
Copy link
Contributor

cowboyd commented Apr 27, 2018

and I thought it was odd that #do(), although intended for side-effects, can perform side-effects and passes on the return value in the new Convergence

It's mildly Monadic in so far as it allows you to change both the value and the context, similar to promise.then, so it seems natural that you would not throw away the value?

do is about sequencing side-effects that don't actually run until all antecedent assertions have been met. So, for example, you could model your entire beforeEach chain for your nested contexts as a single convergence, each chained off the previous one.

when w
always x
do first thing
when y
never z
do second thing

The idea is to capture this sequence, so that it can be run again and again and again. But you're right. There are probably both map and flatMap in there somewhere, and I'd be very curious to see what that api would look like.

@wwilsman
Copy link
Contributor

wwilsman commented Apr 27, 2018

I'll clarify what's going on with flattening the return value inside of #do:

A feature of convergences is currying the return value from a previous function in the stack to the next function in the stack. You can use convergences without returning anything, so it's an optional feature that other libraries like @bigtest/interactor utilize a lot.

Support for flattening promise and convergence values inside of #do was only added because we need to wait for those side effects to finish before continuing on to the next function in the stack.

@pittst3r
Copy link
Contributor Author

Thanks y'all. I will ponder this some more and open an issue to track this discussion.

@Robdel12
Copy link
Contributor

Awesome, I'm going to merge this. 🎉

@Robdel12 Robdel12 merged commit 6b4149b into bigtestjs:master Apr 27, 2018
@pittst3r pittst3r deleted the types branch April 27, 2018 17:12
@pittst3r
Copy link
Contributor Author

Noice 🙌

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

Successfully merging this pull request may close these issues.

4 participants