svrcekmichal / redux-axios-middleware

Redux middleware for fetching data with axios HTTP client
MIT License
920 stars 96 forks source link

How about an onRequest hook? #33

Closed ckald closed 7 years ago

ckald commented 7 years ago

Hi

I am working on a notification system for my app and was able to use onError and onSuccess hooks to dispatch additional actions like this:


import {onSuccess as defaultOnSuccess, onError as defaultOnError} from 'redux-axios-middleware/lib/defaults'

export const onSuccess = ({ action, next, response }, options) => {
  if (action.messages && action.messages[1]) {
    let message = action.messages[1]
    if (typeof message == 'function') {
      message = message(action)
    }
    next(notify(message, 'success'))
  }
  return defaultOnSuccess({action, next, response}, options)
};

export const onError = ({ action, next, error }, options) => {
  if (action.messages && action.messages[2]) {
    let message = action.messages[2]
    if (typeof message == 'function') {
      message = message(action)
    }
    next(notify(message, 'error'))
  }
  return defaultOnError({action, next, error}, options)
};

However, I cannot find any kind of onRequest handler to display the loading message. I can add another middleware, but it seems that it would make sense for redux-axios-middleware to have this

nmaves commented 7 years ago

Sounds like you want to dispatch an action on every single request you make?

If that is the case you could use a request interceptor.

ckald commented 7 years ago

I did not know we had dispatch in interceptors! Thanks, let me try this and come back to you

ckald commented 7 years ago

Actually, interceptors do not help in this case at all. I added them in this way:

 interceptors: {
        request: [({dispatch, action}, config) => {
          console.log("request", action)
          if (action.messages && action.messages[0]) {
            let message = action.messages[0]
            if (typeof message == 'function') {
              message = message(action)
            }
            dispatch(notify(message, 'info'))
          }
          return config
        }],
        response: [
          {
            success: ({dispatch, action}, response) => {
          console.log("success", action)
              if (action.messages && action.messages[1]) {
                let message = action.messages[1]
                if (typeof message == 'function') {
                  message = message(action)
                }
                dispatch(notify(message, 'success'))
              }
              return response
            },
            error: ({dispatch, action}, error) => {
          console.log("error", action)
              if (action.messages && action.messages[2]) {
                let message = action.messages[2]
                if (typeof message == 'function') {
                  message = message(action)
                }
                dispatch(notify(message, 'error'))
              }
              return Promise.reject(error)
            }
          }
        ]
      }

But these functions fire only in response to middleware's own actions and do not contain the data I need — messages:

svrcekmichal commented 7 years ago

Issue is little bit different from what you describe, but here's the problem. After initialization of middleware, one of the arguments are clients. Client have their own interceptors and interceptors are bound to client when first action which require client is fired. And here comes the issue, interceptor is bound with getState, dispatch and action, but problem of action is that it is the inicialization action not the action which interceptor runs for.

Example:

I init middleware with client google, client google got request and response interceptor. I fire first action to google client, called Action1. First request interceptor gets Action1, then axios get Action1 then response interceptor get Action1. I fire second action called Action2. Here request interceptor get Action1 axios get Action2 and response interceptor get Action1

the question is, if we can move interceptors to this middleware and use axios just for request, would it be big deal for us?

@ckald To give you solution, I would probably use custom middleware before our one. I believe this would take some time to solve

ckald commented 7 years ago

Thanks, this is precisely what I did.

On Sat, Sep 17, 2016, 17:22 Michal Svrček notifications@github.com wrote:

Issue is little bit different from what you describe, but here's the problem. After initialization of middleware, one of the arguments are clients. Client have their own interceptors and interceptors are bound to client when first action which require client is fired. And here comes the issue, interceptor is bound with getState, dispatch and action, but problem of action is that it is the inicialization action not the action which interceptor runs for.

Example:

I init middleware with client google, client google got request and response interceptor. I fire first action to google client, called Action1. First request interceptor gets Action1, then axios get Action1 then response interceptor get Action1. I fire second action called Action2. Here request interceptor get Action1 axios get Action2 and response interceptor get Action1

the question is, if we can move interceptors to this middleware and use axios just for request, would it be big deal for us?

@ckald https://github.com/ckald To give you solution, I would probably use custom middleware before our one. I believe this would take some time to solve

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/svrcekmichal/redux-axios-middleware/issues/33#issuecomment-247780722, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMf11vfh4Y00zVUCHgjnyLKX-e4h1dsks5qrAXBgaJpZM4J74QT .

Andrii Magalich

ckald commented 7 years ago

This is the source, for the reference:

export const axiosNotificationsMiddleware = store => next => action => {
  if (action.type) {
    if (action.type.endsWith('REQUEST') && action.messages && action.messages[0]) {
      let message = action.messages[0]
      if (typeof message == 'function') {
        message = message(action)
      }
      store.dispatch(notify(message, 'info'))
    }
    if (action.type.endsWith('SUCCESS')) {
      let message = (action.meta && action.meta.previousAction
          && action.meta.previousAction.messages
          && action.meta.previousAction.messages[1])
      if (message) {
        if (typeof message == 'function') {
          message = message(action)
        }
        store.dispatch(notify(message, 'success'))
      }
    }

    if (action.type.endsWith('FAIL')) {
      let message = (action.meta && action.meta.previousAction
          && action.meta.previousAction.messages
          && action.meta.previousAction.messages[2])
      if (message) {
        if (typeof message == 'function') {
          message = message(action)
        }
        store.dispatch(notify(message, 'error'))
      }
    }
  }
  return next(action)
}

As you can see, it relies on a "messages" field just like axios middleware relies on "request". Messages can be strings or even functions.

nmaves commented 7 years ago

I believe this was fixed in the 4.0.0 release. We have added a getSourceAction parameter to the request and response interceptors.