Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise<Promise<T>> cannot exist in JS #27711

Closed
delautour opened this issue Oct 11, 2018 · 18 comments · Fixed by #35998 or #45350
Closed

Promise<Promise<T>> cannot exist in JS #27711

delautour opened this issue Oct 11, 2018 · 18 comments · Fixed by #35998 or #45350
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@delautour
Copy link

TypeScript Version: Version 3.2.0-dev.20181011

Search Terms:
is:open label:Bug promise
label:Bug wrapped promise
label:Bug wrapped nested

Code

const p1 = new Promise<Promise<Number>>((resolveOuter) => {
    const innerPromise = new Promise<Number>((resolveInner) => {
        console.log('Resolving inner promise')
        resolveInner(1)
    })

    console.log('Resolving outer promise')
    resolveOuter(innerPromise)
})

p1.then((p2: Promise<Number>) => {
    p2.then((num) => 
        console.log('the number is: ', num)
    )
})

Expected behavior:
Compilation should fail, because p1 is actually a Promise<number> due to promise unwrapping.
Actual behavior:
Compilation should fail, requiring code which looks like:


const p1 = new Promise<Number>((resolveOuter) => {
    const innerPromise = new Promise<Number>((resolveInner) => {
        console.log('Resolving inner promise')
        resolveInner(1)
    })

    console.log('Resolving outer promise')
    resolveOuter(innerPromise)
})

p1.then((p2: Number) => {
    console.log('the number is: ', p2)
})

Playground Link:
Runtime error
No runtime error

Related Issues:
Didn't find related issue

@DanielRosenwasser
Copy link
Member

@rbuckton do we have an issue tracking this?

All I have is #17077 and the design notes at #17621, #18155, #19169, #20724.

@weswigham weswigham added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Oct 16, 2018
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Oct 25, 2018

Seems like you want something like negated types (#18280 or #4196).

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 25, 2018
@the1mills
Copy link

Seems like a legit bug, but as a side note, a little strange to resolve a promise from a promise executor callback. Promise executor should basically be reduced down to the lowest level that isn't already promisified. If you already have a promise in the executor, then you could just make that your promise call, instead of creating a wrapper promise around a promise.

@hcomnetworkers
Copy link

hcomnetworkers commented Mar 22, 2019

We have the same problem. We wrap unknown return types into an additional Promise and can get stuck with a resolved type of Promise<Promise<T>> if the original type was a Promise.

This nested Promise however is problematic:

async function xxx(x: Promise<Promise<string>>): Promise<void> {
  x.then((y) => {
    y.match(/foo/); //invalid
  });

  const z = await x;
  z.match(/foo/); //valid
}

Here, y is wrongfully seen as a Promise<string> while z is correctly seen as string.

Why does await and then behave differently?

Also why can't we assign a Promise<Promise<T>> to a Promise<T> variable? This is valid JS since nested Promises always unwrap.

@ORESoftware
Copy link

ORESoftware commented Mar 23, 2019

@hcomnetworkers Promise<Promise<T>> can exist, it looks like this

const resolvesToPromise = function(){
  return new Promise(r => r(new Promise(...));
}

it doesn't get unwrapped in this case since the executor resolves synchronously

@hcomnetworkers
Copy link

hcomnetworkers commented Mar 26, 2019

@ORESoftware It does not matter how often you nest a Promise, the result is the same:

const prom = new Promise((r) => r(new Promise((r2) => r2(42))));
prom.then((x) => console.log(x)); //prints 42
console.log(await prom); //prints 42

This is just how Promises work. You simply cannot get the "inner" Promise, it's gone, flattened.

I'm not saying that the type Promise<Promise<T>> is wrong, it might just be the result of a function, wrapping something into a Promise. However, TypeScript should always collapse nested Promises when type checking and compiling because that is what JavaScript does.

Promise<T> === Promise<Promise<T>> === Promise<Promise<Promise<T>>> //...

@Kinrany
Copy link

Kinrany commented Mar 26, 2019

The Promise type seems broken:

playground

const a = Promise.resolve()
    .then<Promise<void>, never>(() => Promise.resolve());

// b is undefined, but its type is Promise<void>
a.then(b => console.log(b));

@ORESoftware
Copy link

@hcomnetworkers you're right, never would have guessed that wrt to the promise executor

@Velenir
Copy link

Velenir commented May 9, 2019

I encountered this error when writing a higher-order wrapping function

async function asf(s: string) {
        return s
    }

const wrap = <T extends (...args: any[]) => any>(f: T) => {
    return async function(this: any, ...args: Parameters<T>) {
        const r: ReturnType<T> = f.apply(this, args)
        // do something with r
        return r
    }
}

const wrappedFunc = rest(asf) // (this: any, s: string) => Promise<Promise<string>>
const ret = wrappedFunc('string') // TS gives Promise<Promise<string>> here

I have to do

return async function(this: any, ...args: Parameters<T>) {
        const r: ReturnType<T> = f.apply(this, args)
        return r
    } as T

to get ret typed Promise<string>, but I'd rather not use as

@jstevans
Copy link

jstevans commented Sep 10, 2019

@DanielRosenwasser @weswigham @rbuckton would this suffice in lib.es2015.promise.d.ts? It's a bit unintuitive (the type of new Promise<Promise<number>>() would actually evaluate to Promise<number>), but I believe this provides the desired compile-time error.

Playground link with some additional testing/assertions 🙂

export type FlattenedPromise<T> = 
    unknown extends T 
      ? Promise<T> 
      : T extends Promise<infer U> 
        ? T
        : Promise<T>;

interface PromiseConstructor {
// <snip>
    new <T>(executor: (resolve: (value?: T | PromiseLike<T>) => void, reject: (reason?: any) => void) => void): FlattenedPromise<T>;
// <snip>
}

@ExE-Boss
Copy link
Contributor

#35998 (comment) will make Promise<Promise<T>> assignable to Promise<T>:

This last commit adds the capability to measure a type parameter to determine whether it is awaited in a then method, and if so unwraps the awaited during assignability checks between two identical type references. What this means is that when comparing assignability between two Promise instantiations, the type argument of each promise is first unwrapped before assignability is compared, so Promise<awaited T> is assignable to Promise<T>, and Promise<Promise<Promise<T>>> is now also assignable to Promise<T>.

@rbuckton
Copy link
Member

We are pushing awaited until after TS3.9 while we continue to assess the impact of the change. As a result, I am reopening this issue until we have a solution in place.

@rbuckton rbuckton reopened this Mar 25, 2020
@millsp
Copy link
Contributor

millsp commented May 21, 2020

The solution would be for Promise not to re-wrap if it's already a Promise. That way, we can never end up with nested Promises. Here's a little proof of concept:

export type Promise<A extends any> =
    globalThis.Promise<
        A extends globalThis.Promise<infer X>
        ? X
        : A
    >

@millsp
Copy link
Contributor

millsp commented May 21, 2020

So now we can do:

type t0 = Promise<1>                    // Promise<1>
type t1 = Promise<Promise<1>>           // Promise<1>
type t2 = Promise<Promise<Promise<1>>>  // Promise<1>

@yume-chan
Copy link
Contributor

awaited keyword was in the 4.0 iteration plan (#38510), but 4.0 just released without it.

I can't find any more discussion about it. It's not in the 4.1 iteration plan (#40124), nor any Design Notes.

I found it being mentioned in #40002, but that pr didn't actually add the Awaited type into libs right?

What's the current state of this issue?

@DanielRosenwasser
Copy link
Member

#40006 mentions it, and #40002 mentions it. @ahejlsberg @RyanCavanaugh we should determine whether the Awaited type alias makes sense for 4.1.

jgehrcke added a commit to opstrace/opstrace that referenced this issue Dec 17, 2020
Suspicious:

    return axios.request(deleteRequest);

Especially being called towards the _end of
the program_ this might result in the
HTTP response not being waited for at
all.

If it is waited for/received then, depending on
other weaknesses in the program, it's unclear
which part of the program is supposed to
catch and handle an unexpected HTTP response.

Root problem: forgetting to put the `await`
keyword where it has to be put.

Carefully putting the `await` keyword where
it has to be put is on the developer --
tooling rarely helps. See

eslint/eslint#13567

When missing an `await` keyword then
the value propagating through code is
often not what we think it is. With a
weak return type signature such as
`Promise<any>` the _actual_ return value
might in fact be a Promise to a Promise.
This is dangerous territory; I was peeking
into

microsoft/TypeScript#27711

two things are clear from this issue:

- TypeScript is in flux w.r.t. situations
  involving things like Promise<Promise<T>>

- I don't even want to know about this topic

The Delete() method is called here:
https://github.com/opstrace/opstrace/blob/f610692c86d0da1d16165c89c22d814594940523/lib/dns/src/index.ts#L208
via redux-saga. Redux-saga has its own
fancy interaction with async functions,
and its own error handling paradigms.

I assume that as of the missing `await` and
the weak return type signature the Delete()
method might in fact have been subject to a
race condition, leaving a "dangling thing"
behind with no proper error handling mechanism
looking at that thing anymore after submission.

That is, it's likely that sometimes the program
either didn't wait for receiving the HTTP response
or didn't have an error handler taking action
when a bad response was received.

Signed-off-by: Dr. Jan-Philip Gehrcke <jp@opstrace.com>
jgehrcke added a commit to opstrace/opstrace that referenced this issue Dec 17, 2020
Suspicious:

    return axios.request(deleteRequest);

Especially being called towards the _end of
the program_ this might result in the
HTTP response not being waited for at
all.

If it is waited for/received then, depending on
other weaknesses in the program, it's unclear
which part of the program is supposed to
catch and handle an unexpected HTTP response.

Root problem: forgetting to put the `await`
keyword where it has to be put.

Carefully putting the `await` keyword where
it has to be put is on the developer --
tooling rarely helps. See

eslint/eslint#13567

When missing an `await` keyword then
the value propagating through code is
often not what we think it is. With a
weak return type signature such as
`Promise<any>` the _actual_ return value
might in fact be a Promise to a Promise.
This is dangerous territory; I was peeking
into

microsoft/TypeScript#27711

two things are clear from this issue:

- TypeScript is in flux w.r.t. situations
  involving things like Promise<Promise<T>>

- I don't even want to know about this topic

The Delete() method is called here:
https://github.com/opstrace/opstrace/blob/f610692c86d0da1d16165c89c22d814594940523/lib/dns/src/index.ts#L208
via redux-saga. Redux-saga has its own
fancy interaction with async functions,
and its own error handling paradigms.

I assume that as of the missing `await` and
the weak return type signature the Delete()
method might in fact have been subject to a
race condition, leaving a "dangling thing"
behind with no proper error handling mechanism
looking at that thing anymore after submission.

That is, it's likely that sometimes the program
either didn't wait for receiving the HTTP response
or didn't have an error handler taking action
when a bad response was received.

Signed-off-by: Dr. Jan-Philip Gehrcke <jp@opstrace.com>
@danielo515
Copy link

So what is the status of this? It is a real deal-braker for function composition. You can't do any composition of functions that returns promises because you end with Promise<Promise<Promise<T>>> which doesn't make any sense in JS

@timoxley
Copy link

timoxley commented May 4, 2021

Is there any generic workaround for this, even an ugly one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment