Return empty on "do not run" for running a script. #3907
Conversation
Previously, the algorithm returned NormalCompletion(undefined) when the script was not run. But the service worker script will need to distinguish between when the script was run vs not, for deciding whether it can proceed with installation. The counterpart service worker spec change is: w3c/ServiceWorker#1346
|
@domenic It turns out I need to distinguish between "do not run" vs ran. WDYT of this? See w3c/ServiceWorker#1346 |
There was a problem hiding this comment.
I'd prefer keeping the return type of an algorithm consistent. Domenic probably has more ideas but I could think of two ways to do that:
- Return NormalCompletion(empty). "empty," distinct from the ECMAScript value undefined, is defined here: https://tc39.github.io/ecma262/#sec-ecmascript-data-types-and-values
- Return a tuple of a boolean indicating of the script was run, and a Completion Record as usual.
|
Thanks @TimothyGu ! "empty" sounds like it does the trick, assuming that running the script will never result in a NormalCompletion(empty). |
|
@mattto See https://tc39.github.io/ecma262/#sec-runtime-semantics-scriptevaluation, specifically step 13, which guarantees that "empty" will never get returned from classic script evaluation. And https://tc39.github.io/ecma262/#sec-module-semantics-runtime-semantics-evaluation for module scripts. |
|
Thanks. Is Type(empty) defined to be Undefined? The javascript: steps take the result and do Type(result). |
|
Revised. It can be make simpler if Type(empty) is Undefined, but I'm not clear on that. |
|
empty is “no value at all” and it doesn’t have a type – i.e., Type(empty) is undefined (as in not defined, not Undefined 😅). |
source
Outdated
| <p class="note">This value is only ever used by the <span data-x="javascript | ||
| protocol"><code>javascript:</code> URL steps</span>.</p> | ||
| </li> | ||
| <li><p>If <var>evaluationStatus</var> is a normal completion, return |
source
Outdated
| @@ -81986,8 +81986,9 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface | |||
| <li><p>Let <var>evaluationStatus</var> be the result of <span data-x="run a classic | |||
| script">running the classic script</span> <var>script</var>.</p></li> | |||
|
|
|||
| <li><p>Let <var>result</var> be undefined if <var>evaluationStatus</var> is an <span>abrupt | |||
| completion</span>, or <var>evaluationStatus</var>.[[Value]] otherwise.</p></li> | |||
| <li><p>Let <var>result</var> be undefined if <var>evalutionStatus</var> is an | |||
There was a problem hiding this comment.
Thanks, also reminding myself to fix a typo: "evalutionStatus"
mfalken
changed the title
|
Thanks, I've updated the PR. I think the merger will need to edit the commit description to say it's returning "empty" instead of "undefined". |
There was a problem hiding this comment.
I'm not super-excited about introducing another not-actually-there value (in addition to undefined and null) into the world of things people have to care about when consuming our specs. In particular completions that can contain non-JS values as their [[Value]] are pretty rare, and mean every consumer has to handle them in a way that they may not be used to.
But, this is reasonably elegant. And we know our consumers are fairly limited for now. I guess we should just do it.
|
Thanks! FWIW I thought the <boolean, CompletionRecord> tuple would be better for readability but a bit harder to write. I guess we can always adjust later. |
Previously, the algorithm returned NormalCompletion(undefined) when the
script was not run.
But the service worker script will need to distinguish between when the
script was run vs not, for deciding whether it can proceed with
installation. The counterpart service worker spec change is:
w3c/ServiceWorker#1346
/browsing-the-web.html ( diff )
/webappapis.html ( diff )