Bug 165681 – Web Inspector: Console could be made useful for very simple await expressions

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Summary:
Console could be made useful for very simple await expressions

In the console it would be really useful to just type:

    json = await fetch("data.json")

However `await` won't work unless it is inside of an async function. So to do something like this you would need to end up with something like:

    var json; (async function() { json = await fetch("data.json") })();

Or, if await was unavailable, then users would have to:

    var json; fetch("data.json").then((x) => { json = x });

I could see a useful convenience for the Inspector Console to allow you to run await expressions to populate a variable. Typing `x = await fetch("foo.json")` will eventually populate `x`. In practice the populate is almost immediate (JavaScript executes very quickly, and a human types relatively slowly). But it can be a long running operation in which case we could inform the user the value has populated.

    Example input:

        (a) await 10
        (b) json = await fetch("data.json")

    Sample Detection:

        1. Input fail to parse normally.
        2. Input succeed parsing if wrapped in "(async function() {" + input + "})"
        3. If there are multiple `await` expressions, bail.
           - Convenience is meant for something very simple, if you are doing complex code just write the full syntax.
        4. If there was an assignment use the variable name, otherwise use $result.
           - We could even make $result a Console ONLY value like $exception, $0, $1..$99 etc

    Transformation for (a) and (b):

        var $result;
        (async function() {
          $result = await 10;
          console.info("`$result` populated");
        })();

        var json;
        (async function() {
             let start = Date.now();
             json = await fetch("data.json");
             let end = Date.now();
             // If this took more than one second to populate, tell the user it populated.
             if ((end - start) > 1000)
                 console.info("`json` populated");
         })();

What do people think?

Advantages: Let users make use of await expressions in the console to simplify poking around in the console
Disadvantages: Obfuscates the fact that await can only be used in async functions. Can be misleading.
Precedent: We already do a convenience conversion for: `{a:1,b:2}` to `({a:1,b:2})` to produce objects instead of producing a Syntax Error for labelled statements

[Blaze Burg]

(In reply to comment #0)
> Summary:
> Console could be made useful for very simple await expressions
> 
> In the console it would be really useful to just type:
> 
>     json = await fetch("data.json")
> 
> However `await` won't work unless it is inside of an async function. So to
> do something like this you would need to end up with something like:
> 
>     var json; (async function() { json = await fetch("data.json") })();
> 
> Or, if await was unavailable, then users would have to:
> 
>     var json; fetch("data.json").then((x) => { json = x });
> 
> I could see a useful convenience for the Inspector Console to allow you to
> run await expressions to populate a variable. Typing `x = await
> fetch("foo.json")` will eventually populate `x`. In practice the populate is
> almost immediate (JavaScript executes very quickly, and a human types
> relatively slowly). But it can be a long running operation in which case we
> could inform the user the value has populated.
> 
>     Example input:
> 
>         (a) await 10
>         (b) json = await fetch("data.json")
> 
>     Sample Detection:
> 
>         1. Input fail to parse normally.
>         2. Input succeed parsing if wrapped in "(async function() {" + input
> + "})"

Are there conflicts? Could other syntax errors aside from `await` be made whole? If so we might need to sniff for the actual `await` keyword.

>         3. If there are multiple `await` expressions, bail.
>            - Convenience is meant for something very simple, if you are
> doing complex code just write the full syntax.
>         4. If there was an assignment use the variable name, otherwise use
> $result.
>            - We could even make $result a Console ONLY value like
> $exception, $0, $1..$99 etc
> 
>     Transformation for (a) and (b):
> 
>         var $result;
>         (async function() {
>           $result = await 10;
>           console.info("`$result` populated");
>         })();
> 
>         var json;
>         (async function() {
>              let start = Date.now();
>              json = await fetch("data.json");
>              let end = Date.now();
>              // If this took more than one second to populate, tell the user
> it populated.
>              if ((end - start) > 1000)
>                  console.info("`json` populated");
>          })();
> 
> What do people think?
> 
> Advantages: Let users make use of await expressions in the console to
> simplify poking around in the console
> Disadvantages: Obfuscates the fact that await can only be used in async
> functions. Can be misleading.
> Precedent: We already do a convenience conversion for: `{a:1,b:2}` to
> `({a:1,b:2})` to produce objects instead of producing a Syntax Error for
> labelled statements

Let's do it!

[Joseph Pecoraro]

> >     Sample Detection:
> > 
> >         1. Input fail to parse normally.
> >         2. Input succeed parsing if wrapped in "(async function() {" + input
> > + "})"
> 
> Are there conflicts? Could other syntax errors aside from `await` be made
> whole? If so we might need to sniff for the actual `await` keyword.

Yes, if the user typed other keywords only valid in functions, such as `return`:

    (c) return 10

So our verification of (2) would need to be smart about this. Basically we should verify the AST tree contains what we expect a single await expression (optionally wrapped in a single assignment expression) and not a return statement or something else.

[Joseph Pecoraro]

Essentially I should rephrase step (3) above be just a smart verification. Given I'd implement step (2) using Esprima, I figured I'd be using it for verification anyways, I was just making broad strokes in my earlier comment.

[Joseph Pecoraro]

Created attachment 296882 [details]
[PATCH] Proposed Fix

I think I want to play with this a bit more. The `console.info` may be a little too weird since for primitive values it stringifys the primitive. So:

    console.info(1);
    console.info("alpha");

Look the same as info messages with the string "1" or "alpha".

We could do something like:

    console.info("Awaited result", x)

Or go whole hog and add a protocol method saying this is expected to produce a result and treat it specially on the frontend. (It could show number of ongoing await expressions, etc).

[Blaze Burg]

Comment on attachment 296882 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=296882&action=review

The code changes look good, but I agree that stringifying the result is going to be confusing and we should try to avoid it somehow.

> LayoutTests/inspector/controller/runtime-controller.html:23
> +function causeExceptionImmediately() {

Nit: throwExceptionImmediately

> LayoutTests/inspector/controller/runtime-controller.html:76
> +        description: "Test evaluating a simple await expression wraps it into an async function and eventually resolves the value.",

Grammaro.

> Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:181
> +            esprimaSyntaxTree = esprima.parse("(async function(){" + originalExpression + "})");

Nit: template literal

> Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:206
> +            && statement.expression.type === "AwaitExpression") {

Unusual deviation from style guide, but it does make this more readable. Hmm. Up to you.

[Blaze Burg]

(In reply to comment #4)
> Created attachment 296882 [details]
> [PATCH] Proposed Fix
> 
> I think I want to play with this a bit more. The `console.info` may be a
> little too weird since for primitive values it stringifys the primitive. So:
> 
>     console.info(1);
>     console.info("alpha");
> 
> Look the same as info messages with the string "1" or "alpha".
> 
> We could do something like:
> 
>     console.info("Awaited result", x)

That will not get `x` assigned to the proper $result or $n. Is there a way we can do that?

> Or go whole hog and add a protocol method saying this is expected to produce
> a result and treat it specially on the frontend. (It could show number of
> ongoing await expressions, etc).

This seems overly complicated given the small utility on the frontend. But maybe its unavoidable given the issues above re: stringification and result bindings.

[Blaze Burg]

Comment on attachment 296882 [details]
[PATCH] Proposed Fix

Clearing r? pending further discussion/prototyping.

[Joseph Pecoraro]

Comment on attachment 296882 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=296882&action=review

>> Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:206
>> +            && statement.expression.type === "AwaitExpression") {
> 
> Unusual deviation from style guide, but it does make this more readable. Hmm. Up to you.

This appears to make our style guide, unless there is something I'm missing:
https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op

[Joseph Pecoraro]

> This appears to make our style guide

match*

[Joseph Pecoraro]

(In reply to comment #6)
> (In reply to comment #4)
> > Created attachment 296882 [details]
> > [PATCH] Proposed Fix
> > 
> > I think I want to play with this a bit more. The `console.info` may be a
> > little too weird since for primitive values it stringifys the primitive. So:
> > 
> >     console.info(1);
> >     console.info("alpha");
> > 
> > Look the same as info messages with the string "1" or "alpha".
> > 
> > We could do something like:
> > 
> >     console.info("Awaited result", x)
> 
> That will not get `x` assigned to the proper $result or $n. Is there a way
> we can do that?

No, there is currently no way to programmatically get a $n in the Console, which is effectively what this is trying to do. We would have to add something to the protocol, console, or somehow detect awaited results and act on them.

I think we can use:

    console.info("%o", value);

And get pretty good behavior here. Certainly good enough for the common case.

[Joseph Pecoraro]

Created attachment 297512 [details]
[PATCH] Proposed Fix

I think this gets us a lot of benefit now. We can improve later with a $n solution.

[Joseph Pecoraro]

> We can improve later with a $n solution.

I think something like:

    Runtime.evaluateAsync

One way this could be implemented is passing a function (like Runtime.callFunctionOn) where `this` is the global object and param provides an API to do things.

    function (injectedScriptAPI) {
        value = ...;
        injectedScriptAPI.result(value); // equivalent of user typing `value` in the console, so $n.
    }

Another approach would be to take in a program / function resulting in a promise. And sending back the result when the promise resolves. So I think this could just send what we want here:

    (async function() {
        return await <expr>;
    })();

InjectedScript could then wrap this in something like:

    async evaluateAsync(id, expression, options)
    {
        try {
            let result = await this._evaluate(expression, options);
            this._saveResult(result);
            InjectedScriptHost.asyncResult(id, {result: this._wrapObject(result), wasThrown: false});
        } catch (e) {
            InjectedScriptHost.asyncResult(id, {result: e, wasThrown: true});
        } 
    }

I think the one drawback of this approach over the other is that you can't actually get back a Promise value here. But perhaps that is perfectly fine.

[Joseph Pecoraro]

Wow, given what I've just described... this could be Runtime.await. That is pretty much the semantics of what I'm doing anyways, lol.

[Blaze Burg]

Comment on attachment 297512 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=297512&action=review

r=me, very nicely done.

> LayoutTests/inspector/controller/runtime-controller.html:107
> +            // The convenience will not apply to these noexpressions.

Nit: non-expressions

[Radar WebKit Bug Importer]

<rdar://problem/29755339>

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/210033>