Skip to content

Remove Flow types for now #299

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

Merged
merged 1 commit into from
Jul 22, 2015
Merged

Remove Flow types for now #299

merged 1 commit into from
Jul 22, 2015

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jul 22, 2015

I decided in favor of not shipping Flow types in 1.0.
This commit just strips types from the current Flow-friendly codebase (initially worked on in #254).

We will not use features unsupported by Flow (such as let or const) from now on so it's easy to gradually add Flow types later.

The reason I'm removing them is they're too naive and I don't want them to be perceived as the public API (at least not yet). I am ready to add Flow when cases like this are solved. Adding them now is confusing because, for #290 to work, different (more polymorphic) signatures are needed. For example, Store type should be parameterized by <MyState, MyAction> instead of any types in the current annotations.

gaearon added a commit that referenced this pull request Jul 22, 2015
@gaearon gaearon merged commit 305f142 into breaking-changes-1.0 Jul 22, 2015
@gaearon gaearon deleted the remove-flow branch July 22, 2015 16:23
@gaearon
Copy link
Contributor Author

gaearon commented Jul 22, 2015

cc @acdlite

@yanivtal
Copy link

Aww I was looking forward to this. Btw let/const should be in soon facebook/flow#431

@gaearon
Copy link
Contributor Author

gaearon commented Jul 22, 2015

Haha. Help make it happen with the correct signatures :-)
I got stuck to be honest, but I really want to get #290 to work!

@leoasis
Copy link
Contributor

leoasis commented Jul 22, 2015

@gaearon do you have any branch you could publish with your progress there? If you have done any significant progress it would be nice to start from there

@gaearon
Copy link
Contributor Author

gaearon commented Jul 22, 2015

@leoasis

I had one but it descended into pointlessness so it would be easier for you to start fresh.
Some advice:

  • Concentrate on createStore, don't add annotations anywhere else, this will only confuse you.
  • Make it return Store<State, Action> where State and Action types are user specified.
  • You'll have a problem with the dispatch of initial action because it won't match the user's Action type. Good luck there. :-P Maybe it's easiest to change signature to createStore<State, Action>(reducer: Reducer<State, Action>, initialState: State, initialAction: Action) just to get it to type check for now, and then figure out something that makes sense..

@gaearon
Copy link
Contributor Author

gaearon commented Jul 22, 2015

The goal is to get #290 working at least in the minimal way of type checking action/state type validity with createStore. Then figure out how to get the rest of the API working. Middleware and higher order stores might get super tricky (if not impossible) because applyMiddleware(...) changes dispatch type with each middleware. Don't go there until you figure out the basics..

@leoasis
Copy link
Contributor

leoasis commented Jul 22, 2015

Cool, will give it a good go tomorrow, thanks for the advice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants