Flow types, take 3 (actually 4) #254

acdlite · 2015-07-12T16:02:06Z

It works!

(Did I git it right this time?)

gaearon · 2015-07-12T16:06:39Z

src/types.js

+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;


Woah. That's pretty awesome.

Oops obviously the type of reducer should be Reducer :)

acdlite · 2015-07-12T16:12:51Z

Oh shit just realized I forgot to add /* @flow */ to the top of the types file. :D Surfaced a few errors. I'll fix and update.

gaearon · 2015-07-12T17:44:56Z

Can we integrate this into the build pipeline? Check Flow before running tests?

acdlite · 2015-07-12T17:45:54Z

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

gaearon · 2015-07-12T23:46:21Z

Can you please bring this up to date with the last changes I made to breaking changes branch?

gaearon · 2015-07-12T23:49:05Z

Also, should we take this in now? After 1.0?

gaearon · 2015-07-14T19:11:09Z

Also, do we want https://github.com/babel-plugins/babel-plugin-flow-comments for the non-minified builds?

chicoxyzzy · 2015-07-14T20:23:46Z

Don't sure flow-comments plugin is necessary. It's not very readable and flow is useful for compile time not after.

chicoxyzzy · 2015-07-14T20:28:05Z

http://flowtype.org/blog/2015/02/20/Flow-Comments.html
Flow comments are alternative to standard Flow annotations actually

gaearon · 2015-07-14T20:35:40Z

It's not very readable and flow is useful for compile time not after.

Still might be useful for people browsing the compiled source (e.g. when debugging some issue).
Writing them as comments gets cumbersome, but I'd welcome comments in the compiled NPM output.

chicoxyzzy · 2015-07-14T23:58:23Z

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.

gaearon · 2015-07-15T00:14:58Z

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.

taylorhakes · 2015-07-15T00:36:31Z

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.

gaearon · 2015-07-15T01:10:09Z

File size isn't a problem because comments are stripped by any minification tool.

chicoxyzzy · 2015-07-15T01:16:30Z

I think our core audience is at least passingly familiar with Flow.

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.

It would help me find a type bug earlier.

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?

chicoxyzzy · 2015-07-15T01:28:15Z

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

chicoxyzzy · 2015-07-15T01:54:12Z

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?

chicoxyzzy · 2015-07-15T01:56:16Z

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

chicoxyzzy · 2015-07-15T02:02:39Z

So I suggest supporting Flow's interface file, TypeScript's tsd file and cut of all Flow annotations from any builds

taylorhakes · 2015-07-15T03:39:16Z

File size isn't a problem because comments are stripped by any minification tool.

@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 'hello' value and see /*: number*/ next to it.

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.

hzoo · 2015-07-16T03:05:51Z

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

gaearon · 2015-07-19T19:09:54Z

I updated the pull request to match the 1.0 API.
This is ready to merge, but I wonder if there is anything we can (or should) expose to Flow users..

(e.g. #206?)

gaearon · 2015-07-19T19:40:02Z

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 createStore<MyState, MyAction>()? How does this play with the middleware? And how do we expose these definitions?

Flow types, take 3 (actually 4)

gaearon · 2015-07-19T19:47:15Z

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.

gaearon · 2015-07-19T20:51:12Z

Also Flow comments are created as a temporary solution which should help in evaluating migration to standard Flow type annotation not for code documentation.

This says otherwise:

We plan to update our Flow transformation to wrap Flow syntax with these special comments, rather than stripping it away completely. This will help people write Flow code but publish code that works with or without Flow.

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?

volkanunsal · 2015-07-19T21:16:10Z

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.

gaearon · 2015-07-19T21:18:48Z

One thing I'm not sure of is how to handle the fact that our types are in a separate types file.
@hzoo Can the plugin handle import type... somehow?

hzoo · 2015-07-20T14:50:42Z

@gaearon I see you already got a PR for import type 😄

acdlite added 3 commits July 9, 2015 23:41

Add Flow type annotations

4c68696

Fix linting

e854617

Merge branch 'breaking-changes-1.0' into flow-types-take-2

dc44972

gaearon reviewed Jul 12, 2015
View reviewed changes

gaearon mentioned this pull request Jul 12, 2015

Flow type annotations #87

Closed

gaearon mentioned this pull request Jul 14, 2015

Runtime type checking #268

Closed

gaearon mentioned this pull request Jul 15, 2015

Typescript definition #206

Closed

gaearon added 2 commits July 19, 2015 20:17

Update Flow annotations with Redux 1.0 API changes

eaa20fa

Fix the missing Flow annotations

ac8a184

gaearon mentioned this pull request Jul 19, 2015

Add flow types to dispatcher facebookarchive/flux#243

Merged

gaearon added a commit that referenced this pull request Jul 19, 2015

Merge pull request #254 from gaearon/flow-types-take-3

98ab1de

Flow types, take 3 (actually 4)

gaearon merged commit 98ab1de into breaking-changes-1.0 Jul 19, 2015

gaearon deleted the flow-types-take-3 branch July 19, 2015 19:41

gaearon mentioned this pull request Jul 19, 2015

Explore using Flow for statically analyzing state and actions #290

Closed

This was referenced Jul 20, 2015

use function bind operator for hasOwn #292

Closed

Remove Flow types for now #299

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flow types, take 3 (actually 4) #254

Flow types, take 3 (actually 4) #254

acdlite commented Jul 12, 2015

gaearon Jul 12, 2015

acdlite Jul 12, 2015

acdlite commented Jul 12, 2015

gaearon commented Jul 12, 2015

acdlite commented Jul 12, 2015

gaearon commented Jul 12, 2015

gaearon commented Jul 12, 2015

gaearon commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

gaearon commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

gaearon commented Jul 15, 2015

taylorhakes commented Jul 15, 2015

gaearon commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

taylorhakes commented Jul 15, 2015

hzoo commented Jul 16, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

volkanunsal commented Jul 19, 2015

gaearon commented Jul 19, 2015

hzoo commented Jul 20, 2015

Flow types, take 3 (actually 4) #254

Flow types, take 3 (actually 4) #254

Conversation

acdlite commented Jul 12, 2015

gaearon Jul 12, 2015

Choose a reason for hiding this comment

acdlite Jul 12, 2015

Choose a reason for hiding this comment

acdlite commented Jul 12, 2015

gaearon commented Jul 12, 2015

acdlite commented Jul 12, 2015

gaearon commented Jul 12, 2015

gaearon commented Jul 12, 2015

gaearon commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

gaearon commented Jul 14, 2015

chicoxyzzy commented Jul 14, 2015

gaearon commented Jul 15, 2015

taylorhakes commented Jul 15, 2015

gaearon commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

chicoxyzzy commented Jul 15, 2015

taylorhakes commented Jul 15, 2015

hzoo commented Jul 16, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

gaearon commented Jul 19, 2015

volkanunsal commented Jul 19, 2015

gaearon commented Jul 19, 2015

hzoo commented Jul 20, 2015