-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
Explore using Flow for statically analyzing state and actions #290
Comments
I don't know if there's a better way to represent algebraic data types in flow's type system Also, to get a safe and exhaustive pattern matching you can use church encoding |
oh, apparently there's a more idiomatic way of handling ADTs http://flowtype.org/blog/2015/07/03/Disjoint-Unions.html |
I managed to use the Flow definitions from the Redux source code by using the following Flow config in my standalone example:
Note that this requires It's pretty cool already! I'll play with it and see if any common Redux mistakes can be caught this way, and if I can tweak the type definitions to check the action parameter types. |
@gaearon nice, I was trying to find out how to do that! I'm playing with this a little bit to see if I can get to something |
I made some progress but bumped into this: facebook/flow#652. |
Just wanted to show some progress I made by putting everything into a single file (since it is far from intuitive how to make Flow work across modules, and even more when those modules are in different npm packages). I was able to make the basic What Flow does not understand is using the constants directly in the Action union type, it has to be a string literal. For that reason, and since we now have Flow, I found it no longer necessary to use constants, at least in this example. /* @flow */
//Redux types (provided by redux lib)
type ReduxInitAction = {type: '@@redux/INIT'};
type ReduxAction<Action> = ReduxInitAction | Action;
type Reducer<State, Action> = (state : State, action : ReduxAction<Action>) => State;
type Store<State, Action> = {
dispatch : (action : ReduxAction<Action>) => ReduxAction<Action>,
subscribe : (listener : () => void) => (() => void),
getState : () => State,
getReducer : () => Reducer<State, Action>,
replaceReducer : any
};
//Mock implementation that just type checks
function createStore<State, Action>(reducer : Reducer<State, Action>, initialState : State) : Store<State, Action> {
return {
dispatch(action) { return action; },
getReducer() { return reducer; },
getState() { return initialState },
replaceReducer() { },
subscribe() { return function() {} }
}
}
// App's types
type State = number
type Action = {
type: 'INCREMENT_COUNTER', increment: number
} | {
type: 'DECREMENT_COUNTER', decrement: number
}
function counter(state : State = 0, action : ReduxAction<Action>) {
switch (action.type) {
case 'INCREMENT_COUNTER':
return state + action.increment;
case 'DECREMENT_COUNTER':
return state - action.decrement;
default:
return state;
}
}
// This needs to be type casted for some reason
var store: Store<State, Action> = createStore(counter, 0);
store.dispatch({type: 'INCREMENT_COUNTER', increment: 1});
store.dispatch({type: 'foo'}); // fails type check
store.dispatch({type: 'INCREMENT_COUNTER'}); // fails type check Next steps, for which I may need some help:
|
This is a great starting point already, thank you!
That's exactly what we have with the config I posted above. “Flow as comments in compiled output” doesn't work well because it complains about Babel generated code. The steps are:
|
Oh great @gaearon forgot about that above. Will try to move forward with that |
Hey @gaearon I've been using more and more flow in my own code, and having it to check action constants is quite awesome. Even if you don't put flow everywhere else your code-base it's quite awesome to have them just for action constants like so:
Now when dispatching an action, if you use the constants, flow does error checking for you.
When using strings for events and actions, misspelling can cause a lot of pain. This solves that problem completely. Further, it should possible to generate flow types automatically with minimal tooling. |
I have implemented type-checked actions in my own framework in a way that I think works well. I wanted to share those ideas here. I wrote up the details in #780. |
It seems to me that there are two possible goals for typechecking actions: one is to ensure that actions dispatched to a store are restricted to a given set of action types, another is to ensure that reducer implementations are valid for the action types that they operate on. It should be possible to accomplish both goals; but Flow is fighting me a bit (with subtyping issues that have already been brought up here). I did get a working implementation that checks type-correctness of reducer implementations that I think could be quite useful. This example requires giving up the idea of defining a specific type that describes valid actions for a particular store. What you get in exchange is checking of specific action types against specific reducers and dispatch sites. With this implementation, if an action is modified to add parameters or to change the types of existing parameters, the type checker will point to any reducers or dispatch sites that need to be updated. Modifying the example from @leoasis: type Action<T> = { type: T }
type Reducer<State,A:Action<*>> = (state: State, action: A) => State
function reducer<State,T,A:Action<T>>(type: T, fn: Reducer<State,A>): Reducer<State,A> {
return function (state, action) {
return action.type === type ? fn(state, action) : state
}
}
function compose<State,T>(...reducers: Reducer<State,any>[]): Reducer<State,Action<T>> {
return function (state, action) {
return reducers.reduce((s, fn) => fn(s, action), state)
}
}
type Store<State> = {
dispatch: <A:Action<*>>(action: A) => A,
subscribe: (listener: () => void) => (() => void),
getState: () => State,
getReducer: () => Reducer<State, Action<any>>,
replaceReducer: <T>(fn: Reducer<State,Action<T>>) => void,
}
//Mock implementation that just type checks
function createStore<State>(reducer: Reducer<State, Action<any>>, initialState: State): Store<State> {
return {
dispatch<A:Action<*>>(action: A): A { return action },
getReducer() { return reducer },
getState() { return initialState },
replaceReducer() {},
subscribe() { return function() {} },
}
}
// App's types
type AppState = number
type IncrementCounter = { type: 'INCREMENT_COUNTER', increment: number }
type DecrementCounter = { type: 'DECREMENT_COUNTER', decrement: number }
// Passes typechecking
const counter: Reducer<AppState,*> = compose(
(reducer('INCREMENT_COUNTER', (state = 0, { increment }) => state + increment): Reducer<AppState,IncrementCounter>),
(reducer('DECREMENT_COUNTER', (state = 0, { decrement }) => state + decrement): Reducer<AppState,DecrementCounter>)
)
// The explicit type annotation on each reducer is necessary to get type
// checking for action parameters.
// Does not pass typechecking due to attempts to use parameter that does not
// match up with action type
const counter_: Reducer<AppState,*> = compose(
(reducer('INCREMENT_COUNTER', (state = 0, { decrement }) => state + decrement): Reducer<AppState,IncrementCounter>),
(reducer('DECREMENT_COUNTER', (state = 0, { increment }) => state + increment): Reducer<AppState,DecrementCounter>)
)
// Also does not pass typechecking because the type of the action argument does
// not match up with the string that is given for the runtime check.
const counter__: Reducer<AppState,*> = compose(
(reducer('INCREMENT_COUNTER', (state = 0, { decrement }) => state + decrement): Reducer<AppState,DecrementCounter>)
)
const store: Store<AppState> = createStore(counter, 0); Edit: I forgot to parameterize an occurrence of |
@hallettj thanks for jumping in to solve this problem in redux! Would really love to have this soon. Having said that, I think that in my previous example we were already able to achieve both goals (not sure if there was anything missing or not working, let's discuss this), the only problem I was having was how to have a correct type signature for functions like To show the problem: var reducer = combineReducers(reducers) if reducers has the shape {
foo: Reducer<number, Action>,
bar: Reducer<string, Action>
} then the output should be Reducer<{foo: number, bar: string}, Action> Which I think it's quite impossible to specify with types. |
Oh, and also waiting on this to be fixed in Flow: facebook/flow#582 |
Wow, I did not realize how well type narrowing works in the case statement. That is really nifty! @leoasis: I'm sorry, I should have looked more closely at what you had already done. As for function combineReducers<T:Object,U,Action>(
reducers: T & {[keys: $Keys<T>]: Reducer<$Subtype<U>,Action>}
): Reducer<{[keys: $Keys<T>]: U}, Action> {
// Simplified implementation for demonstration
return (state, action) => {
let result = ({}: any)
for (let k of Object.keys(reducers)) {
result[k] = reducers[k](state, action)
}
return result
}
} The type above describes the input to function combineReducers<T:Object,U,Action>(reducers: T): Reducer<{[keys: $Keys<T>]: U}, Action> {
// Simplified implementation for demonstration
return (state, action) => {
let result = ({}: any)
for (let k of Object.keys(reducers)) {
result[k] = reducers[k](state, action)
}
return result
}
} I'm relaying the first version because I thought that being able to refer to
type CounterState = number
type TodoState = string[]
type Action = {
type: 'INCREMENT_COUNTER', increment: number
} | {
type: 'DECREMENT_COUNTER', decrement: number
}
| {
type: 'ADD_TODO', text: string
}
function counter(state : CounterState = 0, action : ReduxAction<Action>) {
switch (action.type) {
case 'INCREMENT_COUNTER':
return state + action.increment;
case 'DECREMENT_COUNTER':
return state - action.decrement;
default:
return state;
}
}
function todos(state : TodoState = [], action: ReduxAction<Action>) {
switch (action.type) {
case 'ADD_TODO':
return state.concat([action.text]);
default:
return state;
}
}
// no annotation
const reducer = combineReducers({ counter, todos })
// Passes type checking
reducer({ counter: 1, todos: ['string'] }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Does not pass if an invalid key is given
reducer({ counter: 1, todoss: ['string'] }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Passes type checking when some object properties are absent
reducer({ counter: 1 }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Passes if a key is given with the wrong type
reducer({ counter: 1, todos: 'string' }, { type: 'INCREMENT_COUNTER', increment: 1 }) So that will at least catch typos in reducer names. But if there is an annotation at the call site: const reducer_: Reducer<{ counter: CounterState, todos: TodoState }, Action> =
combineReducers({ counter, todos })
// Passes type checking
reducer_({ counter: 1, todos: ['string'] }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Does not pass if an invalid key is given
reducer_({ counter: 1, todoss: ['string'] }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Does not pass when some object properties are absent
reducer_({ counter: 1 }, { type: 'INCREMENT_COUNTER', increment: 1 })
// Does not pass if a key is given with the wrong type
reducer_({ counter: 1, todos: 'string' }, { type: 'INCREMENT_COUNTER', increment: 1 }) If the programmer gets the name of a reducer wrong in the type annotation, that will also result in a type error. If Flow ever gets a A caveat of Another thought I have is that it would be much easer to check variations of function combine2<A,B,Action>(reducers: [Reducer<A,Action>,Reducer<B,Action>]): Reducer<[A,B],Action> {
return ([a, b], action) => [
reducers[0](a, action),
reducers[1](b, action),
]
}
function combine3<A,B,C,Action>(reducers: [Reducer<A,Action>,Reducer<B,Action>,Reducer<C,Action>]): Reducer<[A,B,C],Action> {
return ([a, b, c], action) => [
reducers[0](a, action),
reducers[1](b, action),
reducers[2](c, action),
]
}
// Example use:
const reducer__ = combine2([counter, todos])
reducer__([1, ['do a thing', 'do another thing']], { type: 'INCREMENT_COUNTER', increment: 1 }) This version provides all of the typechecking guarantees as the Maybe there is a niche of functional programming fans who would appreciate having these available as alternative combining functions? I would use them. Though I understand that fixed-arity functions like these are not very Javascript-y. |
The thing is that with this way you will never have the complete safety that you are doing what you want. In your first examples, you don't define the correct types for the reducers that you accept, and you don't specify the correct type of the state of the resulting reducer. I'm ultimately leaning towards just manually creating the reducer, since type AppState = {
foo: FooState,
bar: BarState
}
function reducer(state?: AppState, action: Action) : AppState {
return {
foo: fooReducer(state && state.foo, action),
bar: barReducer(state && state.bar, action)
}
} It's not that bad in my optinion, and it has complete type check, since we are defining the reducers signatures. What do you think? Perhaps it's worth exploring this by adding a new example that would be one of the existing ones but ported to flow without using |
Fair enough, we can do that. I'm also concerned about middleware—seems like adding even something simple like |
Perhaps we would have to have a version of |
Is it possible to define several overloads? e.g. can we somehow define 10 overloads for different number of parameters? |
Just tried and with the |
I think that optional arguments might work better than overloads. E.g.: function foo<A,B,C,D>(a: A, b?: B, c?: C, d?: D) { /* ... */ } |
That would make stuff like this possible: foo(a, null, c, undefined, e) anyway, it may be good enough if there's no overload support in type annotations |
I've sent a PR #817 to tackle this, and we can discuss with some code. I've written some concerns I've been having, if you guys know how to solve any of those, or want to try and play with this code, jump in! Really REALLY want Flow types for redux |
Hi everyone. I'm just curious: are you exploring Flow and TypeScript or was it already decided to go with Flow? Just would like to know the pro's and con's. I haven't really used one of them, but from the outside it looks like TypeScripts community is bigger and there are problems using Flow on windows. |
@donaldpipowitch There are Redux definition files for TypeScript in DefinitelyTyped repo, although they're a bit more generic than we're trying here with Flow. Feel free to explore the same directions for TS! |
@donaldpipowitch Yeah, I've not used Typescript before as well, but I was attracted to the type inference power from Flow, though it is still in its early stages (check the PR I'm working on here to see some problems I'm facing). It would be cool if someone made the same exercise and made the definition file for redux in Typescript, but in a truly polymorphic way, without losing any (or as little as possible) type information provided in the app |
So Flow integration is/was just an experiment and there was nobody who did the same for TypeScript yet. Is this right? |
Well, not "just and experiment". I really want to use it and have it working properly. As for Typescript, yes, no attempts with polymorphic types so far. |
Can Flow be used to get rid of |
That was one of the motivations for it in React team so yes. |
Has anyone had success with type checking props that are bound from a mapStatetoProps function via the connect decorator? In the following code, my bound action creators typecheck perfectly, but I get no errors when I pass incorrect values through a mapStatetoProps method. Apologies if this is not the right place to discuss this. /* @flow */
import React, { Component } from 'react';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import * as playerActions from 'actions/playerActions';
import { format } from 'util';
type P = {
currentTrackSource: ?string;
play: Function;
stop: Function;
backward: Function;
forward: Function;
};
class Controls extends Component {
static defaultProps: {};
props: P;
_audio: any;
render(): any {
return (
<div>
<audio
key={this.props.currentTrackSource}
ref={cmp => this._audio = cmp}>
<source src={this.props.currentTrackSource} />
</audio>
<button
onClick={() => this.props.backward()}
className="fa fa-backward" />
<button className="fa fa-play"
onClick={() => {
this._audio.play();
this.props.play();
}}/>
<button
className="fa fa-stop"
onClick={() => {
this._audio.pause();
this._audio.currentTime = 0;
this.props.stop();
}} />
<button
onClick={() => this.props.forward()}
className="fa fa-forward" />
</div>
);
}
}
function mapStateToProps(state): {currentTrackSource: ?string} {
const {
tracks,
currentTrackId,
} = state.player;
const trackIndex = tracks.map(track => track.id).indexOf(currentTrackId);
const { bucket, key } = tracks[trackIndex];
const currentTrackSource = format(
'http://%s.s3.amazonaws.com/%s',
bucket,
key
);
return { currentTrackSource };
}
function mapDispatchToProps(dispatch): {[key: string]: Function} {
return {
...bindActionCreators(playerActions, dispatch),
};
}
export default connect(mapStateToProps, mapDispatchToProps)(Controls); |
@donaldpipowitch Off topic but just thought I'd chime in on your question as I came across this thread - I have just started a new project, and have spent a few days evaluating both Flow and Typescript, so my experiences might be of interest. My initial preference was for Typescript, due primarily to the IDE integration (once you've seen how well it integrates with Atom (with the atom-typescript plugin) or Visual Studio Code - more or less like working with a language like Java or C# - it's hard to forget about it!), but I kept hitting stumbling blocks that I didn't yet know how to bypass (e.g. libraries not available or outdated in the DefinitelyTyped repo) and as this is a project with a deadline, I decided go with Flow as it's much "lighter touch" - chances are, you won't need to modify your existing code much to get it to type check with Flow compared to Typescript. However, as I continued, I noticed quite a few situations where Flow wasn't catching fairly obvious (to me) errors (e.g. broken import statements which broke the app, but Flow said "no errors"), and I also found the documentation/community support to be lacking (e.g. trying to work out how to get Flow to properly type check React component prop types) - plus I couldn't stop thinking about the Typescript IDE support (Flow is nowhere near, even with Facebook's Nuclide editor). So armed with a bit more knowledge (primarily how to hack together a quick, non-type-safe module definition), I spent half a day converting the project back to Typescript and I have to say, I think it is well worth the learning curve/effort to get it going (which was pretty steep for an ES6 project with "cutting edge" libraries). If the Typescript compiler is happy, it generally seems to mean your application is happy - the only times I've been caught out with runtime errors is where I've used an I'm not too familiar with the details of the two type systems (although it does seem Flow's might be more "correct" in the strict FP-type-system sense based on what I've read - for example, types are not nullable by default, whereas in Typescript they are), and if you want something lighter weight (and therefore easier to remove), Flow might suit better, but ultimately I found there weren't enough people using Flow or enough information out there to get much useful value out of it, whereas the extra effort to get TS up and running (which I plan to document soon) was well worth it. HTH! |
Definitely. Thank you! Did you used it with JSX and React? By coincidence I tried TypeScript yesterday with Cycle and webpack which needs Babel for JSX-transpilation, because TypeScript only supports transpilation for React by default and Cycle uses virtual-dom. It was quite troublesome and didn't work at the end :( |
@donaldpipowitch Yeah, this was with JSX and React (and Typescript ES6 mode with Babel). I can't share the code as it's proprietary but can share config etc. if needed. No idea about using it with Cycle but as far as I know, |
You can use standard functions everywhere where you can use JSX. This is not special to virtual-dom. What is special is that everyone defaults to React, if you use JSX and if you want to use it with something else you need to configure the tools carefully and use special annotations... Somewhere in this step the code is so much changed that Babel can't pick it up :/ Anyway... This is probably not the right place for this discussion :/ |
Does this new flow release help? https://github.com/facebook/flow/releases/tag/v0.19.0 |
This issue discussion is giant. Is there any one file with the latest flow definition file for redux? |
Oh. It seems that you used Flow types internally, but then removed them :( too bad |
So, Babel has a plugin for changing flow types into comments. It's only limitation currently is ES6 classes. Redux, however has ZERO classes in it's code base. So it should be possible to add flow types everywhere and just use the babel plugin to turn them into comments for people using them. That way, people using flow will get type definitions automatically. That said, flow may or may not provide the type-safety that many are hoping for. It will definitely help, but flow will allow a lot more to go through than a strict type system. Still flow is improving fast, and it has a great tooling story, so I think we should add flow types to redux. Typescript type definitions can and should be added as a separate file. Since redux is mostly functions, the type definitions will look very similar for both flow and typescript. It's possible the type definitions file may be generated automatically by typescript. |
i have an open pull request to fix the flow-comments plugin. |
I have an annotation that works @cdebotton - I am not using react-redux, but I have built a similar plugin for Fluxxor. This annotation is not perfect - it is not able to diff the props that are provided via I don't use Here are the relevant type signatures: export type StateMapper<Props, U: $Shape<Props>> = (state: AppState, props: ?Props) => U;
export default function connect<StateProps, DefaultProps, Props, T: _ReactClass<DefaultProps, Props, *, *>>(
mapStateToProps: StateMapper<Props, StateProps>, options: ConnectOptions = {})
: (WrappedComponent: T) => Class<React.Component<DefaultProps, $Diff<Props, StateProps>, *>> { ... } |
This PR (#1887) contains some examples with Flowtype support and two libdefs (redux and react-redux). I'd love to hear some feedback (especially on the libdefs) |
Type-safety in reducer switch statements can be improved by explicitly declaring the action set and allowing TypeScript to narrow down the contents of each action based on the `type` constant. See (e.g.) reduxjs/redux#290
See the example from this PR to Facebook's Flux:
Now that #254 is merged (into 1.0 branch), I encourage you to help me figure out how to have the same awesomeness in Redux. We probably need to make
createStore
definition generic so that you can saycreateStore<MyState, MyAction>(reducer, initialState)
and have your whole reducer tree type checked.I'm a Flow noob so any help is welcome. You can start by copying the Counter example, annotating it with Flow and trying to make it type check like in facebookarchive/flux#243.
The text was updated successfully, but these errors were encountered: