Trying to put API calls in the correct place

[5h1rU]

I'm trying to make a login success/error flow but my main concern is where I can put this logic.

Currently I'm using actions -> reducer (switch case with action calling API) -> success/error on response triggering another action.

The problem with this approach is that the reducer is not working when I call the action from the API call.

am I missing something?

Reducer

import { LOGIN_ATTEMPT, LOGGED_FAILED, LOGGED_SUCCESSFULLY } from '../constants/LoginActionTypes';
import Immutable from 'immutable';
import LoginApiCall from '../utils/login-request';

const initialState = new Immutable.Map({
  email: '',
  password: '',
}).asMutable();

export default function user(state = initialState, action) {
  switch (action.type) {
    case LOGIN_ATTEMPT:
      console.log(action.user);
      LoginApiCall.login(action.user);
      return state;
    case LOGGED_FAILED:
      console.log('failed from reducer');
      return state;
    case LOGGED_SUCCESSFULLY:
      console.log('success', action);
      console.log('success from reducer');
      break;
    default:
      return state;
  }
}

actions

import { LOGIN_ATTEMPT, LOGGED_FAILED, LOGGED_SUCCESSFULLY } from '../constants/LoginActionTypes';

export function loginError(error) {
  return dispatch => {
    dispatch({ error, type: LOGGED_FAILED });
  };
}

/*
 * Should add the route like parameter in this method
*/
export function loginSuccess(response) {
  return dispatch => {
    dispatch({ response, type: LOGGED_SUCCESSFULLY });
    // router.transitionTo('/dashboard'); // will fire CHANGE_ROUTE in its change handler
  };
}

export function loginRequest(email, password) {
  const user = {email: email, password: password};
  return dispatch => {
    dispatch({ user, type: LOGIN_ATTEMPT });
  };
}

API calls

 // Use there fetch polyfill
 // The main idea is create a helper in order to handle success/error status
import * as LoginActions from '../actions/LoginActions';

const LoginApiCall = {
  login(userData) {
    fetch('http://localhost/login', {
      method: 'post',
      headers: {
        'Accept': 'application/json',
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        email: userData.email,
        password: userData.password,
      }),
    })
    .then(response => {
      if (response.status >= 200 && response.status < 300) {
        console.log(response);
        LoginActions.loginSuccess(response);
      } else {
        const error = new Error(response.statusText);
        error.response = response;
        LoginActions.loginError();
        throw error;
      }
    })
    .catch(error => { console.log('request failed', error); });
  },
};

export default LoginApiCall;

[gaearon]

This is almost correct, but the problem is you can't just call pure action creators and expect things to happen. Don't forget your action creators are just functions that specify what needs to be dispatched.

// CounterActions
export function increment() {
  return { type: INCREMENT }
}


// Some other file
import { increment } from './CounterActions';
store.dispatch(increment()); <--- This will work assuming you have a reference to the Store
increment(); <---- THIS DOESN'T DO ANYTHING! You're just calling your function and ignoring result.


// SomeComponent
import { increment } from './CounterActions';

@connect(state => state.counter) // will inject store's dispatch into props
class SomeComponent {
  render() {
    return <OtherComponent {...bindActionCreators(CounterActions, this.props.dispatch)} />
  }
}


// OtherComponent
class OtherComponent {
  handleClick() {
    // this is correct:
    this.props.increment(); // <---- it was bound to dispatch in SomeComponent

    // THIS DOESN'T DO ANYTHING:
    CounterActions.increment(); // <---- it's just your functions as is! it's not bound to the Store.
  }
}

[gaearon]

Now, let's get to your example. The first thing I want to clarify is you don't need async dispatch => {} form if you only dispatch a single action synchronously and don't have side effects (true for loginError and loginRequest).

This:

import { LOGIN_ATTEMPT, LOGGED_FAILED, LOGGED_SUCCESSFULLY } from '../constants/LoginActionTypes';

export function loginError(error) {
  return dispatch => {
    dispatch({ error, type: LOGGED_FAILED });
  };
}

export function loginSuccess(response) {
  return dispatch => {
    dispatch({ response, type: LOGGED_SUCCESSFULLY });
    // router.transitionTo('/dashboard');
  };
}

export function loginRequest(email, password) {
  const user = {email: email, password: password};
  return dispatch => {
    dispatch({ user, type: LOGIN_ATTEMPT });
  };
}

can be simplified as

import { LOGIN_ATTEMPT, LOGGED_FAILED, LOGGED_SUCCESSFULLY } from '../constants/LoginActionTypes';

export function loginError(error) {
  return { error, type: LOGGED_FAILED };
}

// You'll have a side effect here so (dispatch) => {} form is a good idea
export function loginSuccess(response) {
  return dispatch => {
    dispatch({ response, type: LOGGED_SUCCESSFULLY });
    // router.transitionTo('/dashboard');
  };
}

export function loginRequest(email, password) {
  const user = {email: email, password: password};
  return { user, type: LOGIN_ATTEMPT };
}

[gaearon]

Secondly, your reducers are supposed to be pure functions that don't have side effects. Do not attempt to call your API from a reducer. Reducer is synchronous and passive, it only modifies the state.

This:

const initialState = new Immutable.Map({
  email: '',
  password: '',
  isLoggingIn: false,
  isLoggedIn: false,
  error: null
}).asMutable(); // <---------------------- why asMutable?

export default function user(state = initialState, action) {
  switch (action.type) {
    case LOGIN_ATTEMPT:
      console.log(action.user);
      LoginApiCall.login(action.user); // <------------------------ no side effects in reducers! :-(
      return state;
    case LOGGED_FAILED:
      console.log('failed from reducer');
      return state;
    case LOGGED_SUCCESSFULLY:
      console.log('success', action);
      console.log('success from reducer');
      break;
    default:
      return state;
  }

should probably look more like

const initialState = new Immutable.Map({
  email: '',
  password: '',
  isLoggingIn: false,
  isLoggedIn: false,
  error: null
});

export default function user(state = initialState, action) {
  switch (action.type) {
    case LOGIN_ATTEMPT:
      return state.merge({
        isLoggingIn: true,
        isLoggedIn: false,
        email: action.email,
        password: action.password // Note you shouldn't store user's password in real apps
      });
    case LOGGED_FAILED:
      return state.merge({
        error: action.error,
        isLoggingIn: false,
        isLoggedIn: false
      });
    case LOGGED_SUCCESSFULLY:
      return state.merge({
        error: null,
        isLoggingIn: false,
        isLoggedIn: true
      });
      break;
    default:
      return state;
  }

[gaearon]

Finally, where do you put the login API call then?

This is precisely what dispatch => {} action creator is for. Side effects!

It's just another action creator. Put it together with other actions:

import { LOGIN_ATTEMPT, LOGGED_FAILED, LOGGED_SUCCESSFULLY } from '../constants/LoginActionTypes';

export function loginError(error) {
  return { error, type: LOGGED_FAILED };
}

export function loginSuccess(response) {
  return dispatch => {
    dispatch({ response, type: LOGGED_SUCCESSFULLY });
    router.transitionTo('/dashboard');
  };
}

export function loginRequest(email, password) {
  const user = {email: email, password: password};
  return { user, type: LOGIN_ATTEMPT };
}

export function login(userData) {
  return dispatch =>
    fetch('http://localhost/login', {
      method: 'post',
      headers: {
        'Accept': 'application/json',
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        email: userData.email,
        password: userData.password,
      }),
    })
    .then(response => {
      if (response.status >= 200 && response.status < 300) {
        console.log(response);
        dispatch(loginSuccess(response));
      } else {
        const error = new Error(response.statusText);
        error.response = response;
        dispatch(loginError(error));
        throw error;
      }
    })
    .catch(error => { console.log('request failed', error); });
}

In your components, just call

this.props.login(); // assuming it was bound with bindActionCreators before

// --or--

this.props.dispatch(login()); // assuming you only have dispatch from Connector

[gaearon]

Finally, if you find yourself often writing large action creators like this, it's a good idea to write a custom middleware for async calls compatible with promises are whatever you use for async.

The technique I described above (action creators with dispatch => {} signature) is right now included in Redux, but in 1.0 will be available only as a separate package called redux-thunk. While you're at it, you might as well check out redux-promise-middleware or redux-promise.

[dariocravero]

@gaearon 👏 that was an amazing explanation! ;) It should definitely make it into the docs.

[andreychev]

@gaearon awesome explanation! 🏆

[tomkis]

@gaearon What if you need to perform some logic to determine which API call should be called? Isn't it domain logic which should be in one place(reducers)? Keeping that in Action creators sounds to me like breaking single source of truth. Besides, mostly you need to parametrise the API call by some value from the application state. You most likely also want to test the logic somehow. We found making API calls in Reducers (atomic flux) very helpful and testable in large scale project.

[gaearon]

We found making API calls in Reducers (atomic flux) very helpful and testable in large scale project.

This breaks record/replay. Functions with side effects are harder to test than pure functions by definition. You can use Redux like this but it's completely against its design. :-)

Keeping that in Action creators sounds to me like breaking single source of truth.

“Single source of truth” means data lives in one place, and has no independent copies. It doesn't mean “all domain logic should be in one place”.

What if you need to perform some logic to determine which API call should be called? Isn't it domain logic which should be in one place(reducers)?

Reducers specify how state is transformed by actions. They shouldn't worry about where these actions originate. They may come from components, action creators, recorded serialized session, etc. This is the beauty of the concept of using actions.

Any API calls (or logic that determines which API is called) happens before reducers. This is why Redux supports middleware. Thunk middleware described above lets you use conditionals and even read from the state:

// Simple pure action creator
function loginFailure(error) {
  return { type: LOGIN_FAILURE, error };
}

// Another simple pure action creator
function loginSuccess(userId) {
  return { type: LOGIN_SUCCESS, userId };
}

// Another simple pure action creator
function logout() {
  return { type: LOGOUT };  
}


// Side effect: uses thunk middleware
function login() {
  return dispatch => {
    MyAwesomeAPI.performLogin().then(
      json => dispatch(loginSuccess(json.userId)),
      error => dispatch(loginFailure(error))
    )
  };
}


// Side effect *and* reads state
function toggleLoginState() {
  return (dispatch, getState) => {
    const { isLoggedIn } = getState().loginState;
    if (isLoggedIn) {
      dispatch(login());
    } else {
      dispatch(logout());
    }
  };
}

// Component
this.props.toggleLoginState(); // Doesn't care how it happens

Action creators and middleware are designed to perform side effects and complement each other.
Reducers are just state machines and have nothing to do with async.

[tomkis]

Reducers specify how state is transformed by actions.

This is indeed very valid point, thanks.

[gaearon]

I'll reopen for posterity until a version of this is in the docs.

[suhaotian]

👍

[vladar]

I would argue that while reducers are state machines, so are action creators in general (but implicitly).

Say user clicked "submit" button twice. First time you will send HTTP request to the server (in the action creator) and change state to "submitting" (in the reducer). Second time you do not whant to do that.

In current model your action creator will have to choose what actions to dispatch depending on current state. If it is not currently "submitting" - then send HTTP request and dispatch one action, otherwise - just do nothing or even dispatch different action (like warning).

So your control flow is effectively split into two state machines and one of them is implicit and full of side effects.

I'd say that this Flux approach handles the majority of use-cases of typical web app very well. But when you have complex control logic it may become problematic. All of existing examples just do not have complex control logic, so this problem is somewhat hidden.

https://github.com/Day8/re-frame is an example of different approach where they have single event handler which acts as the only FSM.

But then reducers have side effects. So it is interesting how they deal with "replay" feature in such cases - asked them in day8/re-frame#86

In general, I think this is a real problem and it's not a surprise that it appears again and again. It's interesting to see what solutions will emerge eventually.

[gaearon]

Keep us posted about re-frame and side effects!

[vladap]

In my book there are 3 state types in any decent Redux web app.

1. View components (React this.state). This should be avoided but sometimes I just want to implement some behavior which can be reused by just reusing a component independently of Redux.

2. App shared state, i. e. Redux.state. It is state a view layer is using to present an app to a user and view components use it to "communicate" between each other. And this state can be used to "communicate" between ActionCreators, Middlewares, views as they decisions can depend on this state. Hence the "shared" is important. It doesn't have to be a complete app state though. Or I think it is impractical to implement everything as this type of state (in terms of actions).

3. State which is held by other side-effecting complex modules/libs/services. I would write these to handle scenarios vladar is describing. Or react-router is another example. You can treat it as a black-box which has its own state or you can decide to lift part of react-router state into app shared state (Redux.state). If I would write a complex HTTP module which would cleverly handle all my requests and its timings, I'm usually not interested in request timing details in Redux.state. I would only use this module from ActionCreators/Middlewares, possibly together with Redux.state to get new Redux.state.

If I would like to write view component showing my request's timings I would have to get this state to Redux.state.

@vladar Is it what you mean that AC/MW are implicit state machines? That because they don't held state by itself but they are still dependent on state held somewhere else and that they can define control logic how this states evolve in time? For some cases, I think they still could be implemented as closures and held its own state, becoming explicit state machines?

Alternatively I could call Redux.state a "public state", while other states are private. How to design my app is an act to decide what to keep as a private state and what as a public state. It seems to me like nice opportunity for encapsulation, hence I don't see it problematic to have state divided into different places as far as it doesn't become a hell how they affect each other.

[vladap]

@vladar

But then reducers have side effects. So it is interesting how they deal with "replay" feature in such cases

To achieve easy replay the code has to be deterministic. It is what Redux achieves by requiring pure reducers. In an effect Redux.state divides app into non-deterministic (async) and deterministic parts. You can replay bahavior above Redux.state assuming you don't do crazy thing in view components. The first important goal to achieve deterministic code is to move async code away and translate it into synchronous code via action log. It is what Flux architecture does in general. But in general it is not enough and other side-effecting or mutating code can still break deterministic processing.

Achieving replay-ability with side-effecting reducers is imho either impractically hard, even impossible or it will work only partially with many corner cases with probably a little practical effect.

[tomkis]

To achieve easy timetravel we store snapshots of app state instead of replaying actions (which is always hard) as done in Redux.

[gaearon]

To achieve easy timetravel we store snapshots of app state instead of replaying actions (which is always hard) as done in Redux.

Redux DevTools store both snapshots and actions. If you don't have actions, you can't hot reload the reducers. It's the most powerful feature of the workflow that DevTools enable.

Time travel works fine with impure action creators because it happens on the level of the dispatched (final) raw actions. These are plain objects so they're completely deterministic. Yes, you can't “rollback” an API call, but I don't see a problem with that. You can rollback the raw actions emitted by it.

[vladar]

That because they don't held state by itself but they are still dependent on state held somewhere else and that they can define control logic how this states evolve in time?

Yeah, that's exactly what I mean. But you may also end up with duplication of your control logic. Some code to demonstrate the problem (from my example above with double submit click).

function submit(data) {
  return (dispatch, getState) => {
    const { isSubmitting } = getState().isSubmitting;
    if (!isSubmitting) {
      dispatch(started(data));

      MyAPI.submit(data).then(
        json => dispatch(success(json)),
        error => dispatch(failure(error))
      )
    }
  }
}

function started(data) {
   return { type: SUBMIT_STARTED, data };
}

function success(result) {
  return { type: SUBMIT_SUCCESS, result };
}

function failure(error) {
  return { type: SUBMIT_FAILURE, error };
}

And then you have your reducer to check "isSubmitting" state again

const initialState = new Immutable.Map({
  isSubmitting: false,
  pendingData: null,
  error: null
});

export default function form(state = initialState, action) {
  switch (action.type) {
    case SUBMIT_STARTED:
      if (!state.isSubmitting) {
        return state.merge({
          isSubmitting: true,
          pendingData: action.data
        });
      } else {
        return state;
      }
  }
}

So you end-up having same logical checks in two places. Obviously duplication in this example is minimal, but for more complex scenarious it may get not pretty.

[gaearon]

I think in the last example the check should not be inside the reducer. Otherwise it can quickly descend into inconsistency (e.g. action is dispatched by mistake, but ignored, but then the “success” action is also dispatched but it was unexpected, and state might get merged incorrectly).

(Sorry if that was the point you were making ;-)

[vladap]

I agree with gaeron because the !isSubmitting is already encoded in the fact that SUBMIT_STARTED action has been dispatched. When an action is a result of some logic than that logic should not be replicated in reducer. Then reducer responsibility is just that when anytime it receives SUBMIT_STARTED it doesn't think and just updates state with action payload because somebody else had taken the responsibility to decide that SUBMIT_STARTED.

One should always aim that there is a single thing which takes responsibility for a single decision.

Reducer can then further build on SUBMIT_STARTED fact and extend it with additional logic but this logic should be different. F.e.:

case SUBMIT_STARTED:
  if (goodMood) loading...
  else nothing_happens

I can imagine that it can be sometimes tricky to decide who should be responsible for what. ActionCreator, Middleware, standalone complex module, reducer?

I would aim to have as much decisions as possible in reducers while keeping them pure, because they can be hot-reloaded. If decisions are required for side-effects/async then they go to AC/MW/somewhere_else. It can as well depend how best practices evolve regarding code-reuse, i.e. reducers vs. middleware.

[vladap]

AC can't mutate state.isSubmitting directly. Hence, if its logic depends on it AC needs guarantees that state.isSubmitting will be in sync with its logic. If AC logic wants to set state.isSubmitted to true with new data and reads is back it expects it is set as such. There shouldn't by other logic potentially changing this premise. ACTION is the sync primitive itself. Basically actions define protocol and protocols should be well and clearly defined.

But I think I know what you are trying to say. Redux.state is shared state. Shared state is always tricky. It is easy to write code in reducers which changes state in a way which is unexpected by AC logic. Hence I can imagine it can be hard to keep logic between AC and reducers in sync in some very complex scenarios. This can lead to bugs which might be hard to follow and debug. I believe that for such scenarios it is always possible to collapse the logic to AC/Middleware and make reducers only stupidly act based on well defined actions with no or little logic.

[vladap]

Somebody can always add new reducer which will break some old dependent AC. One should think twice before making AC dependent on Redux.state.

[jedwards1211]

@halt-hammerzeit you have a point there. Redux certainly seems intended to get most people to agree on how to manage state updates, so it's a shame that it hasn't gotten most people to agree on how to perform side effects.

[gaearon]

Have you seen the "examples" folder in the repo?

While there are multiple ways to perform side effects, generally the recommendation is to either do that outside Redux or inside action creators, like we do in every example.

[jedwards1211]

Yes, I know...all I'm saying is, this thread has illustrated a lack of consensus (at least among people here) about the best way to perform side effects.

[jedwards1211]

Though perhaps most users do use thunk action creators and we're just outliers.

[catamphetamine]

^ what he said.

I'd prefer all the greatest minds to agree on a single Righteous solution
and carve it in stone so that i don't have to read through a 9000 screens
thread

On Thursday, January 7, 2016, Andy Edwards notifications@github.com wrote:

Yes, I know...all I'm saying is, this thread has illustrated a lack of
consensus (at least among people here) about the best way to perform side
effects.

—
Reply to this email directly or view it on GitHub
#291 (comment).

[catamphetamine]

So, i guess, the currently agreed dolution is to do rverything in action
creators. I,m gonna try using this approach and incase i find any flaws in
it i'll post back here

On Thursday, January 7, 2016, Николай Кучумов kuchumovn@gmail.com wrote:

^ what he said.

I'd prefer all the greatest minds to agree on a single Righteous solution
and carve it in stone so that i don't have to read through a 9000 screens
thread

On Thursday, January 7, 2016, Andy Edwards <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Yes, I know...all I'm saying is, this thread has illustrated a lack of
consensus (at least among people here) about the best way to perform side
effects.

—
Reply to this email directly or view it on GitHub
#291 (comment).

[gaearon]

This and similar threads exist because 1% of Redux users like to look for more powerful / pure / declarative solutions for side effects. Please don't take the impression that nobody can agree on that. The silent majority uses thinks and promises, and are mostly happy with this approach. But like any tech it has downsides and tradeoffs. If you have complex asynchronous logic you might want to drop Redux and explore Rx instead. Redux pattern is easily expressive in Rx.

[catamphetamine]

Ok, thanks

On Thursday, January 7, 2016, Dan Abramov notifications@github.com wrote:

This and similar threads exist because 1% of Redux users like to look for
more powerful / pure / declarative solutions for side effects. Please don't
take the impression that nobody can agree on that. The silent majority uses
thinks and promises, and are mostly happy with this approach. But like any
tech it has downsides and tradeoffs. If you have complex asynchronous logic
you might want to drop Redux and explore Rx instead. Redux pattern is
easily expressive in Rx.

—
Reply to this email directly or view it on GitHub
#291 (comment).

[jedwards1211]

@gaearon yeah, you're right. And I appreciate that Redux is flexible enough to accommodate all of our different approaches!

[slorber]

@halt-hammerzeit take a look at my answer here where I explain why redux-saga can be better than redux-thunk: http://stackoverflow.com/questions/34570758/why-do-we-need-middleware-for-async-flow-in-redux/34599594

[catamphetamine]

@gaearon By the way, your answer on stackoverflow has the same flaw as the documentation - it covers just the simplest case
http://stackoverflow.com/questions/34570758/why-do-we-need-middleware-for-async-flow-in-redux/34599594#34599594
Real-world applications quickly go beyond just fetching data via AJAX: they should also handle errors and perform some actions depending on the output of the action call.
Your advice is to just dispatch some other event and that's it.
So what if my code wants to alert() a user on action completion?
That's where those thunks won't help, but promises will.
I currently write my code using simple promises and it works that way.

I have read the adjacent comment about redux-saga.
Didn't understand what it does though, lol.
I'm not so much into the monads thing and such, and I still don't know what a thunk is, and I don't like that weird word to begin with.

Ok, so I keep using Promises in React components then.

[gaearon]

We suggest returning promises from thunks throughout the docs.

[catamphetamine]

@gaearon Yeah, that's what i'm talking about: on_click() { dispatch(load_stuff()).then(show_modal('done')) }
That will do.

[gaearon]

I think you're still missing something.
Please see README of redux-thunk.
It shows how to chain thunk action creators under the "Composition" section.

[catamphetamine]

@gaearon Yeah, that's exactly what I'm doing:

store.dispatch(
  makeASandwichWithSecretSauce('My wife')
).then(() => {
  console.log('Done!');
});

It's exactly what I wrote above:

on_click()
{
    dispatch(load_stuff())
        .then(() => show_modal('done'))
        .catch(() => show_error('not done'))
}

That README section is well written, thx

[gaearon]

No, this wasn't my point. Take a look at makeSandwichesForEverybody. It's a thunk action creator calling other thunk action creators. This is why you don't need to put everything in components. See also async example in this repo for more of this.

[catamphetamine]

@gaearon But I think it wouldn't be appropriate to put, say, my fancy animation code into the action creator, would it?
Consider this example:

button_on_click()
{
    this.props.dispatch(load_stuff())
        .then(() =>
        {
                const modal = ReactDOM.getDOMNode(this.refs.modal)
                jQuery(modal).fancy_animation(1200 /* ms */)
                setTimeout(() => jQuery.animate(modal, { background: 'red' }), 1500 /* ms */)
        })
        .catch(() =>
        {
                alert('Failed to bypass the root mainframe protocol to override the function call on the hyperthread's secondary firewall')
        })
}

How would you rewrite it the Right way?

[slorber]

@halt-hammerzeit you can make the actionCreator return the promises so that the component can show some spinner or whatever by using local component state (hard to avoid when using jquery anyway)

Otherwise you can manage complex timers to drive animations with redux-saga.

Take a look at this blog post: http://jaysoo.ca/2016/01/03/managing-processes-in-redux-using-sagas/

[gaearon]

Oh, in this case it's fine to keep it in component.
(Note: generally in React it's best to use declarative model for everything rather than showing modals imperatively with jQuery. But no big deal.)

[catamphetamine]

@slorber Yeah, I'm already returning Promises and stuff from my "thunks" (or whatever you call them; errr, i don't like this weird word), so I can handle spinners and such.

@gaearon Ok, I got you. In the ideal machinery world it would of course be possible to stay inside the declarative programming model, but reality has its own demands, irrational, say, from the view of a machine. People are irrational beings not just operating with pure zeros and ones and it requires compromising the code beauty and purity in favour of being able to do some irrational stuff.

I'm satisfied with the support in this thread. My doubts seem to be resolved now.

[gaearon]

Relevant new discussion: #1528

[juliandavidmr]

@gaearon very awesome explanation! 🏆 Thanks 👍