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

Include redux-thunk? #73

Open
MarcoScabbiolo opened this issue Mar 22, 2017 · 8 comments
Open

Include redux-thunk? #73

MarcoScabbiolo opened this issue Mar 22, 2017 · 8 comments

Comments

@MarcoScabbiolo
Copy link

MarcoScabbiolo commented Mar 22, 2017

Given that it is essential if you want to use asynchronous operations in actions, I think it would be nice to include redux-thunk. Maybe it's just me, but I end up adding it to every proyect. It is a very small dependency and as a middleware it's not invasive at all.

On top of including redux-thunk as a dependency, these would be the changes needed:

At generators/root/templates/store.js:

Add

import thunk from 'redux-thunk';

Change
import { createStore } from 'redux';
To
import { createStore, compose, applyMiddleware } from 'redux';

And change:

const store = createStore(reducers, initialState,
    window.devToolsExtension && window.devToolsExtension());

To:

const store = createStore(reducers, initialState, compose(
  applyMiddleware(thunk),
  window.devToolsExtension ? window.devToolsExtension() : f => f
));

It could be implemented optionally with a prompt.
What do you think? I can make the pull request if you're ok with this request.

@andytango
Copy link

Hey,

I'm not sure I agree that it's essential. I use redux-saga, and at a pinch you could get away with just using promises and closures.

I have thought of making a generator that creates sagas in the same style that this generator does for containers etc, but just putting it out as a normal yeoman generator.

Perhaps you could do the same with redux-thunk?

@MarcoScabbiolo
Copy link
Author

MarcoScabbiolo commented Mar 22, 2017

@andytango The difference with redux-saga and redux-thunk is that the latter is very straight forward and no additional sub-generators would be needed to take full advantage of it, all you need is to import it as a middleware. Maybe a prompt "Would you like to use redux-thunk" that defaults to "no" could do the trick. It's not that redux-thunk is not widely used, it is mentioned in the official Redux docs.

@andytango
Copy link

Hey @MarcoScabbiolo yes I think those are fair points

+1

@MarcoScabbiolo MarcoScabbiolo changed the title Include redux-thunk by default? Include redux-thunk? Mar 22, 2017
@stylesuxx
Copy link
Owner

In fact this has been on my mind for quite some time. During setup as @MarcoScabbiolo describes.

Will see what I can do.

@MarcoScabbiolo if you by any chance already built that, feel free to submit a PR ;-)

@MarcoScabbiolo
Copy link
Author

@stylesuxx I don't have it ready yet, but I have some experiencie with generators and Inquirer, I might be able to get it done this weekend, If you are not already doing it let me know and i'll submit the PR when it's ready.

MarcoScabbiolo added a commit to MarcoScabbiolo/generator-react-webpack-redux that referenced this issue Mar 24, 2017
@umair-khanzada
Copy link

@MarcoScabbiolo, @stylesuxx I think we didn't need thunk in this generator, because in this generator we are using bindActionCreators and according to documentation bindActionCreators wrapped into a dispatch call so they may be invoked directly.
am I right ?

@MarcoScabbiolo
Copy link
Author

@umair-khanzada bindActionCreators calls your action creators forwarding the arguments and passes the resulting action to dispatch, it doesn't pass dispatch to the action creator to turn it into a thunk, for that you need redux-thunk or redux-saga or redux-loop. Unless there is something else going on in bindActionCreators but the documentation doesn't say anything about that.

@umair-khanzada
Copy link

@MarcoScabbiolo Yeah you are right I was confused, both are different concept, bindActionCreators simply wrap action into dispatch.

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

4 participants