Bug 165681

Summary: Web Inspector: Console could be made useful for very simple await expressions
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, caitp, inspector-bugzilla-changes, joepeck, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix bburg: review+

Joseph Pecoraro
Reported 2016-12-09 13:43:34 PST
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
Attachments
[PATCH] Proposed Fix (13.85 KB, patch)
2016-12-11 17:29 PST, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (13.87 KB, patch)
2016-12-19 23:01 PST, Joseph Pecoraro
bburg: review+
Blaze Burg
Comment 1 2016-12-09 14:45:54 PST
(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
Comment 2 2016-12-09 16:36:01 PST
> > 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
Comment 3 2016-12-09 16:37:30 PST
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
Comment 4 2016-12-11 17:29:19 PST
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 5 2016-12-12 09:34:20 PST
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
Comment 6 2016-12-12 09:35:56 PST
(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 7 2016-12-12 09:36:20 PST
Comment on attachment 296882 [details] [PATCH] Proposed Fix Clearing r? pending further discussion/prototyping.
Joseph Pecoraro
Comment 8 2016-12-12 09:58:36 PST
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
Comment 9 2016-12-12 10:00:09 PST
> This appears to make our style guide match*
Joseph Pecoraro
Comment 10 2016-12-19 22:51:59 PST
(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
Comment 11 2016-12-19 23:01:28 PST
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
Comment 12 2016-12-19 23:27:09 PST
> 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
Comment 13 2016-12-19 23:29:00 PST
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 14 2016-12-20 09:34:42 PST
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
Comment 15 2016-12-20 09:35:18 PST
Joseph Pecoraro
Comment 16 2016-12-20 13:43:44 PST
Note You need to log in before you can comment on or make changes to this bug.