6 ms·

This is disappointing. Partially for bikeshed reasons, but also for practical ones. Redux thunk is an anti pattern as far as I'm concerned but you're including

This is disappointing. Partially for bikeshed reasons, but also for practical ones. Redux thunk is an anti pattern as far as I'm concerned but you're including it by default? Why? Just because it's the first (bad) decision people make when trying to handle async code doesn't mean it's the one that should be canonized. The createAction function is unnecessarily verbose and the createReducer function obscures more than it abstracts.

acemarke 7y ago
I find your concerns very confusing. In order: Thunks are the most widely used middleware by a large margin [0], and we have frequently received complaints that there is no async middleware included in the Redux store by default. Thunks were in fact part of the store during the original development phase [1], but were made separate once the middleware API was defined [2]. Thunks are effectively the minimum viable way to write async logic that has access to interacting with the Redux store, so we include that in RTK out of the box to make that possible. Sagas, observables, and other similar middleware are far too heavyweight and complex for us to endorse as the default async approach. I also strongly disagree that thunks are an "anti-pattern", and find them to be a highly useful tool (and especially so now that `async/await` syntax makes it even easier to write more complicated async logic in a readable form) [3]. You are always welcome to add another middleware or disable thunks as part of the store setup process instead if you want. I don't see how this: const addTodo = createAction("todos/add") is more verbose than this: const addTodo = (text) => { return { type: "todos/add", payload: text } } We've had a version of `createReducer` shown in the "Reducing Boilerplate" docs page [4] for years, and there are literally hundreds of similar "lookup table"-type utilities out there. Including this in RTK is a no-brainer. It's even more valuable, because ours uses Immer inside to let you write "mutative" immutable update logic instead of having to nest spread operators. I don't see how this is "obscuring" anything. Finally, our recommendation is that most of the time you shouldn't even be calling `createAction` and `createReducer` yourself. Instead, you should be using the `createSlice` function [5], which automatically generates action creators and action types for you, and calls the other two functions internally. [0] Slide 38 in https://blog.isquaredsoftware.com/2017/09/presentation-might-need-redux-ecosystem/ https://blog.isquaredsoftware.com/2017/09/presentation-might... [1] https://github.com/reduxjs/redux/issues/1 https://github.com/reduxjs/redux/issues/1 [2] https://github.com/reduxjs/redux/pull/63#issuecomment-110575809 https://github.com/reduxjs/redux/pull/63#issuecomment-110575... [3] https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/ https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-th... [4] https://redux.js.org/recipes/reducing-boilerplate#generating-reducers https://redux.js.org/recipes/reducing-boilerplate#generating... [5] https://redux-toolkit.js.org/api/createSlice https://redux-toolkit.js.org/api/createSlice
- weeksie 7y ago
  Async middleware is an anti-pattern. Take care of async stuff in mapDispatchToProps. Actions are ALWAYS action(payload) => { type: action, payload } There's no reason to call a function everywhere you need a new action. Just define all of them in one place by listing the types. You can easily write a function that looks like createActions(['TODOS/ADD', 'TODOS/DELETE', ... ]}) That returns all your action creators and types in one place. Import them as needed. The above single call function to create a todo action is verbose without achieving any efficiency. It's bad design. Don't get rid of the switch statements at all. Use them. They're clear. If you need to slice things up, just use child reducers in the switch branches. Lookup tables are less performant and don't buy you anything. Sorry to be so harsh but this kind of cargo-cult best practice stuff is how you end up with Javascript that feels like Enterprise Java™ from back in the 00s.
  - acemarke 7y ago
    Yeah, we'll have to disagree here. We officially recommend consistently using action creators [0], and I talked about the reasons to use them in a blog post [1]. Switch statements and lookups are almost completely equivalent (except for fall-through and default case handling) [2], and "performance" is irrelevant here. In this case, use of `createSlice` means that writing the lookup table object generates the resulting action types and action creators for free, which drastically reduces the amount of code you have to write. I am strongly against defining async logic inside of `mapDispatch`. It's similar to writing thunks in the first place (an arbitrary function that has access to `dispatch`), but it's less testable, not reusable, _and_ doesn't have access to `getState`. In fact, you should be using the "object shorthand" form of `mapDispatch` [3] to pass action creators into connected components, like `const mapDispatch = {addTodo, toggleTodo}`. The advice to put async logic in `mapDispatch` also does not help if you're using our new React-Redux hooks API [4], where you just have `const dispatch = useDispatch()`. The Redux Toolkit API is specifically designed to remove as much as possible of that incidental complexity from your Redux code, and move it _away_ from being that "Enterprise Java" feel. [0] https://redux.js.org/style-guide/style-guide#use-action-creators https://redux.js.org/style-guide/style-guide#use-action-crea... [1] https://blog.isquaredsoftware.com/2016/10/idiomatic-redux-why-use-action-creators/ https://blog.isquaredsoftware.com/2016/10/idiomatic-redux-wh... [2] https://blog.isquaredsoftware.com/2017/05/idiomatic-redux-tao-of-redux-part-2/#switch-statements https://blog.isquaredsoftware.com/2017/05/idiomatic-redux-ta... [3] https://redux.js.org/style-guide/style-guide#use-the-object-shorthand-form-of-mapdispatch-with-connect https://redux.js.org/style-guide/style-guide#use-the-object-... [4] https://react-redux.js.org/api/hooks https://react-redux.js.org/api/hooks
code_biologist 7y ago
What solution do you use for async Redux code?
- weeksie 7y ago
  Handle async stuff in mapDispatchToProps. dispatch => ({ async onRequestFnord(arg) { dispatch(actions.fnordRequested()); const result = await service.fnord(arg); dispatch(actions.fnordReceived(result); } }); Or something to that effect.
  - acemarke 7y ago
    Which is 95% identical to a thunk syntactically, except that you've limited how that logic can be tested and reused. There are valid arguments against using thunks [0], but all you've done is put the exact same logic in a different form. Hardly an argument against using them. All you have to do is change it to: const onRequestFnord = (arg) => async dispatch => { dispatch(actions.fnordRequested()); const result = await service.fnord(arg); dispatch(actions.fnordReceived(result); } // later, in the component const mapDispatch = {onRequestFnord}; At least thunks also give you access to `getState`, and if you need it, the ability to inject an "extra argument" that might contain something like a service reference that can be mocked for testing. [0] https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/ https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-th...
    - weeksie 7y ago
      Except you've added a library. And the function is no less testable than the thunk. I fail to see why adding access to the entire application state is an advantage. This is just cargo culting. In fact that you held up that counter example as a preferable practice tells me everything I need to know about your library.
      - r_p4rk 7y ago
        If you feel so bad about including a "library", the redux-thunk middleware is trivial to implement yourself in about 6 lines of code.
woah 7y ago
The huge bikeshed thread in the replies to this comment is why I’m glad I don’t use Redux any more. It’s an event framework that doesn’t handle asynchrony? Why? Redux makes you write a bunch of boilerplate to do the easy stuff, then doesn’t give you any tools to do the hard stuff.
- acemarke 7y ago
  As I've said elsewhere in the thread, the Redux core is deliberately unopinionated. Redux Toolkit is our opinionated wrapper around that core that provides the best defaults for common cases and removes that "boilerplate": https://redux-toolkit.js.org https://redux-toolkit.js.org

acemarke 7y ago

I find your concerns very confusing. In order: Thunks are the most widely used middleware by a large margin [0], and we have frequently received complaints that there is no async middleware included in the Redux store by default. Thunks were in fact part of the store during the original development phase [1], but were made separate once the middleware API was defined [2]. Thunks are effectively the minimum viable way to write async logic that has access to interacting with the Redux store, so we include that in RTK out of the box to make that possible. Sagas, observables, and other similar middleware are far too heavyweight and complex for us to endorse as the default async approach. I also strongly disagree that thunks are an "anti-pattern", and find them to be a highly useful tool (and especially so now that `async/await` syntax makes it even easier to write more complicated async logic in a readable form) [3]. You are always welcome to add another middleware or disable thunks as part of the store setup process instead if you want. I don't see how this: const addTodo = createAction("todos/add") is more verbose than this: const addTodo = (text) => { return { type: "todos/add", payload: text } } We've had a version of `createReducer` shown in the "Reducing Boilerplate" docs page [4] for years, and there are literally hundreds of similar "lookup table"-type utilities out there. Including this in RTK is a no-brainer. It's even more valuable, because ours uses Immer inside to let you write "mutative" immutable update logic instead of having to nest spread operators. I don't see how this is "obscuring" anything. Finally, our recommendation is that most of the time you shouldn't even be calling `createAction` and `createReducer` yourself. Instead, you should be using the `createSlice` function [5], which automatically generates action creators and action types for you, and calls the other two functions internally. [0] Slide 38 in https://blog.isquaredsoftware.com/2017/09/presentation-might-need-redux-ecosystem/ https://blog.isquaredsoftware.com/2017/09/presentation-might... [1] https://github.com/reduxjs/redux/issues/1 https://github.com/reduxjs/redux/issues/1 [2] https://github.com/reduxjs/redux/pull/63#issuecomment-110575809 https://github.com/reduxjs/redux/pull/63#issuecomment-110575... [3] https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/ https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-th... [4] https://redux.js.org/recipes/reducing-boilerplate#generating-reducers https://redux.js.org/recipes/reducing-boilerplate#generating... [5] https://redux-toolkit.js.org/api/createSlice https://redux-toolkit.js.org/api/createSlice

weeksie 7y ago
Async middleware is an anti-pattern. Take care of async stuff in mapDispatchToProps. Actions are ALWAYS action(payload) => { type: action, payload } There's no reason to call a function everywhere you need a new action. Just define all of them in one place by listing the types. You can easily write a function that looks like createActions(['TODOS/ADD', 'TODOS/DELETE', ... ]}) That returns all your action creators and types in one place. Import them as needed. The above single call function to create a todo action is verbose without achieving any efficiency. It's bad design. Don't get rid of the switch statements at all. Use them. They're clear. If you need to slice things up, just use child reducers in the switch branches. Lookup tables are less performant and don't buy you anything. Sorry to be so harsh but this kind of cargo-cult best practice stuff is how you end up with Javascript that feels like Enterprise Java™ from back in the 00s.
- acemarke 7y ago
  Yeah, we'll have to disagree here. We officially recommend consistently using action creators [0], and I talked about the reasons to use them in a blog post [1]. Switch statements and lookups are almost completely equivalent (except for fall-through and default case handling) [2], and "performance" is irrelevant here. In this case, use of `createSlice` means that writing the lookup table object generates the resulting action types and action creators for free, which drastically reduces the amount of code you have to write. I am strongly against defining async logic inside of `mapDispatch`. It's similar to writing thunks in the first place (an arbitrary function that has access to `dispatch`), but it's less testable, not reusable, _and_ doesn't have access to `getState`. In fact, you should be using the "object shorthand" form of `mapDispatch` [3] to pass action creators into connected components, like `const mapDispatch = {addTodo, toggleTodo}`. The advice to put async logic in `mapDispatch` also does not help if you're using our new React-Redux hooks API [4], where you just have `const dispatch = useDispatch()`. The Redux Toolkit API is specifically designed to remove as much as possible of that incidental complexity from your Redux code, and move it _away_ from being that "Enterprise Java" feel. [0] https://redux.js.org/style-guide/style-guide#use-action-creators https://redux.js.org/style-guide/style-guide#use-action-crea... [1] https://blog.isquaredsoftware.com/2016/10/idiomatic-redux-why-use-action-creators/ https://blog.isquaredsoftware.com/2016/10/idiomatic-redux-wh... [2] https://blog.isquaredsoftware.com/2017/05/idiomatic-redux-tao-of-redux-part-2/#switch-statements https://blog.isquaredsoftware.com/2017/05/idiomatic-redux-ta... [3] https://redux.js.org/style-guide/style-guide#use-the-object-shorthand-form-of-mapdispatch-with-connect https://redux.js.org/style-guide/style-guide#use-the-object-... [4] https://react-redux.js.org/api/hooks https://react-redux.js.org/api/hooks

code_biologist 7y ago

What solution do you use for async Redux code?

weeksie 7y ago
Handle async stuff in mapDispatchToProps. dispatch => ({ async onRequestFnord(arg) { dispatch(actions.fnordRequested()); const result = await service.fnord(arg); dispatch(actions.fnordReceived(result); } }); Or something to that effect.
- acemarke 7y ago
  Which is 95% identical to a thunk syntactically, except that you've limited how that logic can be tested and reused. There are valid arguments against using thunks [0], but all you've done is put the exact same logic in a different form. Hardly an argument against using them. All you have to do is change it to: const onRequestFnord = (arg) => async dispatch => { dispatch(actions.fnordRequested()); const result = await service.fnord(arg); dispatch(actions.fnordReceived(result); } // later, in the component const mapDispatch = {onRequestFnord}; At least thunks also give you access to `getState`, and if you need it, the ability to inject an "extra argument" that might contain something like a service reference that can be mocked for testing. [0] https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/ https://blog.isquaredsoftware.com/2017/01/idiomatic-redux-th...
  - weeksie 7y ago
    Except you've added a library. And the function is no less testable than the thunk. I fail to see why adding access to the entire application state is an advantage. This is just cargo culting. In fact that you held up that counter example as a preferable practice tells me everything I need to know about your library.
    - r_p4rk 7y ago
      If you feel so bad about including a "library", the redux-thunk middleware is trivial to implement yourself in about 6 lines of code.

woah 7y ago

The huge bikeshed thread in the replies to this comment is why I’m glad I don’t use Redux any more. It’s an event framework that doesn’t handle asynchrony? Why? Redux makes you write a bunch of boilerplate to do the easy stuff, then doesn’t give you any tools to do the hard stuff.

acemarke 7y ago
As I've said elsewhere in the thread, the Redux core is deliberately unopinionated. Redux Toolkit is our opinionated wrapper around that core that provides the best defaults for common cases and removes that "boilerplate": https://redux-toolkit.js.org https://redux-toolkit.js.org