[Bug 24291] Add a Promise type to WebIDL, and make it not distinguishable from anything

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

Anne annevk@annevk.nl changed:

       What    |Removed                     |Added

             CC|                            |annevk@annevk.nl

--- Comment #1 from Anne annevk@annevk.nl ---

Duplicate of bug 21422?

Contact us to advertise here
# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #2 from Boris Zbarsky bzbarsky@mit.edu ---

Maybe. The important bits in this bug are non-distinguishability and conversion behavior, not the parametrization and whatnot.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

Tab Atkins Jr. jackalmage@gmail.com changed:

       What    |Removed                     |Added

             CC|                            |jackalmage@gmail.com

--- Comment #3 from Tab Atkins Jr. jackalmage@gmail.com ---

Is the "prevent unwanted unioning" thing about avoiding "type or Promise<type>"

signatures?

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #4 from Boris Zbarsky bzbarsky@mit.edu ---

Yes.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

Jonas Sicking jonas@sicking.cc changed:

       What    |Removed                     |Added

             CC|                            |jonas@sicking.cc

--- Comment #5 from Jonas Sicking jonas@sicking.cc ---

Note that it is debated if Promise.cast(x) should mutate x (by freezing it). This is discussed in

domenic/promises-unwrapping#79

If the outcome of that discussion is indeed to freeze the passed in value, that likely makes Promise.cast inappropriate to ever call implicitly, such as during a type conversion.

In that case using Promise.resolve might be more appropriate.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

Domenic Denicola domenic@domenicdenicola.com changed:

       What    |Removed                     |Added

             CC|                            |domenic@domenicdenicola.com

--- Comment #6 from Domenic Denicola domenic@domenicdenicola.com ---

There was never any talk of freezing the argument to Promise.cast(x).

A sandbox like SES may wish to freeze the primordials, as in [1], but that is a separate concern, and is part of the SES library, not the web platform.

[1]: domenic/promises-unwrapping#79

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #7 from Tab Atkins Jr. jackalmage@gmail.com ---

So, discussion in the Service Worker spec-a-thon shows that this kind of unioning is actually rather useful. It comes with some significant limitations, though:

  1. Allowing "T or Promise<T>" is not just "Sync<T> or Async<T>"; it's "Sync<T> or Async<T> or AsyncError". The third case has to be specially

handled, and often has no precedent in the sync-handling case.

  1. Taking a Promise argument significantly restricts what you can do with other function arguments. No other arguments can interact with the Promise'd argument, because (due to Promise chaining, where if one async call fails you try another one) the author may not know exactly which argument it will eventually resolve to. If you ever want to add such an argument, you have to do one of the following: a. Bake the extra arg into the object the Promise resolves to instead. b. Do something hacky like allowing the argument to be "T or [T, MoreStuff]", and letting the Promise resolve to that as well. c. Accept that using a Promise arg doesn't let you vary the other arg based on the used value, and make sure your API can be called from inside the promise callbacks instead if you need that. (That is, allow inverting the logic: in addition to "foo(myPromise)", allow "myPromise.then(foo)".)

As long as you can accept these restrictions, it should be okay to allow a "T or Promise<T>" union. Doing so is really helps with minimizing API surface in

at least some cases. (In the case of Service Workers, it adds three lines of non-obvious code to the most common, basic async-response case. Skipping those lines appears to work in simple cases but will fail in reasonably common corner cases.)

I'd be fine with a strong warning that you shouldn't do so without API review from a relevant standards body, though.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #8 from Domenic Denicola domenic@domenicdenicola.com ---

Tab, would you mind sharing the specific example where you think unioning is useful?

To be clear, the plan is that an API accepting a promise will take anything you give it and run it through Promise.cast before processing it. So you kind of automatically get "unioning" of a sort: you can pass in any non-promise value, and it becomes (via WebIDL overload resolution) a promise for that value.

Is that what you had in mind, or is it something different?

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #9 from Tab Atkins Jr. jackalmage@gmail.com --- (In reply to Domenic Denicola from comment #8)

Tab, would you mind sharing the specific example where you think unioning is useful?

Sure! I'll do so below.

To be clear, the plan is that an API accepting a promise will take anything you give it and run it through Promise.cast before processing it. So you kind of automatically get "unioning" of a sort: you can pass in any non-promise value, and it becomes (via WebIDL overload resolution) a promise for that value.

Is that what you had in mind, or is it something different?

That's acceptable, but not ideal. It implies consing up a fresh Promise that is then immediately and silently consumed by the API. On the other hand, it keeps you honest and prevents you from designing different behavior in the sync/async argument codepaths.

So, the example.

When a page with an installed Service Worker makes a request, we shoot a "fetch" event at the SW to allow it to intercept the request. To do so, the SW has to at some point call "event.respondWith(...)", with ... being either a Response object or a Promise<Response>. The latter is what's returned by

closely-related APIs like the Cache objects, so it's rather convenient. This means you can just write code like:

e.respondWith(cache.get(e.requestURL))

You might think that you can just invert that and do:

cache.get(e.requestURL).then(e.respondWith.bind(e))

But you can't! (Also, note that this line is more complex by a bit anyway, due to the necessary binding.) Or at least, it's not identical behavior.

You need to signal within the "fetch" event that you're going to handle the response. If you don't, the UA will let the next "fetch" listener handle it, or just send it to the network if there's no one else. So you have to augment the code to:

e.preventDefault();
e.stopImmediatePropagation();
cache.get(e.requestURL).then(e.respondWith.bind(e))

But this still isn't right! In the original form, if the cache promise rejects (because there's nothing in the cache for that URL), the fetch will immediately fail with a network error. In our latest code, a rejection will just do nothing, and so the fetch will spin until timeout. (This doesn't have severely negative effects, but it does mean that you'll see a loading spinner in your tab for much longer than you should.)

To fix it, you need to signal that you're done dealing with the response in the error case too:

e.preventDefault();
e.stopImmediatePropagation();
cache.get(e.requestURL).then(e.respondWith.bind(e), e.respondWith.bind(e))

(This'll send whatever the failure value is to respondWith(). Since it's not a Response, the SW will treat this as a network error.)

Our code is now equivalent in functionality. Let's compare it again to the original code:

e.respondWith(cache.get(e.requestURL));

Considering this is pretty much the simplest-possible async-response case, and this or something very close to it will be very common, the increase in complexity is rather large and frankly untenable. Failing to do everything right will result in bad behavior sometimes (the promise resolution will race with other fetch handlers (microtasks preempt queued events!); cache failures will create long-running spinners; etc).

(Note that some smallish changes can make it less horrible. We can add a shorthand to the event to claim handling, and assume that .finally() is on promises, so we'd write it as:

e.stop();
cache.get(e.responseURL).finally(e.respondWith.bind(e));

But this is still a significant increase in complexity, and not the most obvious way to write it.)

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #10 from Boris Zbarsky bzbarsky@mit.edu ---

That all explains why passing Promise<Response> is a good idea. Why is passing

a Response a good idea? And why would automatically wrapping it in a Promise be a bad idea?

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #11 from Tab Atkins Jr. jackalmage@gmail.com --- (In reply to Boris Zbarsky from comment #10)

That all explains why passing Promise<Response> is a good idea. Why is passing a Response a good idea? And why would automatically wrapping it in a Promise be a bad idea?

Because that's what you're actually doing! You respond with a Response. That's the semantics you want; we only allow Promise<Response> because some of

the APIs naturally return that, and it's more convenient as I explained.

Requiring a Promise<Response> means extra boilerplate for the author to cast it

into a Promise. Auto-wrapping means consing up a Promise that'll immediately get unwrapped and thrown away, since it'll automatically unwrap (and risking triggering thenable coercion, too). It seems silly.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #12 from Cameron McCormack cam@mcc.id.au --- (In reply to Tab Atkins Jr. from comment #11)

Requiring a Promise<Response> means extra boilerplate for the author to cast it into a Promise. Auto-wrapping means consing up a Promise that'll immediately get unwrapped and thrown away, since it'll automatically unwrap (and risking triggering thenable coercion, too). It seems silly.

I still don't understand why auto-wrapping is bad. Sure, you get an extra Promise object to wrap the plain Response object, but that makes it simpler to define respondWith in the spec. You don't need to handle two cases, just one.

I also don't understand how the presence of auto-wrapping and lack of promises-in-union-types has to do with the fact that you can't invert:

e.respondWith(cache.get(e.requestURL))

to:

cache.get(e.requestURL).then(e.respondWith.bind(e))

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #13 from Domenic Denicola domenic@domenicdenicola.com ---

Yes, I would be curious what the observable difference between not-wrapping versus wrapping would be. Presumably if you did not-wrapping, you'd be doing the if (Type(argument) is Object) { var then = argument.then; if (typeof then === "function") ... dance manually.

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #14 from Anne annevk@annevk.nl ---

Auto-wrapping is not a problem for respondWith as far as I know. The operation is asynchronous either way. For things that take a promise we should indeed just use Promise.cast().

# bugzilla at jessica.w3.org (3 years ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #15 from Cameron McCormack cam@mcc.id.au ---

For the initial spec text I've required auto-wrapping and disallowed promise types from being in unions.

heycam/webidl/commit/61403d1705384ed4bb9e15fd4dda319b96faf08a

# bugzilla at jessica.w3.org (a day ago)

www.w3.org/Bugs/Public/show_bug.cgi?id=24291

Domenic Denicola d@domenic.me changed:

       What    |Removed                     |Added

         Status|NEW                         |RESOLVED
     Resolution|---                         |FIXED

Want more features?

Request early access to our private beta of readable email premium.