Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support let.await once Reason is updated in BuckleScript #52

Open
aantron opened this issue Feb 20, 2020 · 17 comments
Open

Support let.await once Reason is updated in BuckleScript #52

aantron opened this issue Feb 20, 2020 · 17 comments
Labels

Comments

@aantron
Copy link
Owner

aantron commented Feb 20, 2020

See reasonml/reason#2487.

@aantron
Copy link
Owner Author

aantron commented Feb 20, 2020

.await is the obvious choice for JS familiarity. For some reason, the term never grew on me. I always liked .async, even though in JS it is used on the "other side." If anyone has ideas about what the word should be and why, please say :)

This was referenced Feb 20, 2020
@jordwalke
Copy link

How does the bind/map stuff come into play with a Reason async monadic API? Does one have to do:

() => {
  let.async x = foo();
  let.async y = bar();
  let.asyncMap z = baz();
  x + y + z;
}

@aantron
Copy link
Owner Author

aantron commented Feb 21, 2020

Yes, there will have to be a separate suffix for map. I haven't tried it yet, but maybe we can do let.async.map or similar.

Just thinking now, another option, which would clearly associate the let with the module that's being used, is let.promise (and let.promise.map).

@jordwalke
Copy link

I'm not sure that let.x.y is currently supported, but could be. I really wish users didn't have to deal with thinking about map. One of the reasons is that it makes it really hard to copy/paste/rearrange/insert new async let bindings within a series.

@aantron
Copy link
Owner Author

aantron commented Feb 21, 2020

In my experience, in practice, you almost never end up using map with let-like bindings, because a sequence of lets is actually expanded to nested code like this:

flatMap(p1, v1 =>
  flatMap(p2, v2 =>
    flatMap(p3, v3 =>
       ...)))

rather than what we typically see written manually without let,

p
->flatMap(v1 => ...)
->flatMap(v2 => ...)

Because of the types, there is less opportunity to insert a map in the nested calls anyway. You can usually only insert it as the final let. In practice, for me, that has always meant simply not using map at all, and using resolved manually.

@jordwalke
Copy link

Can you show an example of what this would look like using the syntax extension:

Because of the types, there is less opportunity to insert a map in the nested calls anyway. You can usually only insert it as the final let. In practice, for me, that has always meant simply not using map at all, and using resolved manually.

@aantron
Copy link
Owner Author

aantron commented Feb 22, 2020

@jordwalke I added an experimental Promise.Let module in branch let:

module Let: {
let (let.promise):
(promise('a), 'a => promise('b)) =>
promise('b);
let (let.promise_map):
(promise('a), 'a => 'b) =>
promise('b);
};

module Let = {
let (let.promise) = flatMap;
let (let.promise_map) = map;
};

It's in Promise native only for now, because I didn't want to mess around with getting a development Reason into BuckleScript.

I added some tests for the syntax, you can run them with dune exec test/test_main.exe.

  • A couple of flatMaps:

    test("sequence", () => {
    let.promise n = Promise.resolved(42);
    let.promise n' = Promise.resolved(n + 1);
    Promise.resolved(n == 42 && n' == 43);
    }),

    This body of this test expands to

    Promise.resolved(42)->Promise.flatMap(n =>
      Promise.resolved(n + 1)->Promise.flatMap(n' =>
        Promise.resolved(n == 42 && n' == 43)));

    It has to be this way, because nesting the second flatMap inside the callback of the first is the only straightforward way the last expression could have access to both n and n', as it clearly does.

    It's also consistent with the meaning of let. Everything after a let is nested inside the "continuation" of the let. That's why the variables bound in a let are visible in the code beneath it, including if there are subsequent lets.

    This is different from pipe, which would typically look like

    Promise.resolved(42)
    ->Promise.flatMap(n => Promise.resolved(n + 1))
    ->Promise.flatMap(n' => /* n is not visible. */)
  • A single map:

    test("map", () => {
    let.promise_map n = Promise.resolved(42);
    n == 42
    }),

    This code expands to

    Promise.resolved(42)->Promise.map(n =>
      n == 42);

    This is okay, but, as the rest of this comment should show, it's about as far as you can get with let.map for Promise, which makes let.map kind of useless. let.map is mainly useful with and.map (parallelization) rather than sequencing.

  • What happens if we try to sequence let.map:

    test("map sequence", () => {
    let.promise_map n = Promise.resolved(42);
    let.promise_map n' = Promise.resolved(43);
    n == 42 && n' == 43
    }),

    This test is commented out, because it does not compile. It expands to

    Promise.resolved(42)->Promise.map(n =>
      Promise.resolved(n + 1)->Promise.map(n' =>
        n == 42 && n' == 43));

    which fails with

    Error: This expression [the whole thing] has type
          bool Promise.t Promise.t
        but an expression was expected of type
          bool Promise.t
    

    That's because of a double wrapping of n == 42 && n' == 43 in a Promise, once by the inner map, and then again by the outer map.

    Intuitively, this shouldn't work anyway, because everything after the first let.map goes into map's callback, and that callback has type 'a => 'b, rather than 'a => 'b Promise.t. That means the callback is not allowed to do any more async operations.

    So, if there is a sequence of lets, there can only be one map, and it has to be last. Here's how such a sequence looks:

    test("flatMap then map", () => {
    let.promise n = Promise.resolved(42);
    let.promise_map n' = Promise.resolved(n + 1);
    n == 42 && n' == 43;
    }),

    Compare to the first test case:

    test("sequence", () => {
    let.promise n = Promise.resolved(42);
    let.promise n' = Promise.resolved(n + 1);
    Promise.resolved(n == 42 && n' == 43);
    }),

    map's only effect here is that the code underneath the map does not use Promise anymore, which, in practice, only means that you don't have to wrap the final result in Promise.resolved by hand. I'd argue that this is a pretty useless convenience, compared to the confusion of even having let.map.

    I also think let.map is confusing in Promise, because it reads to me as if the right side of the let is the function to use for mapping, when actually the function to use is everything under the let. By comparison, let.promise reads as if you're pattern-matching on a promise to extract its value, which is pretty accurate.

    To further confirm that you can't insert a let.map anywhere else, you could try commenting in this test, and rearranging it any way:

    test("flatMap then map", () => {
    let.promise_map n = Promise.resolved(42);
    let.promise n' = Promise.resolved(n + 1);
    Promise.resolved(n == 42 && n' == 43);
    }),

    But it fails with the same error

    Error: This expression [the whole thing] has type
          bool Promise.t Promise.t
        but an expression was expected of type
          bool Promise.t
    

Unlike map, get composes nicely with nesting, and I've seen it used successfully with bs-let.

BTW don't worry about the underscore in promise_map. It should be a dot, if we can add support for that. But, because of the above, I'd rather add promise.get instead, and maybe promise.ok for flatMapOk.

@aantron
Copy link
Owner Author

aantron commented Feb 22, 2020

To summarize the above comment, people won't be inserting maps into sequences of lets. And, it's best not to use map even at the end of such sequences, because that actually harms composability since you can't do async operations anymore under the map.

@lessp
Copy link

lessp commented Mar 3, 2020

I've been fiddling around a bit with using let-operators. I find it pretty difficult thinking about what could be defined as future conventions and best practices, but thought I'd share some ideas! Hope you're OK with some bikeshedding! 😅

Map the whole API to let-operators:

let.flatMap
let.map
// ... etc

let.flatMapOk {Response.body, _} = fetch(”...”);

Provide flatMap and map (with result-types), pro's are probably mostly for new onboarding:

/* flatMap */
let.await 
/* or, to follow OCaml-conventions */
let*await

/* map */
let+await

/* flatMapOk */
let.await?

/* mapOk */
let+await?

@aantron
Copy link
Owner Author

aantron commented Mar 4, 2020

@lessp have you written code that substantially uses let.map? Curious about your experience with it in light of #52 (comment).

@lessp
Copy link

lessp commented Mar 7, 2020

@lessp have you written code that substantially uses let.map? Curious about your experience with it in light of #52 (comment).

Ok, so as an experiment, and trying to take on the role as a guinea pig, I used the https://hacker-news.firebaseio.com/v0/-API to:

/* retrieve all ID's of the top stories */
let.flatMapOk {Response.body} = fetch(".../v0/topstories.json");

/* parse the JSON into a list of postIds */
let postIds = /* ... implementation */

/* retrieve a post for each postId */
let.flatMap rawPosts = 
  posts 
  |> List.map(postId => fetch(".../v0/item/" ++ postId ++ ".json")) 
  |> Promise.all;

/* parse the JSON into a list of posts */
let posts = /* ... implementation */

Promise.resolved(Ok(posts)); /* Ok(posts) - if we'd use let.map above */

I'm not sure if this is helpful, but for me, who's main-reference of async/await is from JavaScript, I think the biggest confusion is the fact (correct me if I'm wrong here), that every function that uses "await" now returns a Promise.t('a). So the explicit wrapping actually helps in making that a bit clearer.

Don't know if that helps!

Hackernews Endpoints

let base = "https://hacker-news.firebaseio.com/v0";
let topStories = base ++ "/topstories.json";
let newsStories = base ++ "/newstories.json";
let showStories = base ++ "/showstories.json";
let askStories = base ++ "/askstories.json";
let jobStories = base ++ "/jobstories.json";
let item = id => base ++ "/item/" ++ id ++ ".json";

@Kingdutch
Copy link

Drive by comment below :D

As a programmer who comes from PHP, JavaScript, some Rust to ReasonML, I'd much prefer sticking to the await keyword for "waiting for an async operation to resolve to a value":

  1. It better signals that that statement will hold up your program and not perform any work in the meantime.
  2. It's consistent with other languages so makes switching between JS, Rust, Reason easier.

@lessp
Copy link

lessp commented May 27, 2020

Drive by comment below :D

As a programmer who comes from PHP, JavaScript, some Rust to ReasonML, I'd much prefer sticking to the await keyword for "waiting for an async operation to resolve to a value":

  1. It better signals that that statement will hold up your program and not perform any work in the meantime.
  2. It's consistent with other languages so makes switching between JS, Rust, Reason easier.

The conclusion I arrived at, and the problem as I see it using await is that it implies that it's somehow special-cased, and as long as it is not, it might lead to unnecessary confusion in the long run.

We're already using flatMap, map and other operations for option, result and more so as soon as people get used to that concept I think it'll make sense?

let.flatMap user = Some(user);
let.flatMap user = Ok(user);
let.flatMap user = IO.retrieveUser(~id=1);

I'm not very familiar with Rust, but IIUC you do not have to worry about mapping in its implementation of async/await?

@Kingdutch
Copy link

I'm not very familiar with Rust, but IIUC you do not have to worry about mapping in its implementation of async/await?

Not entirely sure if I'm interpreting your question correctly. As far as I'm aware Rust doesn't do anything special to await a promise (known as Future in Rust) returned from a function you're awaiting.

Rust's implementation of async/await compiles away to a state machine. So you'd be left with a certain value in a certain state, that you'd have to .await to move on. So the user would have to worry about the return type and whether that's a future or some other type.

I think my main problem with flatMap is that I also associate it with arrays and data transformations. Awaiting a promise can be seen as a data transformation. However, using that name for awaiting a promise means that in code it's harder to distinguish where you're accidentally suspending.

Rust's definition for flatMap is

An iterator that maps each element to an iterator, and yields the elements of the produced iterators.

Even though a promise wraps a value, I don't see it as an iterator. The definition of let.flatMap above would probably be something like

Awaits any promises that are returned until a resolved or rejected non-promise-value is reached.

@alexeygolev
Copy link

I think one can alias flatMap to await if that's more convenient. flatMap makes sense because Promise (within the lib, not the original JS one) is a monad. Rust has map for functors such as Result and Option and and_then for monads. Reason community landed on flatMap for (containerType('a), ('a) => containerType('b)) => containerType('b) (so did JS and Scala), while Haskell and OCaml call it bind. Using map and flatMap for Option and Result when programming in ReasonML removes the "iteratior" idea from those functions context.

My main point is flatMap means the same thing for all monads. Whether something is suspending or not is irrelevant thanks to this library.

@lessp
Copy link

lessp commented May 27, 2020

I'm not very familiar with Rust, but IIUC you do not have to worry about mapping in its implementation of async/await?

Not entirely sure if I'm interpreting your question correctly. As far as I'm aware Rust doesn't do anything special to await a promise (known as Future in Rust) returned from a function you're awaiting.

Sorry, what I meant was that, if Reason decided to go with something like:

let foo: unit => Promise.t(User.t);

let async foo = () => {
  let.await result = IO.retrieveUser(~id=1);

  result;
};

instead of:

let foo: unit => Promise.t(User.t);

let foo = () => {
  let.flatMap result = IO.retrieveUser(~id=1);
  

  Promise.resolve(result); 
  /* or `result`, if the final operation would be map instead of flatMap */ 
};

Then, IMHO, it might make more sense to use await.

...and as I understand it, in Rust, you'd do it similarly to that first example where you do not have to map over the result, but in Reason, you would. Hopefully someone can and will correct me if I'm wrong here. 🙂

@alexeygolev
Copy link

@lessp Rust handles async/await as a separate language feature (similar to js). In Reason it's just another monad. This allows for using different implementations for async tasks. As long as they conform to being a monad the syntax will work with them. So one could use Future or some other async implementation if they need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants