Flow: Object construction is wrongly inferred to return `any` when constructing a class with a Promise-using method

Created on 14 Aug 2018  Â·  1Comment  Â·  Source: facebook/flow

In the following code, the type of new Engine() is wrongly inferred to be any instead of Engine:

~~~javascript
// @flow

class Engine {
constructor() {}

pipe(aPromise: Promise): Engine {
aPromise.then(() => this);
return this;
}
}

const anEngine = new Engine(); // bug: anEngine has type any instead of Engine
~~~

Try Flow demo

You can reproduce this on the Try Flow website by moving the cursor onto the word anEngine, then by looking at the left side of the bar at the bottom of the window:

Demonstration of the Try Flow feature of inspecting the type at a point

It seems that only a combination of very specific factors causes this bug. The following properties of the above code are necessary to reproduce this; the linked Try Flow demos show that removing that particular factor avoids this bug:

  • Overriding the default constructor – demo. This is strange because the overridden constructor is identical to a JavaScript class’s default constructor.
  • Returning this from the pipe method – demo
  • Calling .then on the passed-in aPromise – demo
  • Making the callback passed to aPromise.then _return_ this, rather than referencing this and returning nothing – demo
  • Using a Promise instead of a synchronous function – demo

This problem was encountered in the wild while solving this Stack Overflow question. The above test case was reduced from the code in that question.

bug

Most helpful comment

Lovely: a new, terrifying incarnation of #6400.

As in that issue, the regression was introduced in Flow v0.54.0 (last
good version: v0.53.1).

Like Array.from in that issue, Promise.then has a bunch of
overloads.

As in that issue, explicitly annotating that the constructor returns
void fixes the bug.

Due to all these similarities, I’m inclined to think that this is a new
manifestation of #6400. That’s not to say that it’s not important or
disturbing—it is both.

Thanks for sharing the list of necessary factors. That’s always
interesting to read.

In production, we annotate constructors to have void return type in
order to avoid this issue. I had to patch in another one of these as
recently as today (https://github.com/sourcecred/sourcecred/pull/661).
Until this is fixed, you might want to do the same.

(cc @decentralion; you’ll want to be aware of this.)

>All comments

Lovely: a new, terrifying incarnation of #6400.

As in that issue, the regression was introduced in Flow v0.54.0 (last
good version: v0.53.1).

Like Array.from in that issue, Promise.then has a bunch of
overloads.

As in that issue, explicitly annotating that the constructor returns
void fixes the bug.

Due to all these similarities, I’m inclined to think that this is a new
manifestation of #6400. That’s not to say that it’s not important or
disturbing—it is both.

Thanks for sharing the list of necessary factors. That’s always
interesting to read.

In production, we annotate constructors to have void return type in
order to avoid this issue. I had to patch in another one of these as
recently as today (https://github.com/sourcecred/sourcecred/pull/661).
Until this is fixed, you might want to do the same.

(cc @decentralion; you’ll want to be aware of this.)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mjj2000 picture mjj2000  Â·  3Comments

bennoleslie picture bennoleslie  Â·  3Comments

ghost picture ghost  Â·  3Comments

davidpelaez picture davidpelaez  Â·  3Comments

Beingbook picture Beingbook  Â·  3Comments