facebook / flow

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

Can't use a class's type as an `extends` #3186

Open danfuzz opened 7 years ago

danfuzz commented 7 years ago

Context

I noticed that the flow-typed project has a module definition for express which includes a naked top-level import of { Server } from 'http' which AIUI is problematic. I figured I'd pitch in and try to pull the import into the declare module block, now that Flow supports that (as of v0.37). Doing this means pulling in some top-level declare classes as well, because those are where the import is used. The result is something like:

declare module `express` {
  declare class $Application {
    ...
  }
  ...

  declare module.exports: {
    (): express$Application, // If you try to call like a function, it will use this signature
    static: serveStatic, // `static` property on the function
    Router: typeof express$Router, // `Router` property on the function
  };
}

whereas originally there were global declarations along the lines of:

declare class express$Application extends express$Router mixins events$EventEmitter {
  ...
}

Handy reference: https://github.com/flowtype/flow-typed/tree/master/definitions/npm/express_v4.x.x

Problem

The status quo declaration of express supports extending its declared classes by virtue of the fact that those classes are declared in the global scope (with a prefix), such as the following snippet from the flow-typed test for the module:

declare class test_express$CustomApplication extends express$Application {
  constructor(expressConstructor: () => express$Application): this;
  ...
}

But once express$Application becomes $Application and is moved into the declare module block, there doesn't seem to be a way to refer to it such that it can be named as a class to extend.

Because it's not exported as a value, saying import { $Application } from 'express' causes an error along the lines of:

  2: import { $Application } from 'express';
              ^^^^^^^^^^^^ Named import from module `express`. This module has no named export called `$Application`.

But it's also not exported as a type, so saying import type { $Application } from 'express' causes pretty much the same error.

It looks like I can get the type to be exported by adding something like this to the module declaration: declare type $ApplicationType = $Application. If I do this, then I can import the type, but I can't actually use it in an extends clause in a class declaration:

  4: declare class Foo extends $ApplicationType { }
                               ^^^^^^^^^^^^^^^^ $ApplicationType. type referenced from value position
  2: import type { $ApplicationType } from 'express';
                   ^^^^^^^^^^^^^^^^ type $ApplicationType

Transcript

I have a branch of flow-typed with a new version of express as outlined above, at https://github.com/danfuzz/flow-typed/tree/express-on-v37. In this version (as of this writing), I have declare class $ApplicationClass ... in the module definition, which I try to import as a value in the tests.

When I run the flow-typed tests over it, here's the errors I get (with some duplication elided):

ERROR: express_v4.x.x/flow_v0.37.x-
 * express_v4.x.x/flow_v0.37.x- (flow-v0.37.1): Unexpected Flow errors(2):
   1-test_overrideUseMethodClassExtension.js:2
     2: import express, { $ApplicationClass, $RequestClass, $ResponseClass, Router } from 'express'
                          ^^^^^^^^^^^^^^^^^ Named import from module `express`. This module has no named export called `$ApplicationClass`.

   1-test_overrideUseMethodClassExtension.js:2
     2: import express, { $ApplicationClass, $RequestClass, $ResponseClass, Router } from 'express'
                                             ^^^^^^^^^^^^^ Named import from module `express`. This module has no named export called `$RequestClass`.

   1-test_overrideUseMethodClassExtension.js:2
     2: import express, { $ApplicationClass, $RequestClass, $ResponseClass, Router } from 'express'
                                                            ^^^^^^^^^^^^^^ Named import from module `express`. This module has no named export called `$ResponseClass`.

   1-test_overrideUseMethodClassExtension.js:57
    57:   // $ExpectError
          ^^^^^^^^^^^^^^^ Error suppressing comment. Unused suppression

   1-test_overrideUseMethodClassExtension.js:59
    59:   // $ExpectError
          ^^^^^^^^^^^^^^^ Error suppressing comment. Unused suppression

   1-test_overrideUseMethodClassExtension.js:63
    63: // $ExpectError
        ^^^^^^^^^^^^^^^ Error suppressing comment. Unused suppression

   Found 6 errors

   null
danfuzz commented 7 years ago

Related issue: https://github.com/flowtype/flow-typed/pull/545