New Take on adding Flow Types #817
Conversation
|
|
||
|
|
||
| [options] | ||
| module.system=node |
There was a problem hiding this comment.
As you can see, I removed the line module.name_mapper='^\(redux\)$' -> '\1/src/index' and opted to continue with Flow declaration modules (the [libs] option) since that regex was conflicting with redux-thunk, and I was not able to create a regex that wouldn't stop that problem. If anyone can give this a try, or discuss approaches, let's start a thread here.
|
|
||
| declare type Middleware<State, Dispatchable, DispatchableNext> = (api: MiddlewareAPI<State, Dispatchable>) => (next: Dispatch<DispatchableNext>) => Dispatch<Dispatchable>; | ||
|
|
||
| declare function applyMiddleware<State, Action, Dispatchable, DispatchableNext>(middleware: Middleware<State, Dispatchable, DispatchableNext>) |
There was a problem hiding this comment.
You'd need to call applyMiddleware many times to have several middleware while staying typed, right? Seems like a good enough compromise to me.
There was a problem hiding this comment.
I can define a couple of overrides, was just testing with one (since the counter example just uses one middleware), but it should be possible to define multiple, as we discussed in the other thread (perhaps up to 5, 6 middleware functions)
|
Hey, thanks for your work on this again. I feel like we can get it right with three or four more iterations 😉 Can I ask you to write up a summary of the blocker issues in Flow that currently prevent us from finalizing this and recommending this setup to Flow users? |
| @@ -0,0 +1,32 @@ | |||
| /* @flow */ | |||
|
|
|||
| import React, { Component, PropTypes } from 'react'; | |||
There was a problem hiding this comment.
Ah forgot to remove this from the original example, thanks!
|
Currently the issues that are blocking us in Flow are:
The recommended setup for Flow users would be to include the module declaration files as part of their In order to not forget to maintain these, it would be nice to type check the source code of the libraries internally and generate that declaration module, but that is currently not possible (facebook/flow#714). So I'm afraid this would be something to pay attention when changing the API of any lib :(. |
…dux init action in action types
|
|
||
| React.render( | ||
| // Flow limitation: Does not detect children type if we don't explicitly | ||
| // set the `children` prop |
There was a problem hiding this comment.
This is being tracked here: facebook/flow#892
| }; | ||
| } | ||
|
|
||
| export default connect(mapStateToProps, mapDispatchToProps)(Counter); |
There was a problem hiding this comment.
Finally made this to type check!!! The $Diff type was working, the problem was that the correct flow type for classes that extend React.Component is ReactClass, so that it knows how to infer the polymorphic props and state types
|
@leoasis have you looked at this more recently? I believe that the majority of those issues in flow has been fixed, in addition to the ability for flow to type check files in node_modules (files with |
|
@iamdustan no, haven't been able to, but will definitely try to take a look into it sometime soon. If interested, let me know and we can discuss what needs to be done to get this |
|
i definitely am interested, just not sure on current availability :( |
|
If I could get back to this (or someone else), would you be willing to have the flow types directly in code if it were already possible? Or would you prefer to have them as external declaration files? What would you guys prefer? |
|
I don’t care either way as long as we can meaningfully export them for consumers. |
|
If the source files are copied to the npm distro with .flow extensions flow will automatically pick them up. :) |
|
I am closing for inactivity because this hasn’t been updated for quite a while. |
|
Interested in this as well! |
Alright, another take, but not yet complete :(
Here I mostly made the counter example work with flow types (even types for redux-thunk and react-redux, with some caveats).
I've opted for just moving on with type declaration modules instead of types in the redux src directly since I've had problems with the
name_mapperflow option (more details on that particular line). When we solve that, and a couple more problems that hopefully we can all solve together, we can be ready to add Flow types to redux!And if not, then it will be here for posterity to perhaps try again in a future Flow version.
Pending stuff here:
const cannot be reassignederrors facebook/flow#844 (fixed, but not yet released)Union of unions doesn't work as I expected facebook/flow#582 (supporting union of unions, not yet fixed) This requires us to include the redux INIT action as part of the action types in userland.Type signature for object resulting in the merge of two others facebook/flow#879 (type signature to signify an object type being a merge from two other object types). No workaround for this for now