cujojs / most

Ultra-high performance reactive programming
MIT License
3.49k stars 231 forks source link

fix(types): Added flow type definitions for await and awaitPromises. #520

Closed critocrito closed 6 years ago

critocrito commented 6 years ago

Summary

Using most with function chaining doesn't type check using flow when using awaitPromises with the following error: Cannot call fromEvent(...).map(...).awaitPromises because property awaitPromises is missing in Stream. The fix adds a type definition for the await and awaitPromises property and allows flow to type check.

I saw that the original type definitions were removed in #494. From my understanding flow doesn't require a type definition for this. But since I'm fairly new to flow I might be missing something. I use flow version 0.73.

briancavalier commented 6 years ago

Hi @critocrito. The problem is that the awaitPromises method should only be callable when this is Stream<Promise<A>>. Unfortunately, while Flow has good inference of this, it lacks the capability to specify constraints on this. That means Flow would silently hide a mistake like:

const s1 = most.of(Promise.resolve(1))
const s2: Stream<string> = s1.awaitPromises() // Ideally would fail, but doesn't

Since one goal of using a type system should be to prevent such mistakes, we removed the method type definition. It will be great if Flow adds support for this constraints. Please voice your support here: https://github.com/facebook/flow/issues/452

As a workaround, you can use the function version of awaitPromises, which does provide the proper constraint. You can still have a fluent API by using thru:

fromEvent(...).map(...).thru(awaitPromises)

Hope that helps!

critocrito commented 6 years ago

Hey @briancavalier, thanks a lot for your quick and informative response. I learned a thing about flow and a thing about most. The workaround solves my problem.

briancavalier commented 6 years ago

Happy to help, @critocrito.