Skip to content
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

Closed
gaearon opened this issue Jul 19, 2015 · 43 comments
Closed

Explore using Flow for statically analyzing state and actions #290

gaearon opened this issue Jul 19, 2015 · 43 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Jul 19, 2015

See the example from this PR to Facebook's Flux:

type Action =
  {
    type: 'foo',
    foo: number,
  } |
  {
    type: 'bar',
    bar: string,
  };

var FooDispatcher: Dispatcher<Action> = new Dispatcher();

function acceptsNumber(x: number): void {}

FooDispatcher.register(function(action: Action) {
  switch (action.type) {
    case 'foo':
      acceptsNumber(action.foo);
      acceptsNumber(action.bar); // flow error, property not found in object
      break;

    case 'bar':
      acceptsNumber(action.bar); // flow error, string incompatible with number
      break;
  }
});

type NotAction = string;
FooDispatcher.register(function(action: NotAction) {}); // flow error

// flow error
FooDispatcher.dispatch({
  type: 'foo-typo',
  foo: 100,
});

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 say createStore<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.

@meoyawn
Copy link

meoyawn commented Jul 19, 2015

I don't know if there's a better way to represent algebraic data types in flow's type system
https://gist.github.com/hallettj/281e4080c7f4eaf98317

Also, to get a safe and exhaustive pattern matching you can use church encoding
https://apocalisp.wordpress.com/2009/08/21/structural-pattern-matching-in-java/

@meoyawn
Copy link

meoyawn commented Jul 19, 2015

oh, apparently there's a more idiomatic way of handling ADTs http://flowtype.org/blog/2015/07/03/Disjoint-Unions.html

@gaearon
Copy link
Contributor Author

gaearon commented Jul 21, 2015

I managed to use the Flow definitions from the Redux source code by using the following Flow config in my standalone example:

[ignore]
.*/node_modules/webpack/.*
.*/node_modules/webpack-dev-server/.*
.*/node_modules/redux/examples/.*
.*/node_modules/redux/lib/.*

[include]

[libs]

[options]
module.system=node
module.name_mapper='\(redux\)' -> '\1/src/index'

Note that this requires src to be in the NPM distribution (we'll be including it then).

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.

@leoasis
Copy link
Contributor

leoasis commented Jul 21, 2015

@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

@gaearon
Copy link
Contributor Author

gaearon commented Jul 21, 2015

I made some progress but bumped into this: facebook/flow#652.

@leoasis
Copy link
Contributor

leoasis commented Jul 26, 2015

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 createStore type check with a basic reducer and state (the counter example). The code is below. I've added two different properties on the INCREMENT and DECREMENT actions on purpose in order to verify that Flow is type checking correctly inside the switch case (it does after all!).

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:

  1. Correctly put the types in replaceReducer since that can change the reducer State and Action types, so it may not be correctly type checking if we just assign that reducer to the current one internally in the store. Perhaps this is a hint that we should instead return a new store with that new reducer?
  2. Be able to make this work among different files, especially with some types being in the redux library, which will have the types stripped after being transpiled with babel. Ideas: 1) transform those types into flow comments (https://github.com/babel-plugins/babel-plugin-flow-comments), 2) have a declarations file and a types file that can be required and included as a lib in flow (perhaps this declarations file could be extracted somehow from the types in the src?).
  3. Finish adding types for the utils functions.
  4. Add types for other packages such as redux-thunk?

@gaearon
Copy link
Contributor Author

gaearon commented Jul 26, 2015

This is a great starting point already, thank you!

have a declarations file and a types file that can be required and included as a lib in flow (perhaps this declarations file could be extracted somehow from the types in the src?).

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:

  • Use the config above as your .flowconfig
  • Manually put src into node_modules/redux (we'd need to remove src from .npmignore)
  • Export some types from index.js via export type declarations like the ones Remove Flow types for now #299 removed

@leoasis
Copy link
Contributor

leoasis commented Jul 26, 2015

Oh great @gaearon forgot about that above. Will try to move forward with that

@nmn
Copy link

nmn commented Jul 27, 2015

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:

type ActionStrings = 'ACTION_ONE'
                   | 'ACTION_TWO'
                   | 'ACTION_THREE'
                   | 'ACTION_FOUR'

export default var actions: {[key: ActionStrings]: ActionStrings} = {
  'ACTION_ONE': 'ACTION_ONE',
  'ACTION_TWO': 'ACTION_TWO',
  'ACTION_THREE': 'ACTION_THREE',
  'ACTION_FOUR': 'ACTION_FOUR'
}

Now when dispatching an action, if you use the constants, flow does error checking for you.

import actions from './actions'

dispatch(actions.ACTION_ONE) // will work
dispatch(actions.ACTIONS_1) // flow will throw an error

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.

@hallettj
Copy link

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.

@hallettj
Copy link

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 Action.

@leoasis
Copy link
Contributor

leoasis commented Sep 24, 2015

@hallettj thanks for jumping in to solve this problem in redux! Would really love to have this soon.
As per your example, it seems you have a lot of redundant type signatures specified (e.g: when composing the reducers for single actions). Does that still work if you remove those? I think trying to make Flow infer most types is a good thing we should strive to have in order to make using Flow with redux less cumbersome.

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 combineReducers, which is very hard because you need to specify the output type as something that is a transformation from the input type, which I think it's not supported in Flow (and I think not even in type systems from Elm or Haskell).

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.
So I think we only have the option to make the output have a more generic type, and then explicitly cast the result to the type we want. After all, the store should be configured and created in a single file in most apps, so it should be easy to verify that the cast is safe (I think)

@leoasis
Copy link
Contributor

leoasis commented Sep 24, 2015

Oh, and also waiting on this to be fixed in Flow: facebook/flow#582

@hallettj
Copy link

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 combineReducers: I have a thought regarding an internal Flow feature, $Keys<T>. $Keys<T> is the union of (usually string literal) types that make up the keys of an object T. Using this feature, a signature for combineReducers can be made that specifies that keys in the output object must also exist in the input object:

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 combineReducers as both an object and a map. That lets us refer to the set of keys of the input, as well as to a generic supertype of its values. Unfortunately, Flow does not infer the most specific possible type for U; so an implementation that just universally qualifies U behaves nearly the same:

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 U in both the input and output types might lead somewhere useful. The examples below work with either of the above implementations.

$Keys<T> at least ensures that any keys referenced in the combined reducers object also exist in the input reducer set. Here is what we get using this function relying on type inference, without any type annotations at the call site:

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 $Values<T> feature to complement $Keys<T>, that could be used to get better type inference in the absence of call-site annotations.

A caveat of $Keys<T> is it won't work if the keys of the input object cannot be inferred statically. I believe that in a case where a reducer set is computed dynamically, the typechecker will throw an error.

Another thought I have is that it would be much easer to check variations of combineReducers that produce and consume tuples instead of objects. There would have to be a few implementations with different arities:

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 combineReducers implementation above gets with a call-site annotation - except that combineN does not require the call-site annotation.

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.

@leoasis
Copy link
Contributor

leoasis commented Sep 25, 2015

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.
The solution with tuples, while it may work, will need to define a lot of combineN functions, since in most big enough apps you have a lot of reducers.

I'm ultimately leaning towards just manually creating the reducer, since combineReducers does mostly type and shape checking (both things will also be checked by Flow), and it just add a little convenience shortcut, but, it's not that bad either. Consider the plain solution without combineReducers:

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 combineReducers

@gaearon
Copy link
Contributor Author

gaearon commented Sep 25, 2015

Fair enough, we can do that. I'm also concerned about middleware—seems like adding even something simple like redux-thunk would confuse Flow, wouldn't it? You can now dispatch both objects of your action type and functions that can dispatch those actions or functions..

@leoasis
Copy link
Contributor

leoasis commented Sep 26, 2015

Perhaps we would have to have a version of applyMiddleware that only accepts one middleware at a time, so that we can correctly define the types. Perhaps the middleware would have an extra generic type that would be the Dispatchable<Action> type, which for stores could just be the Action, and in the case for redux-thunk, it would be Action | Thunk<State, Dispatchable<Action>>. If we specify the correct type for applyMiddleware, I think we could chain them in order to get a correct final store that has the correct Dispatchable type, and even ensure that the middleware order cannot have the thunk after the logger, for example

@gaearon
Copy link
Contributor Author

gaearon commented Sep 26, 2015

Perhaps we would have to have a version of applyMiddleware that only accepts one middleware at a time, so that we can correctly define the types.

Is it possible to define several overloads? e.g. can we somehow define 10 overloads for different number of parameters?

@leoasis
Copy link
Contributor

leoasis commented Sep 26, 2015

Just tried and with the declare module syntax it's possible to do that. There should be a way to do it with the type annotations as well

@hallettj
Copy link

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) { /* ... */ }

@leoasis
Copy link
Contributor

leoasis commented Sep 26, 2015

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

@leoasis
Copy link
Contributor

leoasis commented Sep 28, 2015

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

@donaldpipowitch
Copy link

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.

@gaearon
Copy link
Contributor Author

gaearon commented Oct 1, 2015

@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!

@leoasis
Copy link
Contributor

leoasis commented Oct 1, 2015

@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

@donaldpipowitch
Copy link

So Flow integration is/was just an experiment and there was nobody who did the same for TypeScript yet. Is this right?

@leoasis
Copy link
Contributor

leoasis commented Oct 2, 2015

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.

@donaldpipowitch
Copy link

Can Flow be used to get rid of React.PropTypes, too?

@gaearon
Copy link
Contributor Author

gaearon commented Oct 5, 2015

Can Flow be used to get rid of React.PropTypes, too?

That was one of the motivations for it in React team so yes.

@cdebotton
Copy link

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

@tomduncalf
Copy link

@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 any type (either out of laziness or out of not knowing the correct type yet) and so it couldn't type check it - and again I have to repeat, the IDE support (I'm using Atom) is incredible - needless to say, both of these are a revelation coming from plain Javascript!

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!

@donaldpipowitch
Copy link

so my experiences might be of interest.

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 :(

@tomduncalf
Copy link

@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, virtual-dom just uses standard JS functions rather than a new syntax like JSX, so you should just need to download or create a typing file to get it working.

@donaldpipowitch
Copy link

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 :/

@tonyxiao
Copy link

tonyxiao commented Dec 3, 2015

Does this new flow release help? https://github.com/facebook/flow/releases/tag/v0.19.0

@karelbilek
Copy link

This issue discussion is giant. Is there any one file with the latest flow definition file for redux?

@karelbilek
Copy link

Oh. It seems that you used Flow types internally, but then removed them :( too bad

@nmn
Copy link

nmn commented Jan 3, 2016

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.

@fshowalter
Copy link
Contributor

@gaearon, given @nmn's comment above, is there still interest in annotating counter example as a starting point? If so, I'd be willing to give it a shot, as I've wanted an excuse to try out Flow.

@nmn
Copy link

nmn commented Mar 10, 2016

i have an open pull request to fix the flow-comments plugin.

@STRML
Copy link

STRML commented Apr 21, 2016

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 connect() and those that are not, so while it can validate the types of props, it cannot validate if props are missing. It does, however, validate the type of the root store, and the return value of mapStateToProps.

I don't use mapDispatchToProps and mergeProps so they are missing.

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>, *>> { ... }

@gcanti
Copy link
Contributor

gcanti commented Aug 9, 2016

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)

rjz added a commit to rjz/typescript-react-redux that referenced this issue Sep 24, 2016
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
@timdorr timdorr closed this as completed Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests