990043 - Consider adding a warning along the lines of the Promise.jsm "Cannot resolve a promise with an async function." warning

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect, P5)

Description

[Boris Zbarsky [:bzbarsky]]

Paolo, what sort of situations is that warning trying to catch?

Comment 1

[:Paolo Amadini]

Briefly, misusing "Task.async" instead of "Task.spawn".

See bug 966182 comment 10 for a full explanation.

Comment 2

[Boris Zbarsky [:bzbarsky]]

That helps some, but what is it about the return value of Task.async that makes it not suitable for passing to Promise.resolve, exactly?  And is it specific to return values of Task.async, or can it come up in general in other contexts?

Comment 3

[:Paolo Amadini]

(In reply to Boris Zbarsky [:bz] from comment #2)
> That helps some, but what is it about the return value of Task.async that
> makes it not suitable for passing to Promise.resolve, exactly?

You want to pass to Promise.resolve the invocation of the result of Task.async, not the result of Task.async.

If the async function itself is propagated as the resolution value, and a Promise handler invokes it, I guess that 99.9% of the times that indicates a mistake fixed by putting an invocation in the wrong place, and not really an API design that intended to return an async function instead of a Promise.

(In the unlikely case that that providing an async function is the desired behavior, you can just resolve to an object with an async function as one of its method.)

> And is it
> specific to return values of Task.async, or can it come up in general in
> other contexts?

Mostly specific to the result of Task.async.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> You want to pass to Promise.resolve the invocation of the result of Task.async, not the
> result of Task.async.

Ah, I see.  So the point is that Task.async is a Promise-returning function, and resolving a Promise with such a function is a bit odd.

We could in fact add warning to DOM promises for that sort of thing in the case when the function in question is declared in WebIDL, since then we can tell that it returns a Promise.

Anne, Domenic, does that seems reasonable?  Or are there use cases for resolving a Promise with such a function that make such a warning undesirable?

Comment 5

[Domenic Denicola]

Resolving promises with functions is common (functions can be constructed asynchronously, or the choice between different functions can depend on information made available only asynchronously) and I don't really see why you would warn when doing that. That would be tantamount to warning when a synchronous function has a function as its return value.

Comment 6

[Boris Zbarsky [:bzbarsky]]

This is not about resolving a promise with a function.  It's about resolving a promise with a function that itself resolves a promise.  It's possible that you'd do that, I guess, instead of just building the async choice of which thing to call into a single promise...

Comment 7

[Domenic Denicola]

Yes, that seems analogous to a synchronous function returning another synchronous function instead of just building the sync choice of which thing to call into a single function.

Comment 8

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 9

[Mark Banner (:standard8)]

Task.jsm has gone away, and we're working towards removing Promise.jsm (Bug 1378173), so I think this is a wontfix.