Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow module declarations in external file #593

Closed
Macil opened this issue Jun 30, 2015 · 12 comments
Closed

Flow module declarations in external file #593

Macil opened this issue Jun 30, 2015 · 12 comments

Comments

@Macil
Copy link
Contributor

Macil commented Jun 30, 2015

I'm looking forward to #447 and being able to create declarations for external modules that I'm using. Recently I started thinking in-depth about whether external modules could include their own declarations.

If I develop a module using Flow, and then publish it on NPM, I'd like the published module to include its declarations and automatically be typesafe to other Flow users who use it. But I can't just publish a regular Flow-annotated file onto NPM, because 1) I want it to work with regular Javascript users, so I'm going to publish the transpiled type-stripped file, and 2) transpilers like Babel (which I use for transpiling Flow code; I assume the official transform tool works similarly) specifically ignore the node_modules/ directory, so Flow-annotated files published on NPM wouldn't even work for Flow users anyway. (I'd also like to be able to write declaration files for popular libraries which weren't made with Flow, and then submit pull requests to cooperating libraries to get them to include the declaration file without affecting their code or build process.)

When I publish a module onto NPM, I'd like to be able to include an extra file ("flow.js", "flow.declarations.js", "something.d.js", etc. Maybe it's named in the module's package.json?) which contains all of the public declarations for the module. Or maybe the module's main file can have a special /* @flow external:declarations.js */ comment (this one seems more general, but the build process would need to cooperate because the source file probably has a normal /* @flow */ comment).

@Macil
Copy link
Contributor Author

Macil commented Jun 30, 2015

Related: I'm pretty the above are a lot of the same problems that TypeScript's external .d.ts files were meant to solve.

I'm a little hesitant to mention them since all of my experiences of trying to use TypeScript and NPM together have been painful and drove me away from TypeScript (not me but http://blog.nemo157.com/2014/07/typescript-and-npm-modules.html describes it pretty well), but I think most of that pain came from TypeScript not knowing about the node_modules/ directory (which Flow is good with) and having some mysterious difference in how definitions worked between "external modules" and "global modules".

@samwgoldman
Copy link
Member

I agree it would be great to support type declarations as a part of node modules. One possibility is supporting nested .flowconfig files, so the node module could include one which points to a lib file declaring its own types.

For your own modules, there is an option that works today: you can use this babel plugin to translate the annotations into comment syntax, which flow will pick up, but non-flow parsers will ignore.

@gaearon
Copy link
Contributor

gaearon commented Jul 21, 2015

For your own modules, there is an option that works today: you can use this babel plugin to translate the annotations into comment syntax, which flow will pick up, but non-flow parsers will ignore.

I'm trying to use this, but Flow doesn't like the code generated by Babel ES6 module support.

@samwgoldman
Copy link
Member

@gaearon Can you share more details? Steps, expected result, and actual result would help us understand what is going on.

@gaearon
Copy link
Contributor

gaearon commented Jul 21, 2015

@samwgoldman

Compiled Babel code contains a lot of code like

exports.__esModule = true;
exports['default'] = createStore;

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { 'default': obj }; }

var _utilsIsPlainObject = require('./utils/isPlainObject');

var _utilsIsPlainObject2 = _interopRequireDefault(_utilsIsPlainObject);

Flow is not impressed at this kind of code:

createStore.js:32:3,24: access of computed property/element
Property not found in
isPlainObject.js:8:1,14:1: statics of function

(and a bunch of these)

In the end I'm giving up on the compiled output, and will encourage users to use name_mapper to use the source code instead: reduxjs/redux#290 (comment). (Unless there's a better solution I'm missing.)

@samwgoldman
Copy link
Member

@gaearon Ahh. Thanks for clarifying. When I wrote about that babel plugin, I wasn't considering all the other source transformations that babel does, but that was clearly short-sighted of me.

As far as flow understanding all of babel's various transpilation products, there are a couple of issues:

  1. Flow is incomplete. For example, flow doesn't understand computed property key access, even when the key is a string literal. There isn't anything preventing flow from understanding this code, it just hasn't been implemented.
  2. Babel's output may defy static analysis. Due to the nature of the language itself, there are many perfectly valid JavaScript programs that flow will reject. We are looking to close the gap, but since there's so much to do, we need to focus on the highest impact areas. Unfortunately, some of the transpilation output might not represent patterns often found in manually-written code.

For 1, we have a variety of issues dedicated to resolving incompleteness in flow. #252 tracks support for computed property keys, for example. For 2, I can see two paths to resolution: A) Browsers/node will eventually support more ES6/7, so the transpiled output goes away. B) Babel might consider, where it's possible, to change their output to JavaScript which is supported by flow. I am not sure if that's a reasonable course of action, but it's up to them.

For now, I think the name_mapper is your best bet. Thanks for bringing this issue up!

@Macil
Copy link
Contributor Author

Macil commented Aug 1, 2015

Nested .flowconfig support sounds like a clever solution, though I think it still leaves open problems because many flow-typed NPM modules will be transpiled and the transpiled output may contain @flow comments but not be valid flow code because of issues like #677. I'm not convinced that keeping the transpiled output as validateable is tractable (I'm thinking about how async functions transpile to very different code, etc) and I wouldn't want this issue to depend on that being solved first or ever.

Also, if a nested .flowconfig file defines an [ignore] and its own [lib] files for common modules that the application also uses (like lodash), it's not really obvious what should happen. If my application depends on module bar and bar happens to contain lib files that declare other modules like lodash, I don't think I want to use bar's declarations. (The application might have its own.)

There should be a way for flow to know to ignore the transpiled javascript files in a module, and only pay attention to the bundled flow declarations for the module. Maybe there could be an entry in package.json which defines a flow declarations file to use instead of the module's own javascript files.

@sebmck
Copy link
Contributor

sebmck commented Aug 15, 2015

Babel might consider, where it's possible, to change their output to JavaScript which is supported by flow. I am not sure if that's a reasonable course of action, but it's up to them.

This is absolutely an option. Another option (and what I'd prefer) is to do something extra in the flow-comments Babel plugin that outputs more comments about the original type information so that Flow can understand it.

I'm not as familiar with Flow as I'd like but ideally it'd be cool to output something like:

class Foo {
  foo: string;

  bar(): string {

  }
}

into:

/*::declare class Foo {
  foo: string
}*/

var Foo = ...;

This way the compiled code can be however it wants, but Flow will be able to read the typed comment and understand.

@samwgoldman
Copy link
Member

I think that's definitely the best path forward and I'd be happy to help in any way to get there.

@Macil
Copy link
Contributor Author

Macil commented Aug 15, 2015

I'd still like for module declarations in external files to be possible. There are projects not written with Flow or Babel that I think would be willing to include a separate hand-written flow declaration file.

@Macil
Copy link
Contributor Author

Macil commented Sep 28, 2015

For my ud library, I've included a hand-written Flow type declaration file and instructions in the readme to include that file under the [libs] section of the application's .flowconfig (and instructions on working around Flow bug #676).

It would be convenient for users if I could specify the Flow declaration file in package.json like Typescript supports.

@Macil
Copy link
Contributor Author

Macil commented Dec 2, 2015

Woot, Flow 0.19 quietly solved this.

Here's an example of Flow declarations being included with a library on NPM if anyone is curious for an example. It took me a few tries to get it working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants