stylesuxx / generator-react-webpack-redux

React Webpack Generator including Redux support
Other
552 stars 85 forks source link

Include redux-thunk? #73

Open MarcoScabbiolo opened 7 years ago

MarcoScabbiolo commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

Hey @MarcoScabbiolo yes I think those are fair points

+1

stylesuxx commented 7 years ago

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 commented 7 years ago

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

umair-khanzada commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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