facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.07k stars 1.85k forks source link

Type inference of Promise.all breaks in 0.55 with generic union types #4936

Closed noppa closed 6 months ago

noppa commented 6 years ago

After upgrading from 0.54 to 0.55, our Promise.all calls started giving errors.

class A {}
class B {}

declare var cond: boolean;
declare var promiseA: Promise<A>;
declare var promiseB: Promise<B>;

Promise.all([cond ? promiseA : promiseB])
  .then((results: [A|B]) => {
  });

flowtype.org/try

The above works using version 0.54 but gives errors in 0.55:

9:   .then((results: [A|B]) => {
                      ^ A. This type is incompatible with the expected param type of
[LIB] static/v0.55.0/flowlib/core.js:611:     static all<Elem, T:Iterable<Elem>>(promises: T): Promise<$TupleMap<T, typeof $await>>;
                                                                                                       ^ B
9:   .then((results: [A|B]) => {
                        ^ B. This type is incompatible with the expected param type of
[LIB] static/v0.55.0/flowlib/core.js:611:     static all<Elem, T:Iterable<Elem>>(promises: T): Promise<$TupleMap<T, typeof $await>>;
                                                                                                       ^ A

I'm not quite sure if this is a bug or an unfortunate side-effect of a new feature, but I think the issue might be related to $TupleMap and the way generic types are "combined".

This is the most minimal example I can think of that works in 0.54 but gives errors in 0.55:


class A {}
class B {}

class Container<+T> {
  +val: T;
}

declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;

(tuple: [Container<A|B>]); // Error in 0.55:
/*
9: (tuple: [Container<A|B>]);
            ^ Container. This type is incompatible with
8: declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;
                      ^ union: type application of class `Container`(s)
Member 1:
8: declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;
                      ^ type application of class `Container`Error:
9: (tuple: [Container<A|B>]);
                        ^ B. This type is incompatible with
8: declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;
                                           ^ A
Member 2:
8: declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;
                      ^ type application of class `Container`Error:
9: (tuple: [Container<A|B>]);
                      ^ A. This type is incompatible with
8: declare var tuple: $TupleMap<[Container<A>|Container<B>], <T>(T) => T>;
                                                        ^ B
*/

flowtype.org/try

In the above, replacing (tuple: [Container<A|B>]) with (tuple: [Container<A>|Container<B>]) works in both versions, but if I have them both

(tuple: [Container<A>|Container<B>]);
(tuple: [Container<A|B>]);

Version 0.54 gives errors too (as expected, I guess):

10: (tuple: [Container<A|B>]);
             ^ Container. This type is incompatible with
9: (tuple: [Container<A>|Container<B>]);
            ^ union: type application of polymorphic type: class type: Container(s)

The Promise.all example also works if I explicitly mark the source promise as Promise<A|B> instead of the inferred type Promise<A>|Promise<B>.

const promises: [Promise<A|B>] = [cond ? promiseA : promiseB];
Promise.all(promises)
  .then((results: [A|B]) => {
  });

Which is ok, but it would be much nicer if the types could be inferred instead.

lll000111 commented 6 years ago

Is this related to https://github.com/facebook/flow/issues/4923

SamChou19815 commented 6 months ago

The issue seems to be fixed a long time ago