Add the ability to construct a callback function #328
Conversation
This helps move some of the heavy lifting into IDL, to avoid issues like whatwg/html#2381.
index.bs
Outdated
| perform the following steps. | ||
| These steps will either return an IDL value or throw an exception. | ||
|
|
||
| 1. Let |completion| be an uninitialized variable. |
There was a problem hiding this comment.
So this algorithm is pretty similar to https://heycam.github.io/webidl/#invoke-a-callback-function. I wonder whether it's worth trying to factor parts of it out... I guess the main similarity is the preparation (after the basic sanity-check, which is different) and the argument conversions, right?
Maybe that's not enough to worry about it.
There was a problem hiding this comment.
I factored out the arguments conversion, which I think factors the most cleanly. The rest varies a bit too much IMO.
| point |completion| will be set to an ECMAScript completion value. | ||
| 1. [=Clean up after running a callback=] with |stored settings|. | ||
| 1. [=Clean up after running script=] with |relevant settings|. | ||
| 1. Return |completion|. |
There was a problem hiding this comment.
Up front we said "These steps will either return an IDL value or throw an exception." But we're actually returning an ES completion value... I'm not sure how to reconcile those.
Of course I'm also not sure in what sense "converting callResult.[[Value]] to an IDL value" returns an ES completion value...
There was a problem hiding this comment.
This is a preexisting "problem" that ideally I'd like to not fix in this PR.
Personally I think it is OK for specs to treat returning completion values and traditional return/throw flow the same way, without an explicit conversion step. (And similarly, treating return/flow throw as returning completion values.) It's a bit strange that the ES completion record holds a Web IDL value, but IMO fine.
There was a problem hiding this comment.
We could mention this equivalence and our expanded use of ES completion values in the conventions section.
There was a problem hiding this comment.
Yeah, we seem to mix that up all over the place. I guess we should define it more clearly. Maybe that's yet another thing for the infra standard?
There was a problem hiding this comment.
I think having an explicit equivalence here is fine, fwiw. We should just do that.
index.bs
Outdated
| A <dfn>Web IDL arguments list</dfn> is a [=list=] of values each of which is either an IDL value or | ||
| the special value “missing”, which represents a missing optional argument. | ||
|
|
||
| <div algorithm="convert an IDL arguments list to an ECMAScript arguments list"> |
There was a problem hiding this comment.
Since you have a dfn within the algorithm, you don't need to give the algorithm attribute a value.
index.bs
Outdated
| 1. [=list/Append=] |convertResult| to |esArgs|. | ||
| 1. Set |count| to |i| + 1. | ||
| 1. Set |i| to |i| + 1. | ||
| 1. [=list/Remove=] all items from |esArgs| except the first |count|, so that |esArgs|'s |
There was a problem hiding this comment.
"Truncate" is a lot clearer to read, even though it's not defined in infra. Should it be defined there? (see whatwg/infra#100)
| |args|[|i|] to an ECMAScript value. Rethrow any exceptions. | ||
| 1. [=list/Append=] |convertResult| to |esArgs|. | ||
| 1. Set |count| to |i| + 1. | ||
| 1. Set |i| to |i| + 1. |
There was a problem hiding this comment.
Increment? (see whatwg/infra#99).
| <a href="#construct-return"><i>return</i></a>. | ||
| 1. Let |callResult| be [=Construct=](|F|, |esArgs|). | ||
| 1. If |callResult| is an abrupt completion, set |completion| to | ||
| |callResult| and jump to the step labeled <a href="#construct-return"><i>return</i></a>. |
There was a problem hiding this comment.
I know we use steps labelled "return" all over the place. I still find that icky. Of course we shouldn't change it here.
| point |completion| will be set to an ECMAScript completion value. | ||
| 1. [=Clean up after running a callback=] with |stored settings|. | ||
| 1. [=Clean up after running script=] with |relevant settings|. | ||
| 1. Return |completion|. |
There was a problem hiding this comment.
Yeah, we seem to mix that up all over the place. I guess we should define it more clearly. Maybe that's yet another thing for the infra standard?
This helps move some of the heavy lifting into IDL, to avoid issues like whatwg/html#2381.
Preview | Diff