Closed mhart closed 9 years ago
:+1: Reading https://github.com/facebook/flow/blob/master/src/typing/comments_js.ml it looks like its supported.
+1 for this. Would definitely be helpful in evaluating migration.
@Raynos Oh cool – just didn't see it mentioned in the docs (maybe it's not yet?)
Oooh, think I found some tests: https://github.com/facebook/flow/blob/master/tests/docblock/docblock.js
Might change this to be a documentation issue (edit: done)
cool :) Sounds like we just need documentation for this.
This is not a documentation issue. flow port
can convert your code that uses "docblock" based annotation into the flow annotation syntax:
/* @flow */
/**
@param {string} x
*/
function foo(x) {
return x*10;
}
foo("Hello, world!");
will give the following output:
$ ./flow port examples/01_HelloWorld/hello.js
/Users/arthur/Downloads/flow/examples/01_HelloWorld/hello.js
--- old
+++ new
@@ -3,7 +3,7 @@
/**
@param {string} x
*/
-function foo(x) {
+function foo(x: string) {
return x*10;
}
But these annotations can't be directly used for type checking. But in theory, it should be possible to change flow to do this conversion in-place.
Ah cool, makes sense.
Ok – will keep this issue open then
Ah I see. Fair enough.
Hah, at Facebook we have existing code using a comment syntax that we try to convert into Flow annotations. This feature should be treated as experimental, not sure if our comments syntax is anything close to standard (jsdoc?)
It just makes so much more sense to me to add these type annotations in comments. Then you can reuse existing comment-stripping build tools instead of inventing a new one. The only benefit I can see of using a new syntax is that it forces the compile step (and its accompanying type check).
Comments are kind of a parsing nightmare. Knowing which node a comment belongs to is a Hard Problem, and often is solved by "guessing" based on whitespace. The way around this is to stick to conventions, but then you need something to enforce these conventions or risk silently misbehaving when they're broken.
So yeah, adding type annotations to the grammar simplifies things, though it does add a build step. It's a definite tradeoff, though.
Hmm, what if you ignored jsdoc comments and only respected inline comments, like here? That shouldn't be too hard to parse, right? You could potentially even give a warning for any comment inlined that way that didn't parse correctly to prevent silent misbehavior. Or if that inline comment syntax is already used in lots of places maybe require a "flow:" prefix, e.g. function foo(/*flow:string*/ text, /*flow:number*/ count) /*flow:string*/ {}
In other words, why is
function foo(a: mixed, b: number): void { ... }
var x: boolean;
class Bar {
y: string;
}
parseable, but not
function foo(a /*: mixed*/, b /*: number*/) /*: void*/ { ... }
var x /*: boolean*/;
class Bar {
y /*: string*/;
}
?
(Not that I recommend that exact syntax. Just wondering :-)
Having restrict rules around comments is fine.
// foo(a: mixed, b: mixed) : void
function foo(a, b) {
// ...
}
The real problem in parsing comes when you combine the type definition and documentation in one comment block.
Supporting just type definitions in a single comment statement where the author of the type definition is not allowed to leave "comments" in the comments or "documentation" in the comments makes this problem far simpler.
We can also use a flow: prefix as mentioned above and warn about any flow: comments that were not parsed properly.
It is actually fairly straightforward for type annotations. You can steal the logic from the closure compiler parser. On Nov 18, 2014 4:14 PM, "Gabe Levi" notifications@github.com wrote:
Comments are kind of a parsing nightmare. Knowing which node a comment belongs to is a Hard Problem, and often is solved by "guessing" based on whitespace. The way around this is to stick to conventions, but then you need something to enforce these conventions or risk silently misbehaving when they're broken.
So yeah, adding type annotations to the grammar simplifies things, though it does add a build step. It's a definite tradeoff, though.
— Reply to this email directly or view it on GitHub https://github.com/facebook/flow/issues/3#issuecomment-63570619.
I agree it would be very useful for flow to support @param
annotations (as an alternative to the provided :type
syntax) in order to deduce the types. This will make it significantly easier for existing projects to benefit from Flow, and will avoid the transpiler step for live code + the compilation step for minified code.
Avoiding having to use a transpiler is a big deal in making this usable on existing projects, and while I think @cletusw's suggestion was also really neat, for example:
function foo(a /*: string*/, b /*: number*/) /*: number*/ {
// function body...
}
in terms of working with existing idiomatic JavaScript, which is one of Flow's stated goals, then supporting jsdoc annotations would be a huge win also, so:
/**
* @param {String} a
* @param {Number} b
* @returns {Number}
*/
function foo(a, b) {
// function body...
}
Using comment type annotations might be necessary if using simple compile steps like browserify or sweet.js, where JS type annotations are relevant but adding them outside of comments would break the compile steps.
@cletusw's suggestion, and the rest of the arguments on this issue are extremely sound. For a bit more typing (and maybe a tad uglier code), you could get all the benefits of Flow without having to leave standard JavaScript syntax compatibility. This is a HUGE win in terms of bringing existing projects onboard. Also, many people may not want to make the commitment even in greenfield projects, as going over to Flow (or TypeScript) will need support from most other tools and IDE's you might want to use, and will massively limit the ecosystem available to you. Finally, as stated above, this aligns perfectly with Flow's stated goal of being able to work with any JS, and helping out with type annotations only gradually and when needed.
As to the concrete proposal, I would go even further and drop the colons:
/* @flow */
function foo(a /*mixed*/, b /*number*/) /*void*/ { ... }
...as it's highly unlikely inline comments would exist at those exact locations unless they're meant for this exact purpose.
If @avikchaudhuri or @gabelevi could comment as to how likely adding a 100% JS compatible syntax to the project would be, that'd be hugely appreciated! Because if not, someone in the community can take up building a pre-processor for instance.
+1 for @Raynos 's suggestion. I think it's very clear and allows smoother migration.
+1, I agree that this would be an awesome feature with whichever comment syntax was most feasible.
Just to add another possible syntax, the inline Closure Compiler annotations look pretty similar to flow except for the return type before the function rather than after. I don't know if the logic from Closure Compiler to parse this would be helpful at all in porting the capability to Flow, though.
function /** string */ foo(/** string */ a, /** number */ b) {}
Thanks for considering this feature!
+1, I made the same comment on HN when the announcement came out. Annotations belong in annotations (aka. comments); transpiling is quixotic.
@Raynos @jareware @andrewrota
what's the benefit of using a new, custom syntax as opposed to already widely adopted jsdoc?
/**
* Foo
* @param {String} foo
* @param {Number} bar
* @return {Array} An array
*/
function foo (foo, bar) {}
Or (not sure if flow is this smart):
/**
* Bar
* @param {Array} foo An array
* @param {Array<Array>} bar An array of arrays
* @param {DOMElement} baz An element
* @return {Promise}
*/
function bar (foo, bar, baz) {}
@bcherny, I think JSDoc syntax would be great, I just thought it would be mentioning the more inline syntax (especially since Flow's type annotations are inline). Closure Compiler supports both the JSDoc style and the inline comment style.
@bcherny
The benefit is that the JSDoc language is different from the flow language.
Using something that looks like JSDoc but is not JSDoc is confusing.
With that said you might as well use a comment language that maps as cleanly onto flow as possible. This means you have zero legacy baggage from JSDoc and zero legacy baggage from JavaDoc.
Coupling yourself to JSDoc has no benefits and just limits flow.
@Raynos I already use jsdoc for docs and automated interface testing, so my ideal workflow would be to continue using jsdoc, and add in flow as a static typechecking tool that makes use of my existing jsdoc annotations.
The parser is already decoupled from the rest of flow's source, and it would be cool to have a few different parsers available (flow, jsdoc, closure, etc.).
Coupling yourself to JSDoc has no benefits and just limits flow.
Can you explain this a bit more?
@bcherny
JSDoc has no way to express overloaded functions. i.e.
((x: number) => void) & ((x: string) => void)
Trying to express this syntax in the existing "framework" of a comment block with a list of new line seperate @param
statements is going to be frustrating.
What you really want is a one line comment above the function that is the exact same as the flow language itself.
@Raynos By "overloaded" you mean a function which can return multiple types, with params that might accept multiple types? Jsdoc does support that use case, but you're right that it does not support mapping inputs to outputs.
/**
* @param {String|Number} foo
* @param {Any} bar
* @return {Array|Object} An array or an object
*/
function (foo, bar) {}
@bcherny
overloaded means ((x: number) => number) & ((x: string) => string)
. This is different from having parameters that are union types and having return values that are union types.
You are correct that the main difference is, with overloaded functions you can infer the correct return type based on the input types.
This is one of many advance features flow supports that are hard to express in jsdoc.
A flow comment system should have a flow based language.
Having external support for flow converting JSDoc into a subset of the flow type system seems reasonable.
While JSDoc certainly has some holes, I'm not sure why pulling in additional type info from JSDoc would be a problem. I think it's also reasonable to expect JSDocs to continue to improve over time. For reference, here's the Google Closure issue to define/add support for intersection types.
https://github.com/google/closure-compiler/issues/202
@Raynos What do you mean by external support? I'm not sure if you're suggesting
While it's always easy to make feature requests, I'd personally prefer a mix & match approach, where Flow can consume both styles of type info. If both JSDoc and Flow type annotations are provided, Flow could
So with bcherny/Raynos's example, any of the above three options would result in enforcement of the overloaded num->num and string->string behavior.
As an alternative to writing a transpiler, perhaps Flow could provide an addon hook? That would allow a 3rd party to plug in their own parsing/typing components. There could be an addons for generic JSDoc, for closure-style, etc. That would keep it very firmly distinct from the core Flow language while also giving the community the flexibility to play around..
@JaRail
While it's always easy to make feature requests, I'd personally prefer a mix & match approach, where Flow can consume both styles of type info.
I agree completely. Flow has a parser and a validator - the validator should be pluggable, so we can support multiple existing formats:
The validator should be annotation syntax-agnostic, so long as the output produced by the parser is the same regardless of source language/annotation style.
Just out of curiosity, could flow potenitally then be used as a jsdoc/documentation generator? I'd find that somewhat useful for getting started on writing docs for a project that's already got a bit of code.
+1
This would allow compatibility with ES6 transpiled projects (e.g., traceur). Flowtype isn't yet a drop-in replacement for an ES6 transpiler since it's ES6 support is limited.
+1 for me too. Is anyone actually working on this? I don't want an "almost-js -> real js" transpile step in my personal projects, so I'd be keen to follow along/help out.
For @misterspeaker et al, I put together https://github.com/jareware/flotate for just this purpose. That is, using Flow with standard JavaScript syntax.
@jareware Just had a browse of the docs, looks great! I guess it was probably easier to come up with a new syntax rather than try and support JSDoc et al from the get go?
I look forward to trying this out – and hopefully soon we'll be able to check code without the compile/temp-dir step!
@mhart yeah, I gave up on JSDoc pretty quickly after realizing how hard it'd be to match up all params with their types, how much repetition that'd introduce, and that (as @Raynos pointed out above as well) it still wouldn't cover e.g. overloaded function signatures well.
By using the exact same positions as Flow does, and reserving a prefix for the comments (/*:
) the conversion is actually dead-simple, as you don't have to care about where the annotations are encountered, you just strip the surrounding /*
and */
and you're done. :)
I'm hoping something along these lines eventually ends up in Flow itself, at which point the preprocessor obviously becomes unnecessary!
+1 as it would avoid new non-JavaScript syntax and flow through existing annotated libraries. Arguments have been raised that not every case is covered in JSDoc, and while that is true, it would make flow useful for projects without build steps and using regular JS (e.g. almost all node libraries out there).
+1 for this feature. Any of the proposed syntax options would immediately speed my adoption of flow since it is unreasonable for my team to convert existing projects to typed JS right now.
Flotate one-line-syntax looks like a much more prefereble solution. It will not break established workflows, testing routines, IDE support, existing code etc.
Also, it looks like it will be easier to integrate flow with things like coffeescript.
With regard to the discussion above, while flotate doesn't (yet) support full-blown JSDoc, there's now also an alternative syntax for functions/methods, so that in addition to:
function foo(x /*: string */, y /*: number */) /*: boolean */ {
return x.length * y === 5;
}
the following is also supported:
/*: (x: string, y: number): boolean */
function foo(x, y) {
return x.length * y === 5;
}
which is nice in keeping the annotations & code separate.
@jareware that's neat! much better readability, although param names get duplicated, but it's a great option
@jareware Thanks so much for telling us about flotate! I can't wait to use it. As the primary author, do you think that flotate provides evidence that type annotations in comments constitutes a viable method of expressing type information? Seeing as how you went ahead and wrote your own tool, do you think flow proper should support type annotations in comments?
@yonkeltron Definitely viable! I guess the question is more about whether type checking for JavaScript is useful or not. I think it is, and that Flow is currently the best too for the job. flotate just bridges the small syntactic gap to standard JS. For instance, I'm currently gradually adding type information to an existing project using these tools, and have found them helpful!
As to the second question, I think that's something they're considering, and I'd be glad to have the preprocessor become unnecessary. :)
Ok, you guys convinced us! We've merged @jareware's awesome flotate syntax upstream, directly into Flow! Here's a blog post about it and we'll write proper documentation after we've caught up on sleep :)
Yessss, excellent news!
Going to close this – if anyone's got issues with the syntax or anything else they can open up separate issues.
This is brilliant! :bow:
Good job!
flotate looks great! Please support JSDoc syntax also. Thanks.
Didn't see support for this in the documentation, but it would be great if Flow supported type annotations in comments (or similar) – with the aim of allowing everything to be written in pure JavaScript.
This would allow people to keep the same tools they currently use, and skip the transpilation step.
Has this idea been floated at all? Is it doable?
Edit: Looks like this is doable – perhaps the documentation just needs to be updated to expound on this? Edit again: OK, so atm it's just conversion from docblock to flow – would be great if it could be done in-place... or something