Flow: interfaces can no longer have constructors

Created on 28 Oct 2017  路  10Comments  路  Source: facebook/flow

The following worked as recently as 0.54.1, but does not work in 0.55 and later:

interface I {
    constructor(): I;
}

class Impl implements I {
    constructor() {}
}

In 0.55, the error is:

2:      constructor(): I;
                     ^ property `constructor` of I. Property not found in possibly undefined value
6:      constructor() {}
                   ^ undefined

In 0.57.3, the error is:

5: class Impl implements I {
         ^ Impl. This type is incompatible with
5: class Impl implements I {
                         ^ I
Property `constructor` is incompatible:
6:      constructor() {}      ^ function. This type is incompatible with
2:      constructor(): I;
      ^ function type
This parameter is incompatible:
6:      constructor() {}
                   ^ undefined. This type is incompatible with
2:      constructor(): I;
                     ^ I
Property `constructor` is incompatible:
2:      constructor(): I;
                     ^ property `constructor` of I. Property not found in possibly undefined value
6:      constructor() {}
                   ^ undefined

If I add return this to the Impl constructor body, flow accepts the code. It appears that flow forgot that the constructor is special, and the lack of an explicit return on all control flow paths does not mean that it can return undefined.

Most helpful comment

I'd like to add the solution to the problem I had here:

When I change the constructor definition to a return type of void it works for me without getting any flow errors on flow 0.64:

interface I {
    constructor(s: string, n: number): void;
}

All 10 comments

Semantically constructors doesn't make sense in interfaces. Their type always the same.

I'm not sure what you mean by that. Defining a required signature for interface constructors can be quite useful. Consider:

interface I {
    constructor(s: string, n: number): I;
}

class ImplA implements I {
    constructor(n: number, s: string) {}
}

class ImplB implements I {
    constructor(s: string, n: number) {}
}

const registry: {[string]: Class<I>} = {
  a: ImplA,
  b: ImplB
};

new registry.a();

Here, type checking can alert you that ImplA's constructor signature is wrong, and that arguments are missing from new registry.a().

Ah, okay. My bad.

Forget about the newable-class case for a moment and consider the callable-constructor case, e.g.

interface I {
  constructor(): I;
}

function f(): I {
  return {
    constructor() {
      return f();
    }
  };
}

let i: I = f();

Since class constructors are not callable (without new), I believe that an interface with a constructor method should never accept a class.

I thought that I had seen a new () signature somewhere (probably TS), and a quick web search leads me to https://stackoverflow.com/questions/39622778/what-is-new-in-typescript. Maybe Flow will add an analogous syntax someday? I see little point since Flow doesn't impose subtyping on subclass constructors. For instance, Flow would mistakenly allow the following to pass without errors:

interface I {
  new (string): I // Flow accepts this, but it interprets it as a method named
                  // `new`. Under TS this indicates a newable constructor.
}

class A {
  s: string;
  constructor(s: string) { this.s = s; }
}

class B extends A {
  constructor(n: number) { super("B"); }
}

function fn(i: I): I {
  return new i.constructor("I");
}

let b: B = new B(1);
let a: A = b;
let i: I = fn(a); // Whoops. Flow lets me sneak an instance of B into `fn`,
                  // where an incompatible constructor call passes without error.

@popham I think that's the difference between I and Class<I>: yes, in the type I, constructor is just a regular property, as you suggest. But in the type Class<I>, it's a constructor. Similarly, for a class that implements I, there's no ambiguity: constructor should be treated as a constructor.

Oh, I think I understand what you're saying: you're suggesting that flow should not even allow the type Class<I>, because of the following:

interface I {
  constructor(): I;
}

declare function f(): I;
declare function k(): Class<I>;

let a1: I = f().constructor(); // ok
let a2: I = new (f())(); // flow error (expected)

let b1: I = (k())(); // flow error (expected)
let b2: I = new (k())(); // ok

b2.constructor(); // no flow error, but must fail at runtime with "Class constructor cannot be invoked without 'new'"

I'm not sure, but it sounds like maybe you're trying to nail down Flow's semantics. I'm speaking to what they ought to be.

I disagree with your "must fail." There exists a pre-ES6 pattern for silently handling cases where a user forgets new while instantiating a class, e.g.

function Fubar (foo, bar) {
  if (this instanceof Fubar) {
    this._foo = foo;
    this._bar = bar;
  } else {
    return new Fubar(foo, bar);
  }
}

I believe that constructor should define the callable case always. I believe that a separate new signature like TS's should be introduced to handle your use case.

Speaking to your use case, I don't see much value in imposing a constructor signature. Class annotations impose a constructor signature just fine. Javascript diverges from Java in that the constructor property provides access to an instance's class-statics and its constructor. To me, the only legitimate basis for providing a new signature would be to facilitate constructing new instances from an existing instance, e.g.

interface I {
  new (string): I
}
function fn (i: I): I {
  return new i.constructor("another instance");
}

As I demonstrated earlier, however, the lack of class constructor subtyping allows buggy code to sneak through. I consider this problem sufficient reason to leave a new signature feature unimplemented.

I don't see much value in imposing a constructor signature.

Per my earlier exampke, it's useful for a registry-based factory pattern, where new acts as the factory function, and {[string]: Class<I>} serves as a registry such that various implementations can be registered by a string identifier -- typically the class name. I find that this pattern often comes up in domains where data-driven polymorphism is necessary, such as object deserialization and interpreters.

I can of course use a regular function type as the factory type, but it requires more boilerplate. You can't register the class directly:

const registry: {[string]: Class<I>} = {};
function register(klass: Class<I>) { registry[klass.name] = klass; }
register(class Foo implements I { ... });

But instead have to provide the name and a wrapper function separately:

const registry: {[string]: () => I} = {};
function register(name: string, constructor: () => I) { registry[name] = constructor; }

class Foo implements I { ... }
register(Foo.name, () => new Foo());

The soundness issue you raise is interesting, but seems orthogonal to interfaces. Interfaces aren't necessary to your example. It could just as well use the base class instead:

class A {
  s: string;
  constructor(s: string) { this.s = s; }
}

class B extends A {
  constructor(n: number) { super("B"); }
}

function fn(a: A): A {
  return new a.constructor("I");
}

let b: B = new B(1);
let a: A = b;
let i: A = fn(a); // Whoops!

I understand that you've got an existing architecture and you don't want to screw users by shifting your interface. That said, I believe that classes and interfaces should satisfy the following invariant: If class A implements interface I and class B extends class A, then class B implements interface I. Based on this invariant, the following could pass Flow while failing at run-time:

register(class OhGeez extends registry.a {
  x: string;
  constructor(o: {x: string}) {
    super("", 1);
    this.x = o.x;
  }
});
let og = new registry.OhGeez("", 0);
//...

Until (or if) the class constructor subtyping problem is handled, permitting constructors on interfaces invites others to implement similarly flawed code, adding to the universe of broken type checks introduced by a fix and to the universe of run-time errors.

See https://github.com/facebook/flow/issues/803#issuecomment-317205728 for the targetted semantics of interfaces.

I'd like to add the solution to the problem I had here:

When I change the constructor definition to a return type of void it works for me without getting any flow errors on flow 0.64:

interface I {
    constructor(s: string, n: number): void;
}
Was this page helpful?
0 / 5 - 0 ratings