Flow types, take 3 (actually 4) #254
Conversation
| export type Middleware = (methods: { dispatch: Dispatch, getState: () => State }) => (next: Dispatch) => Dispatch; | ||
| export type Store = { dispatch: Dispatch, getState: State, subscribe: Function, getReducer: Reducer, replaceReducer: void }; | ||
| export type CreateStore = (reducer: Function, initialState: any) => Store; | ||
| export type HigherOrderStore = (next: CreateStore) => CreateStore; |
There was a problem hiding this comment.
Woah. That's pretty awesome.
There was a problem hiding this comment.
Oops obviously the type of reducer should be Reducer :)
|
Oh shit just realized I forgot to add |
|
Can we integrate this into the build pipeline? Check Flow before running tests? |
|
I believe we can with https://github.com/sindresorhus/flow-bin. That's what graph-ql uses https://github.com/graphql/graphql-js/blob/master/package.json#L37 |
|
Can you please bring this up to date with the last changes I made to breaking changes branch? |
|
Also, should we take this in now? After 1.0? |
|
Also, do we want https://github.com/babel-plugins/babel-plugin-flow-comments for the non-minified builds? |
|
Don't sure |
|
http://flowtype.org/blog/2015/02/20/Flow-Comments.html |
Still might be useful for people browsing the compiled source (e.g. when debugging some issue). |
|
Are you talking about devs of Redux (who should actually use uncompiled sources) or devs who use Redux? IMO not so much JS devs are familiar with Flow so it's more code pollution than code documentation. |
|
I'm talking about users. I think our core audience is at least passingly familiar with Flow. Also, I'd say even a person unfamiliar with Flow wouldn't have a problem reading most Flow annotations. Anyway I don't have a strong opinion here. Let's see what others have to say. |
|
I really like the type annotation comments in the code. It would help me find a type bug earlier. The only downsides are file size and confusion. I don't imagine too many people confused by the comments (they are just comments) and I think it adds a lot of value. |
|
File size isn't a problem because comments are stripped by any minification tool. |
It would be much better to have types in jsDoc for non-minified build rather then not working actual alternative Flow type annotation. BTW personally I don't care much about it.
I don't think so. You will find a bug for same time as without these comments cause they have nothing to do with types and they can't say "Hey! I want String not Integer!". All they can say is "Hey! Hey look at me! I'm String! Really!" when you scrolling your code. And all of them will do it. Do you really think it can help? |
|
Also Flow comments are created as a temporary solution which should help in evaluating migration to standard Flow type annotation not for code documentation. jsDoc can help by static analysis, autocompletion and function parameters hinting. Flow comments can't do any of that |
|
But maybe I'm wrong... For example can I use standard Flow annotations in my app together with Redux's non-minified version Flow comments? Does someone has any experience with such workflow? |
This is the answer http://flowtype.org/docs/third-party.html |
|
So I suggest supporting Flow's interface file, TypeScript's tsd file and cut of all Flow annotations from any builds |
@gaearon Yes. Minified is the same if comments exist or not. I thought we talking about unminified? That's where the difference lies. @chicoxyzzy Unfortunately, currently, the flow type annotations are stripped before publishing to NPM. You can't access the raw flow files via NPM. A flow interface file like you linked or correct comments would work to remedy a full flow app. If you aren't using flow, the type information inline (via flow comments, correct if possible, or jsDoc) is useful when debugging library code. I can inspect the I am definitely for a correct flow workflow. I am just saying, unless someone is working on creating jsDoc processors or correct flow comments, I think there is value in the flow comments with very little downside. |
|
If you see any issues with the babel plugin's comment output - def make an issue (if it's not the one already there) in the repo or ping me |
|
I updated the pull request to match the 1.0 API. (e.g. #206?) |
|
In particular I'm interested what we need to change to enjoy this kind of static analysis: facebookarchive/flux#243 Do we need to change more signatures to be generic to allow this? Should it be |
Flow types, take 3 (actually 4)
|
I've merged this in the current state, but I opened #290 to track improvements to our definitions to allow facebookarchive/flux#243-like static analysis. |
This says otherwise:
It's not just a temporary measure as I read it; it's a solution they plan to use for publishing Flow code on NPM. You don't need separate interface definitions if the comments “just work”. I'm going to investigate the Babel plugin. If it helps Flow users consume (and interpret!) Redux code with annotations, isn't that awesome? |
|
For what it's worth, in the past, I've found flow-comments useful as a supplement to Flow types. There will be cases when Flow types are in conflict with the ES6 or ES7 notation, e.g. argument destructuring. |
|
One thing I'm not sure of is how to handle the fact that our types are in a separate |
|
@gaearon I see you already got a PR for |
It works!
(Did I git it right this time?)