Duplicate of bug 21422?

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

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

Yes.

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

https://github.com/domenic/promises-unwrapping/issues/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.

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]: https://github.com/domenic/promises-unwrapping/issues/79#issuecomment-31080608

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.

2. 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.

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?

(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.)

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?

(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.

(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))

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.

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().

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

https://github.com/heycam/webidl/commit/61403d1705384ed4bb9e15fd4dda319b96faf08a