Smarter type checking for Promise.all

[tmkn]

TypeScript Version: 3.0.0-dev.201xxxxx

Search Terms:
promise.all

Code
First off, idk if this can be considered a bug or an improvement.
The code is legit but it's probably not what you intended to write or what you would expect to look at when you have to debug JS code.

Basically the code wanted to await a bunch of Promises via Promise.all but the Promises array was accidentally nested:

let delay = (delay: number) => {
    return new Promise(resolve => {
        window.setTimeout(() => {
            console.log("done: ", delay)
            resolve();
        }, delay);
    })
}

let delays = [1000, 2000];

(async () => {
    console.log("start");
    await Promise.all([delays.map(ms => delay(ms))]);//accidentally nested promises array
    console.log("end");
})();

Expected behavior:
"start"
"done: 1000"
"done: 2000"
"end"

Actual behavior:
"start"
"end"
"done: 1000"
"done: 2000"
which is of course correct but probably not what you intended to write.

It would be nice if TypeScript could pick this up and report it.

Playground Link:
http://www.typescriptlang.org/play/index.html#src=let%20delay%20%3D%20(delay%3A%20number)%20%3D%3E%20%7B%0D%0A%20%20%20%20return%20new%20Promise(resolve%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20window.setTimeout(()%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20console.log(%22done%3A%20%22%2C%20delay)%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20resolve()%3B%0D%0A%20%20%20%20%20%20%20%20%7D%2C%20delay)%3B%0D%0A%20%20%20%20%7D)%0D%0A%7D%0D%0A%0D%0Alet%20delays%20%3D%20%5B1000%2C%202000%5D%3B%0D%0A%0D%0A(async%20()%20%3D%3E%20%7B%0D%0A%20%20%20%20console.log(%22start%22)%3B%0D%0A%20%20%20%20await%20Promise.all(%5Bdelays.map(ms%20%3D%3E%20delay(ms))%5D)%3B%2F%2Faccidentally%20nested%20promises%20array%0D%0A%20%20%20%20console.log(%22end%22)%3B%0D%0A%7D)()%3B

Related Issues:

[jcready]

Perhaps a warning when calling Promise.all() with an array with only a single item and offer a code fix:

 (async () => {
     console.log("start");
-    await Promise.all([delays.map(ms => delay(ms))]);
+    await delays.map(ms => delay(ms));
     console.log("end");
 })();

Then perhaps another warning and code fix when awaiting an array:

 (async () => {
     console.log("start");
-    await delays.map(ms => delay(ms));
+    await Promise.all(delays.map(ms => delay(ms)));
     console.log("end");
 })();

[mhegazy]

This goes to the definition of Promise.all that allows a non-promise values. This is much like #8310, where awaited values are not promises. unfortunately in both issues, the code is legal ES2016, and some libraries depend on these behaviors by allowing mixing of Promise and non-Promise values in the same call to all.
As with the #8310, my recommendation here is to use a tslint rule like https://palantir.github.io/tslint/rules/promise-function-async/ for Promise.all arguments with a fix that @jcready suggested earlier.

[tmkn]

Yes, that's also why I initially hesitated to create this issue, since it's correct JavaScript and there might not be a solution to it but TypeScript provides such a nice dev experience that it is the first thing I install and listen to when I have to debug foreign JavaScript and unfortunately it couldn't help me in this case.

I still feel TypeScript should report this, in line with other JavaScript pitfalls that TypeScript reports but I get that it's a very niche error and sometimes it's even deliberately done (none promises in Promise.all), so it's not clear when to report and when not to. Anyway I just wanted to mention it and gather other peoples opinions since it can happen in the wild.

[typescript-bot]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.