Change Delay Signature

[alfonsogarciacaro]

This is a proposal to change the current signature of Delay in the promise CE from:

Delay: (unit -> JS.Promise<'T>) -> JS.Promise<'T>

To:

Delay: (unit -> JS.Promise<'T>) -> (unit -> JS.Promise<'T>)

This follows recommendations from this article by Scott Waschling: https://fsharpforfunandprofit.com/posts/computation-expressions-builder-part3/

The problem with the current signature is Promises are not cold, so in order to have this effect we need to fake a thenable with cold behavior which is not ideal. After reading the article I realized we can just return the generator which didn't cross my mind before 😅 This change forces changes in other signatures of the CE, but I believe this is a good thing and also changes shouldn't affect user code (tests are passing without modifications).

I've also changed other methods in the CE from Emit to actual F# code to have better control on what's happening.

cc @MangelMaxime @AngelMunoz

[MangelMaxime]

I don't fully understand the CEs changes but if this is better and doesn't break the user code I think it is a good idea.

[AngelMunoz]

This makes them cold until run is called right?

I guess this just affects the behavior internally, in userland most of the time the Run will be called automatically at the end sounds like a good optimization to me

[MangelMaxime]

Hum, @alfonsogarciacaro does it means that now people are forced to use Promise.start again ?

promise {
    printfn "Hello"
}

// Will this promise print hello directly or do the user needs to add
|> Promise.start 
// ?

[alfonsogarciacaro]

This makes them cold until run is called right?

Yes... and no :) Delay must produce something that it's delayed (thus the name). So far we were just "faking" a cold promise (that behaved like async, that is, recomputes everytime is invoked). But after reading Scott's article I realized the F# CE mechanism also accepts returning a function from Delay so we can just return the generator instead of wrapping it in a "fake cold" promise.

does it means that now people are forced to use Promise.start again ?

No, because Run is automatically called for all CEs (if defined). This is actually checked by the "Promises are hot" test. I've also modified the "Promise.start works test" to make this clearer: https://github.com/fable-compiler/fable-promise/pull/25/commits/455864ba2a73c1574952abf9a46bb69b67a8a98b