Add functions to convert to / from a Promise to a LazyPromise #12
Conversation
|
cc: @robertdp as you helped with this module -- let me know if you have any thoughts! |
src/Web/Promise/Lazy.purs
Outdated
| toPromise :: forall a. LazyPromise a -> Effect (P.Promise a) | ||
| toPromise (LazyPromise p) = unbox =<< p |
There was a problem hiding this comment.
These two functions could also be renamed toLazyPromise :: Effect (Promise a) -> LazyPromise a and fromLazyPromise :: LazyPromise a -> Effect (Promise a) if desired. I'm open to other names.
There was a problem hiding this comment.
I think the current names are good since they're in the Lazy module already - I assume the common usage would be something like: LazyPromise.toPromise, whereas I'd call them Promise.toLazyPromise etc. if they lived in the non-lazy module 🙂
src/Web/Promise/Lazy.purs
Outdated
| toPromise (LazyPromise p) = unbox =<< p | ||
| where | ||
| unbox :: forall b. P.Promise (Box b) -> Effect (P.Promise b) | ||
| unbox = runEffectFn2 P.then_ (mkEffectFn1 \(Box b) -> pure (P.resolve b)) |
There was a problem hiding this comment.
An unintuitive issue here is promise flattening. LazyPromise uses Box to add structure and prevent flattening. So removing it will reintroduce flattening.
There was a problem hiding this comment.
Does inferring the type for toPromise add the Flatten constraint?
There was a problem hiding this comment.
It doesn't infer the constraint. Are you envisioning something like this implementation?
toPromise :: forall a. Flatten a b => LazyPromise a -> Effect (P.Promise b)
toPromise (LazyPromise p) = runEffectFn2 P.then_ (mkEffectFn1 \(Box b) -> pure (P.resolve b)) =<< por
toPromise :: forall a. Flatten a a => LazyPromise a -> Effect (P.Promise a)I'm unfortunately not familiar with promise flattening 😬.
There was a problem hiding this comment.
Ah, I assumed the module P referred to Web.Promise. This is where the type-safe functions that model flattening are.
I would recommend using Web.Promise.then_ and Web.Promise.resolve for these new functions, instead of the internal unsafe functions.
There was a problem hiding this comment.
Does this inferred type signature look correct to you?
toPromise :: forall a b c. Flatten a c => Flatten c b => LazyPromise a -> Effect (P.Promise b)
toPromise (LazyPromise p) = Promise.then_ (\(Box b) -> pure (Promise.resolve b)) =<< pThere was a problem hiding this comment.
The double flatten constraint looks redundant to me. It looks like the unnecessary one is coming from the Promise.then_ constraint. How about:
toPromise :: forall a b. Flatten a b => LazyPromise a -> Effect (P.Promise b)
toPromise (LazyPromise p) = runEffectFn2 P.then_ (mkEffectFn1 \(Box b) -> pure (Promise.resolve b)) =<< pThere was a problem hiding this comment.
I think this is now basically identical to your version here: https://github.com/purescript-web/purescript-web-promise/pull/12/files#r655780671
I reckon go with your implementation in the link, you had it right.
There was a problem hiding this comment.
Mind taking another look at the latest?
Description of the change
It can be useful to convert a
Promiseinto aLazyPromiseor vice versa -- for example, several other libraries likeweb-fetchcan createPromisevalues, but to be inserted in a chain of binds withLazyPromisevalues you'll need to convert it first. This PR adds two helper functions for this purpose.Checklist:
Linked any existing issues or proposals that this pull request should closeUpdated or added relevant documentation