Safe promises

[cristianoc]

Introduce safe promises, based on #5707

• Add t-first Js.Promise2 with safe bindings
• Remove type warning for nested promises
• Add example illustrating a typical type unsafe example of nested promises, and how it goes away with Js.Promise2

[cristianoc]

TODO: integrate orthogonal quality of life improvements from https://github.com/ryyppy/rescript-promise

TODO2: plan the whole Js.Foo2 story and how it should move to be the default instead. This cleanup could go into v11. For a separate issue.

[cknitt]

Is this really planned for 10.1? Or should it go into 11.0?

[cristianoc]

Is this really planned for 10.1? Or should it go into 11.0?

This is 10.1 material: it's where both async/await and the warning for nested promises were introduced. Based on current understanding, there's no basis for introducing that warning.

[cristianoc]

Not clear how much of rescript-promise should go here.
Leaving the investigation for later: #5719

[DZakh]

Why do you wrap a promise with Promise.resolve?

[cristianoc]

Because promises are flattened at runtime and where you expect to have, say promise<int> because you looked inside promise<promise<int>> what you actually get is an int. When that happens, the int is converted back to the promise<int> you were expecting, so you can call .then on it.

[DZakh]

Because promises are flattened at runtime and where you expect to have, say promise because you looked inside promise<promise> what you actually get is an int.

Once it became a promise it will stay a promise. So if we have promise<promise<int>> in runtime, it'll be flattened to promise<int> (not int). So we can call .then without additional Promise.resolve.

At least it works this way in JavaScript. Maybe we're talking about different things.

[cristianoc]

Not easy to explain in few words. But you can check the example: https://github.com/rescript-lang/rescript-compiler/pull/5709/files#diff-dfcc91a2765ccd42f814d348224f3bc20b343386851e14fd99132000aa56fd78R13

It is flattened to promise<int>, but when you "look inside" i.e. await it, you get int. (Or you could look inside by doing another .then).

Another way to answer: in JS await is happy to receive a value such as a number, it does not need a promise. This was done intentionally. This change does the same for then: it makes then happy to receive a value such as a number.

[DZakh]

Ah, I see. But I'm not sure that the problem should be solved by Promise bindings. It's more of a types problem, not a runtime one.

For example, if xxx has the type promise<promise<promise<int>>>, the Promise2 will also stop working correctly. I think it's fine to provide some helper function to flatten promises eg Promise2.flatten, and leave everything to the user's responsibility.

This code covers an edge-case while going against the runtime-free Js module nature. It tries to keep the correct behavior. That's definitely good, but I'm not sure whether it's worth it.

[cristianoc]

What I suggest is:

let nestedPromise = async (xxx: promise<int>) => {
  let xx = await xxx
  Js.log2("Promise2.then", xx)
}

module Promise2 = {
  external flatten: promise<promise<'a>> => promise<'a> = "%identity"
}

let someFnThatReturnsNestedPromises: unit => promise<promise<promise<int>>> = () => %raw("Promise.resolve(123)")

someFnThatReturnsNestedPromises()
  ->Promise2.flatten
  ->Promise2.flatten
  ->nestedPromise
  ->ignore

This way, we solve the problem immediately as it's raised. No twisting of the promise type meaning. Also, the nestedPromise code became much cleaner.

I think there's some confusion on what the problem is.
The problem is that you can inadvertently write some code that uses nested promises and crash.
The nested promise type can easily be in the middle of an expression, and the user might never see it written down explicitly.
If you already know there are nested promises, then clearly there's no problem and you can flatten them explicitly.
The problem this is trying to solve is when you don't realise you're using nested promises.

[DZakh]

I consider crashing a healthy thing when it happens during development. I think it's where we derail in our opinions.

[DZakh]

Especially when using Js module. Maybe it's for Belt to handle this problem. What do you think?

[cristianoc]

Definitely, wanting those examples to crash is one possible opinion.
What is surprising, at least to me, is that the same example written only in terms of async/await does not crash.
This boils down to await 3 not crashing.

[DZakh]

Definitely, wanting those examples to crash is one possible opinion.

It's how it used to work https://github.com/rescript-lang/rescript-compiler/pull/5709/files#diff-dfcc91a2765ccd42f814d348224f3bc20b343386851e14fd99132000aa56fd78R13

Ideally would be nice to have types that can automatically flatten themselves eg, promise<promise<t>> automatically becomes promise<t> on the type system level. The same thing for the option type and others. But I can imagine that it's not easy to do.

Well, I don't have anything else to add. I'm more into leaving things as they are right now. Both approaches have pros and cons. I'm not a decision-maker here.