Safe promises #5709
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| include Js_promise | ||
|
|
||
| /** Type-safe t-first then */ | ||
| let then: (promise<'a>, 'a => promise<'b>) => promise<'b> = %raw(` | ||
| function(p, cont) { | ||
| Promise.resolve(p).then(cont) | ||
|
There was a problem hiding this comment. Why do you wrap a promise with There was a problem hiding this comment. Because promises are flattened at runtime and where you expect to have, say There was a problem hiding this comment.
Once it became a promise it will stay a promise. So if we have At least it works this way in JavaScript. Maybe we're talking about different things. There was a problem hiding this comment. 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 Another way to answer: in JS There was a problem hiding this comment. 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 This code covers an edge-case while going against the runtime-free There was a problem hiding this comment.
I think there's some confusion on what the problem is. There was a problem hiding this comment. I consider crashing a healthy thing when it happens during development. I think it's where we derail in our opinions. There was a problem hiding this comment. Especially when using There was a problem hiding this comment. Definitely, wanting those examples to crash is one possible opinion. There was a problem hiding this comment.
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, 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. |
||
| } | ||
| `) | ||
|
|
||
| /** Type-safe t-first catch */ | ||
| let catch: (promise<'a>, error => promise<'a>) => promise<'a> = %raw(` | ||
| function(p, cont) { | ||
| Promise.resolve(p).catch(cont) | ||
| } | ||
| `) | ||
|
|
||
| @deprecated("Use then instead") | ||
| let then_ = then | ||
cristianoc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| 'use strict'; | ||
|
|
||
| var Js_promise2 = require("../../lib/js/js_promise2.js"); | ||
|
|
||
| async function nestedPromise(xxx) { | ||
| var xx = await xxx; | ||
| Js_promise2.then(xx, (function (x) { | ||
| return Promise.resolve((console.log("Promise2.then", x), undefined)); | ||
| })); | ||
| Js_promise2.$$catch(xx, (function (x) { | ||
| console.log("Promise2.catch_", x); | ||
| return Promise.resolve(0); | ||
| })); | ||
| var arg1 = function (x) { | ||
| return Promise.resolve((console.log("Promise.then_", x), undefined)); | ||
| }; | ||
| xx.then(arg1); | ||
| } | ||
|
|
||
| async function create(x) { | ||
| console.log("create", x); | ||
| return x; | ||
| } | ||
|
|
||
| var xx = create(10); | ||
|
|
||
| var xxx = create(xx); | ||
|
|
||
| nestedPromise(xxx); | ||
|
|
||
| exports.nestedPromise = nestedPromise; | ||
| exports.create = create; | ||
| exports.xx = xx; | ||
| exports.xxx = xxx; | ||
| /* xx Not a pure module */ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /*** Problematic example of nested promises is safe with Js.Promise2 */ | ||
|
|
||
| let nestedPromise = async (xxx: promise<promise<int>>) => { | ||
| let xx = await xxx | ||
|
|
||
| let _ = xx->Js.Promise2.then(x => Js.log2("Promise2.then", x) |> Js.Promise.resolve) | ||
| let _ = xx->Js.Promise2.catch(x => { | ||
| Js.log2("Promise2.catch_", x) | ||
| 0 |> Js.Promise.resolve | ||
| }) | ||
|
|
||
| // This crashes | ||
| let _ = Js.Promise.then_(x => Js.log2("Promise.then_", x) |> Js.Promise.resolve, xx) | ||
| } | ||
|
|
||
| let create = async (x) => { | ||
| Js.log2("create", x) | ||
| x | ||
| } | ||
|
|
||
| let xx = create(10) | ||
| let xxx = create(xx) | ||
| let _ = nestedPromise(xxx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.