What is the best way to express an inner class?

[Ailrun]

I'm working with openlayers typing. There is following inner class in code of openlayers.

// ol.Collection class
ol.Collection = function (/* ... parameters ... */) {
  // ... class construction codes ...
}
// ... some methods

// ol.Collection.Error class
ol.Collection.Error = function (/* ... parameters ... */) {
  // ... class construction codes ...
}

I try some typescript-like, C++-like, ... methods, (inner module, static inner class, ....) but no one works. For example,

declare module 'ol' {
  declare module 'Collection' {
    declare class Error {
      // ....
    }
  }
}

is rejected by flow.

Is there any clean choice for this case? If it is, could you give me some direction for typing this one?

[popham]

Error is a static member of class Collection, yeah? Try

declare module 'ol' {
  declare class _Error { /*...*/ }
  declare class Collection {
    static Error: Class<_Error>;
    constructor( /*...*/ ): void;
    // ... some methods
  }
}

[Ailrun]

@popham At that case, does flow allow to use new keyword on ol.Collection.Error?

[popham]

Yup. Just beware that many pre-ES6 libraries don't implement ES6's class behavior. In particular I'm thinking of the prototype chaining on statics that ES6 provides with its classes (if your library "extends" Collection, then the library may not propagate the Error static member to the extension classes, while the ES6 semantics of Flow will expose the Error static member on extension classes).

[popham]

Whoops. I just fixed a goof in my earlier code by wrapping a Class<.> around _Error at the static Error field's annotation.

[Ailrun]

@popham Thx!

[popham]

Sure. By the way, the give-away for pre-ES6 libraries that do implement ES6 semantics is Subclass.__proto__ = Superclass for a Subclass that extends Superclass (in addition to the normal prototype chaining).

[Ailrun]

@popham What I notice is they (openlayers) use Collection as a namespace also, not just a class.

For more detail,

ol.Collection class extends (of course they use their own implementation, not ES6 class) ol.Object class, and ol.Object also has ol.Object.Error. The problem in this case is ol.Collection.Error doesn't extends ol.Object.Error and both Error Classes just have same parent class.

As a code,

ol.Collection = function(/* ... */) {
  // ...
}
// Openlayers' implementation for inheritance
ol.inherits(ol.Collection, ol.Object);

ol.Collection.Error = function(/* ... */) {
  // ...
}
// ol.error.Error is the root Error class.
ol.inherits(ol.Collection.Error, ol.error.Error);

ol.Object = function(/* ... */) {
  // ...
}

ol.Object.Error = function(/* ... */) {
  // ...
}
ol.inherits(ol.Object.Error, ol.error.Error);

Is there also a neat solution for this case?

[Ailrun]

In case of typescript, they allow nested module in module, and namespace merging,

so typing is simply

class Collection {
  // ...
}

module Collection {
  class Error {
    // ...
  }
}

[popham]

Confirmed that inherits doesn't chain statics: src/ol/index.js#L215-L218.

If I were you, I would rummage around in flow-typed and see how others have worked around the non-ES6 inheritance. I expect that many others have faced the same issue. Your problem may be unusual because you have non-inherited Errors, so Flow will complain about the non-subtype static field, e.g.

class E1 {}
class E2 {}
class A {
  static Error: Class<E1>
}
class B extends A {
  static Error: Class<E2> // no good because E2 is not a subtype of E1
}

I'm 90% sure that you can't model this accurately (intuition, plus I looked up the inherits from the node library file, lib/node.js#L1501, and there's nothing special happening there, and then I grepped the codebase for "inherits" and came up with only noise). If I'm correct, then you've got to choose among suboptimal solutions:

• ~Convince the openlayers maintainer(s) to model ES6 inheritance. Support for __proto__ was standardized for ES6, so this may break compatibility with older browsers that openlayers intends to support.~ This doesn't fix the Error subtyping within openlayers without more extensive revisions.
• Convince the Flow maintainers to explicitly support this kind of inheritance. To my mind there's a 10% chance that they already do, so be sure to use your manners. I've thought about this in the past and imagineered a __proto__ nonstatic field and static field on classes for use instead of extends. It works great in my fantasy world :).
• Model Collection without inheritance. Documentation is thin, but there's a mixins feature on declared classes (see the commit message and tests at https://github.com/facebook/flow/commit/b72d1c61487f9e95ddbe68c4f994d9699b7b71c9) that will spare you some typing and maintenance. The downside is that some instanceof refinements will break, e.g. Flow won't recognize the Collection instance x from x instanceof OpenLayersObject as an OpenLayersObject.
• Model Error without inheritance. I wouldn't be surprised to learn that the Error classes are identical and the inheritance was done for instanceof filtering of thrown errors. If losing support for instanceof filtering on errors is okay and the errors have identical shapes, then you could model Error as an interface.

If you find another solution, I would love to hear about it.

[Ailrun]

@popham I really appreciate to you for such a long, detail and kind answer.

I tried to find some similar issues on and typing codes those already in flow and flow-typed, but I couldn't find one (Maybe it's just a problem of my own intuition and searching/skimming skill), that's why I reopen this issue.

I will try yours and try to find more solutions. If anything found, of course I will happily share it on this issue.

[Ailrun]

@popham Using commonjs module feature of flow seem to solve the problem.

declare module 'define' {
  declare class A extends B {
    test: boolean;
  }
  declare class _A$C extends C {
    test: number;
  }

  declare class B {
  }  
  declare class _B$C extends C {
    test: number;
  }

  declare class C {
    test: number;
  }

  declare var exports: {
    A: Class<A> & {
      C: Class<_A$C>
    },
    B: Class<B> & {
      C: Class<_B$C>
    },
    C: Class<C>
  };
}

declare module 'test' {
  import type define from 'define';

  declare class InheritTest1C extends InheritTest1P {
    +x: define.A.C;
  }

  declare class InheritTest1P {
    +x: define.C
  }

  declare class InheritTest2C extends InheritTest2P {
    +x: define.A;
  }

  declare class InheritTest2P {
    +x: define.B;
  }
}

Of course it's bit annoying, but at least, it solve the problem without inheritance issue.

[popham]

Interesting, although I don't think that the module system is relevant. It looks like you've avoided the statics chaining by using the intersection operator. The C from Class<A> & { C: Class<_A$C> } is independent of A, so Flow doesn't enforce subtyping (I would call this a bug in Flow). The one drawback that occurs to me is that for an instance a of class A the C is inaccessible. A few modifications of my earlier stub demonstrates the problem:

class E1 {}
class E2 {}
class A {}
declare var FullA: Class<A> & { Error: Class<E1> };
class B extends A {}
declare var FullB: Class<B> & { Error: Class<E2> };

const a = new FullA;
const e1 = new a.constructor.Error; // Flow errors (I think this error is a bug in Flow)
const b = new FullB;
const e2 = new b.constructor.Error; // Flow errors (I think this error is a bug in Flow)

If Flow does eventually handle e1 and e2 properly, I expect that the fix will break your declarations. That said, this seems like a good solution. Hopefully Flow develops better support for non-ES6 inheritance before the helpful bug gets fixed.

[Ailrun]

@popham Ye, you're right. the point is not the commonjs one, but the intersection one. Thank you for clarifying.

Even with that bug, I think that one is the best choice for the problem, at least for just now... I will also try & find some more, and if there is any progress, I will post it!