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

Experiment with async-await. #1

Open
wants to merge 44 commits into
base: master
Choose a base branch
from
Open

Experiment with async-await. #1

wants to merge 44 commits into from

Conversation

cristianoc
Copy link
Owner

@cristianoc cristianoc commented May 19, 2022

Build the compiler as usual by following these instructions.

Try the example in example-async. Or go to one of your ReScript repos and do npm link __path_to_rescript-compiler-experiments__.

Notes:

  • Async functions are written as definitions async (...) => ....
  • Await expressions are written as await e.
  • An await expression can only be used in an async context. So e.g. not in the callback of the body of an async function, unless the callback itself is async.
  • To avoid falling easily in the trap of nested promises, a warning is given when the type checker detects the use of nested promises in an intermediate expression. This check applies to promises in any case, even if one does not use async/await.

@cristianoc
Copy link
Owner Author

cristianoc commented May 19, 2022

This is what has been explored so far:

type testable = (. unit) => Js.Promise.t<unit>

let tests: array<testable> = []

let addTest = t => tests->Js.Array2.push(t)->ignore
let addTest1 = (t, x) => tests->Js.Array2.push((. ()) => t(. x))->ignore

//
//
// Basic tests

let foo = async (. x, y) => x + y

let bar = async (. ff) => {
  let a = await ff(. 3, 4)
  let b = await foo(. 5, 6)
  a + b
}

let baz = async (. ()) => await bar(. foo)

let testBaz: testable = async (. ()) => {
  let n = await baz(.)
  Js.log2("baz returned", n)
}

testBaz->addTest

//
//
// Catching exceptions

exception E(int)

let e1: testable = async (. ()) => raise(E(1000))
let e2: testable = async (. ()) => Js.Exn.raiseError("Some JS error")
let e3: testable = async (. ()) => await e1(.)
let e4: testable = async (. ()) => await e2(.)
let e5: testable = %raw(`function() { return Promise.reject(new Error('fail')) }`)

let testTryCatch = async (. fn) =>
  try {await fn(.)} catch {
  | E(n) => Js.log2("testTryCatch: E", n)
  | JsError(_) => Js.log("testTryCatch: JsError")
  }

testTryCatch->addTest1(e1)
testTryCatch->addTest1(e2)
testTryCatch->addTest1(e3)
testTryCatch->addTest1(e4)
testTryCatch->addTest1(e5)

//
//
// Check for nested promise

let singlePromise = async (. x) => x + 1

let nestedPromise = async (. x) => {
  let resolve = x => [Js.Promise.resolve(x)]
  let _result = singlePromise(. x + 1)->resolve
  32
}

//
//
// Test error handling in fetch

module Fetch = {
  //@raises(JsError)
  let fetch = url => Fetch.fetch(url)

  let status = response => Fetch.Response.status(response)
}

let explainError: unknown => string = %raw(`(e)=>e.toString()`)

let testFetch = async (. url) => {
  open Fetch
  switch {await fetch(url)} {
  | response =>
    let status = response->status
    Js.log2("Fetch returned status:", status)
  | exception JsError(e) => Js.log2("Fetch returned an error:", e->explainError)
  }
}

testFetch->addTest1("https://www.google.com/sdkjdkghdsg")
testFetch->addTest1("https://www.google.comsdkjdkghdsg")

//
//
// Callbacks
let withCallback = async (. ()) => {
  async (. x) => await (x->Js.Promise.resolve) + 1
}

let testWithCallback = async (. ()) =>
  Js.log2("callback returned", await (await withCallback(.))(. 3))

testWithCallback->addTest

//
//
// Async list
module AsyncList = {
  let map = async (. l, f) => {
    let rec loop = async (. l, acc) =>
      switch l {
      | list{} => acc
      | list{p, ...rest} => await loop(. rest, list{await p, ...acc})
      }

    await loop(. l->Belt.List.mapReverse(x => f(. x)), list{})
  }
}

let fetchAndCount = {
  let counter = ref(0)

  let ff = async (. url) => {
    let response = await Fetch.fetch(url)
    counter := counter.contents + 1
    (counter.contents, response->Fetch.status)
  }

  ff
}

let testFetchMany = async (. ()) => {
  let fetchedItems = await AsyncList.map(.
    list{
      "https://www.google.com",
      "https://www.google.com",
      "https://www.google.com",
      "https://www.google.com",
      "https://www.google.com",
    },
    fetchAndCount,
  )
  fetchedItems->Belt.List.forEach(((i, s)) => Js.log3("Fetched", i, s))
}
testFetchMany->addTest

//
//
// Fetch with Result type
module FetchResult = {
  let fetch = async (. url) => {
    switch {await Fetch.fetch(url)} {
    | response => Ok(response)
    | exception JsError(e) => Error(e)
    }
  }
}

let nextFetch = (. _response) => Some("https://github.com/")

let testFetchWithResult = async (. ()) => {
  switch await FetchResult.fetch(. "https://www.google.com") {
  | Ok(response1) =>
    Js.log2("FetchResult response1", response1->Fetch.status)
    switch nextFetch(. response1) {
    | None => ()
    | Some(url) =>
      switch await FetchResult.fetch(. url) {
      | Ok(response2) => Js.log2("FetchResult response2", response2->Fetch.status)
      | Error(_) => ()
      }
    }
  | Error(_) => ()
  }
}

// // imaginary syntax
// let testFetchWithResult = async () =>
//   if let Ok(response1) = await FetchResult.fetch("https://www.google.com")
//      and Some(url) = nextFetch(response1)
//      and Ok(response2) = await FetchResult.fetch(url) {
//     Js.log2("FetchResult response2", response2->Fetch.Response.status)
//   }

testFetchWithResult->addTest

//
//
// Run tests

let rec runAllTests = async (. n) => {
  if n >= 0 && n < Array.length(tests) {
    await (@doesNotRaise tests[n])(.)

    await runAllTests(. n + 1)
  }
}

runAllTests(. 0)->ignore

//
//
// Curried functions

let bb = async (x) => await x

let cc = async (x, ~y=x, z) => (await x) + await y + (await z)

let dd = async (x) => {y => (await x) + (await y)}

let ee = async (. x) => {y => (await x) + (await y)}

//
//
// Errors

// let aa =
//   async
//   (. x) => {
//     let cb = (. _) => await x // Error: Await on expression not in an async context
//     cb
//   }

// let _ = async (_, . x) => await x // Error: Await on expression not in an async context

@IwanKaramazow
Copy link
Collaborator

An await expression can only be used in an async context. So e.g. not in the callback of the body of an async function, unless the callback itself is async.

Modern browsers also allow top-level await in modules. Should this be supported by the ReScript compiler?

@cristianoc
Copy link
Owner Author

An await expression can only be used in an async context. So e.g. not in the callback of the body of an async function, unless the callback itself is async.

Modern browsers also allow top-level await in modules. Should this be supported by the ReScript compiler?

Possibly. What are the pluses and minuses?

@cristianoc
Copy link
Owner Author

One immediate question is why at top level in a file and not at top level in a submodule.
And if one wants at top level in a submodule, should that be an async submodule? What does it mean?

@cristianoc
Copy link
Owner Author

I'd probably be inclined to keep changes to a minimum for now, unless future changes are likely to affect today's design.
As long as it is possible to add the extra features later, it can be done once people have tested this extensively.

@IwanKaramazow
Copy link
Collaborator

IwanKaramazow commented Jul 5, 2022

Agreed, let's revisit this at a later point in time. (if needed)

IwanKaramazow pushed a commit to rescript-lang/syntax that referenced this pull request Jul 6, 2022
Reasoning:
In JavaScript `await` is a keyword *only* inside async functions.
```javascript
let await = 1 // OK ✅

async function wait() {
  let await = 1 // NOT OK ❌
}
```

The plot twist is that the newer browsers (chrome or node) support top-level await. (By implicitly wrapping everything in an async function?)

```javascript
let await = 1  // NOT OK, with top level await ❌
```

This makes me think:
* We can replicate JavaScript parser; inside async function expressions, use of `await` as identifier will result into a syntax error
  - Downside: this feels like a responsibility for another part of the compiler, not the parser? This is already implemented in cristianoc/rescript-compiler-experiments#1, so we would also be doing double work.
  - Other downside: if we ever allow top-level await, then we just implemented the above for nothing.
* Allow `await` as a "non-keyword" everywhere with some "tricks".

```javascript
let await = 1
let x = await // Ok, now it gets tricky. Does this start an "await"-expression?
      = await fetch(url) // could be this
      = await() // could be this
      = await await() // or even this
      = await // or we might just be assigning the identifier `await` to `x`

```

 Seems like we can infer the following rules for an `await expression`:
   - space after `await`
   - next token is on the same line as `await`
   - next token indicates the start of a unary expression

 But this "breaks down" in the case of
 ```javascript
 let x = await - 1
 // could be a binary expression: "identifier" MINUS "1"
 // could also be an await expression with a unary expression: `await` `(-1)`
 ```
These whitespace sensitive rules kinda feel super arbitrary. Which makes me think that introducing `await` as a keyword is an ok compromise.
example-async/src/AA.res Outdated Show resolved Hide resolved
@cristianoc
Copy link
Owner Author

@IwanKaramazow one more thing:

let withCallback = async (. ()) => {
  async (. x) => await (x->Js.Promise.resolve) + 1
}

formats to:

let withCallback = async (. ()) => {
  async (. x) => await x->Js.Promise.resolve + 1
}

which does not compile.

cristianoc pushed a commit to rescript-lang/syntax that referenced this pull request Jul 11, 2022
Reasoning:
In JavaScript `await` is a keyword *only* inside async functions.
```javascript
let await = 1 // OK ✅

async function wait() {
  let await = 1 // NOT OK ❌
}
```

The plot twist is that the newer browsers (chrome or node) support top-level await. (By implicitly wrapping everything in an async function?)

```javascript
let await = 1  // NOT OK, with top level await ❌
```

This makes me think:
* We can replicate JavaScript parser; inside async function expressions, use of `await` as identifier will result into a syntax error
  - Downside: this feels like a responsibility for another part of the compiler, not the parser? This is already implemented in cristianoc/rescript-compiler-experiments#1, so we would also be doing double work.
  - Other downside: if we ever allow top-level await, then we just implemented the above for nothing.
* Allow `await` as a "non-keyword" everywhere with some "tricks".

```javascript
let await = 1
let x = await // Ok, now it gets tricky. Does this start an "await"-expression?
      = await fetch(url) // could be this
      = await() // could be this
      = await await() // or even this
      = await // or we might just be assigning the identifier `await` to `x`

```

 Seems like we can infer the following rules for an `await expression`:
   - space after `await`
   - next token is on the same line as `await`
   - next token indicates the start of a unary expression

 But this "breaks down" in the case of
 ```javascript
 let x = await - 1
 // could be a binary expression: "identifier" MINUS "1"
 // could also be an await expression with a unary expression: `await` `(-1)`
 ```
These whitespace sensitive rules kinda feel super arbitrary. Which makes me think that introducing `await` as a keyword is an ok compromise.
IwanKaramazow pushed a commit to rescript-lang/syntax that referenced this pull request Jul 18, 2022
Reasoning:
In JavaScript `await` is a keyword *only* inside async functions.
```javascript
let await = 1 // OK ✅

async function wait() {
  let await = 1 // NOT OK ❌
}
```

The plot twist is that the newer browsers (chrome or node) support top-level await. (By implicitly wrapping everything in an async function?)

```javascript
let await = 1  // NOT OK, with top level await ❌
```

This makes me think:
* We can replicate JavaScript parser; inside async function expressions, use of `await` as identifier will result into a syntax error
  - Downside: this feels like a responsibility for another part of the compiler, not the parser? This is already implemented in cristianoc/rescript-compiler-experiments#1, so we would also be doing double work.
  - Other downside: if we ever allow top-level await, then we just implemented the above for nothing.
* Allow `await` as a "non-keyword" everywhere with some "tricks".

```javascript
let await = 1
let x = await // Ok, now it gets tricky. Does this start an "await"-expression?
      = await fetch(url) // could be this
      = await() // could be this
      = await await() // or even this
      = await // or we might just be assigning the identifier `await` to `x`

```

 Seems like we can infer the following rules for an `await expression`:
   - space after `await`
   - next token is on the same line as `await`
   - next token indicates the start of a unary expression

 But this "breaks down" in the case of
 ```javascript
 let x = await - 1
 // could be a binary expression: "identifier" MINUS "1"
 // could also be an await expression with a unary expression: `await` `(-1)`
 ```
These whitespace sensitive rules kinda feel super arbitrary. Which makes me think that introducing `await` as a keyword is an ok compromise.
cristianoc pushed a commit to rescript-lang/syntax that referenced this pull request Jul 18, 2022
Reasoning:
In JavaScript `await` is a keyword *only* inside async functions.
```javascript
let await = 1 // OK ✅

async function wait() {
  let await = 1 // NOT OK ❌
}
```

The plot twist is that the newer browsers (chrome or node) support top-level await. (By implicitly wrapping everything in an async function?)

```javascript
let await = 1  // NOT OK, with top level await ❌
```

This makes me think:
* We can replicate JavaScript parser; inside async function expressions, use of `await` as identifier will result into a syntax error
  - Downside: this feels like a responsibility for another part of the compiler, not the parser? This is already implemented in cristianoc/rescript-compiler-experiments#1, so we would also be doing double work.
  - Other downside: if we ever allow top-level await, then we just implemented the above for nothing.
* Allow `await` as a "non-keyword" everywhere with some "tricks".

```javascript
let await = 1
let x = await // Ok, now it gets tricky. Does this start an "await"-expression?
      = await fetch(url) // could be this
      = await() // could be this
      = await await() // or even this
      = await // or we might just be assigning the identifier `await` to `x`

```

 Seems like we can infer the following rules for an `await expression`:
   - space after `await`
   - next token is on the same line as `await`
   - next token indicates the start of a unary expression

 But this "breaks down" in the case of
 ```javascript
 let x = await - 1
 // could be a binary expression: "identifier" MINUS "1"
 // could also be an await expression with a unary expression: `await` `(-1)`
 ```
These whitespace sensitive rules kinda feel super arbitrary. Which makes me think that introducing `await` as a keyword is an ok compromise.
@bobzhang
Copy link
Collaborator

bobzhang commented Sep 9, 2022

Do you have some high level docs on the encoding?
I suppose await is translated into a function, what async will be translated into?

  • since we are going to treat promise type with some special caveats, we can make promise a built in type and alias Js.Promise.t = promise``
    How do you detect await in a non async function, is it done in the ast checker?

@cristianoc
Copy link
Owner Author

Let me point directly at the main repo, which has more cleanup. Here I just wanted to show the code for checking nested promises.

Do you have some high level docs on the encoding? I suppose await is translated into a function, what async will be translated into?

The high level description is quite simple: some well formed-ness checks are performed at the ast level, so you can only await in an async context. Then the info about async and await is simple preserved throughout the pipeline until they are emitted verbatim. Nothing surprising there.

Plus, a warning is emitted in case of nested promises used anywhere (e.g. in a sub-expression). This check is orthogonal in principle, but goes well with this feature. I was not sure how to check "anywhere" hence the possibly deep type traversal. This is best effort of course, as the type could be inferred later.

  • since we are going to treat promise type with some special caveats, we can make promise a built in type and alias Js.Promise.t = promise``

Makes sense.

@bobzhang
Copy link
Collaborator

bobzhang commented Sep 9, 2022

Plus, a warning is emitted in case of nested promises used anywhere (e.g. in a sub-expression). This check is orthogonal in principle, but goes well with this feature. I was not sure how to check "anywhere" hence the possibly deep type traversal. This is best effort of course, as the type could be inferred later

This feature could be submitted in a separate PR? It seems not harmful if it is opt-in

@cristianoc
Copy link
Owner Author

Done the built-in promise
rescript-lang/rescript#5650

Is there a way to check for both promise and Js.Promise.t?
E.g. this uses both:

let doublePromise = async (p:Js.Promise.t<int>) => p

@cristianoc
Copy link
Owner Author

Done the built-in promise rescript-lang/rescript-compiler#5650

Is there a way to check for both promise and Js.Promise.t? E.g. this uses both:

let doublePromise = async (p:Js.Promise.t<int>) => p

I guess one could use something like this done for option?

let extract_option_type env ty =
  match expand_head env ty with {desc = Tconstr(path, [ty], _)}
    when Path.same path Predef.path_option -> ty
  | _ -> assert false

@d4h0
Copy link

d4h0 commented Oct 28, 2022

@cristianoc: Thanks for your amazing work on this!

I'm testing the RC at the moment, and noticed that the current implementation requires parentheses, where I didn't expect them to be required.

Wouldn't it be better, if await had a precedence that would not require these parentheses?

As an example, the following:

let br = playwright.chromium -> setup
let br = await br->BrowserType.launch
let page = await br->Browser.newPage
await page->Page.goto("https://example.com")

...fails with Somewhere wanted: promise<'a>.

Instead, I had to use the following:

let br = playwright.chromium -> setup
let br = await (br->BrowserType.launch)
let page = await (br->Browser.newPage)
await (page->Page.goto("https://example.com"))

...which doesn't seem great.

To me, it seems, that br->BrowserType.launch is equal to BrowserType.launch(br), so await br->BrowserType.launch should behave exactly like await BrowserType.launch(br), I feel.

Was this issue discussed somewhere?

@cknitt
Copy link
Collaborator

cknitt commented Oct 28, 2022

Yes, see rescript-lang/syntax#686

@mvaled
Copy link

mvaled commented Dec 21, 2022

Is this being developed now on the main repo (after removal of submodules)? Is this completely abandoned?

@cristianoc
Copy link
Owner Author

Is this being developed now on the main repo (after removal of submodules)? Is this completely abandoned?

Yes

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

Successfully merging this pull request may close these issues.

6 participants