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

Add possiblity to dispatch other actions on success, fail #21

Closed
oviava opened this issue Nov 11, 2015 · 23 comments · Fixed by #191
Closed

Add possiblity to dispatch other actions on success, fail #21

oviava opened this issue Nov 11, 2015 · 23 comments · Fixed by #191

Comments

@oviava
Copy link

oviava commented Nov 11, 2015

I think it's a must required feature - to be able to dispatch other actions on success/fail. Perhaps using meta object ? Or new object 'cb' ?!

@riophae
Copy link

riophae commented Dec 11, 2015

It's not possible with redux-api-middleware. That's a great weakness.
redux-effects-fetch makes more sense to me. You may give it a try.

@ziogaschr
Copy link

👍

@Sparragus
Copy link

Yep. This is a MUST feature. Otherwise this middleware is way too limited. For example, if the request is successful, I may want to redirect to user to another page using react-router. Right now I have no idea how I would go about doing that.

I'd really like to see this feature. The works so far is very good. This would really make it an AWESOME middleware. I get tired of having to write a middleware for API calls for every project.

@Sparragus
Copy link

Maybe we can do that by adding a onSuccess/onFailure to the API call parameters. Thoughts?

@mjrussell
Copy link

@Sparragus its actually pretty easy to do right now without modifying the library, although I think this would be a nice feature to add anyway.

You can add redux-thunk middleware and get the same ability.

export function doStuff() {
    return dispatch => {
        dispatch({
            [CALL_API]: {
                ...
                types: [
                    Constants.SOMETHING_FETCHED,
                    {
                        type: Constants.SOMETHING_FETCHED_SUCCEEDED,
                        payload: (action, state, res) => {
                            return res.json().then(json => {
                                dispatch(myAction());
                                return json;
                            });
                        },
                    },
                    Constants.SOMETHING_FETCHED_FAILED,
                ],
                ...

@Sparragus
Copy link

I thought on dispatching an action inside the payload function but I doesn't feel it's right. That function converts the response to another format and the action Constants.SOMETHING_FETCHED_SUCCEEDED has still to be dispatched. Maybe a cause for race conditions.

I'm already working on onSucceed and onFailure and using it on my project. Hopefully I can send that pull request before v1.0.0 gets out.

@mjrussell
Copy link

Yeah in my case I dont have any requirement that I need the SUCC action to hit the store before the onSucc dispatched action hits it. I just want to fire something on a 200 response code.

Looking forward to the PR!

@jarodccrowe
Copy link

+1

@Sparragus
Copy link

I'm working on this and already using it. So far what I have are two new properties you may (but don't need to) add to the [CALL_API]: onSucceed and onFailure. These two are functions that receive two parameters: dispatch, and response.

I do have one bug with the response parameter. Since it is the same response as in middleware.js#L103, and that response is being read on util.js#L72 when transforming it, I can't seem to read it again using getJSON(res). So passing as a parameter to onSucceed is basically useless. However, I do think we should find a way to pass the dispatch function and the response.

EDIT: The response bug can be fixed with responce.clone(). So a follow up: how can we passed the transformed response/payload to onSucceed?

dispatch isn't terribly important. Using redux-thunk one can obtain the dispatch function. But I do find it convenient.

The code I have so far is this:

    const { method, body, credentials, bailout, types, onSuccess, onFailure } = callAPI;

   // (...a lot of code in between...)

    // Process the server response
    if (res.ok) {
      next(await actionWith(
        successType,
        [action, getState(), res]
      ));

      if (typeof onSuccess === 'function') {
        onSuccess(dispatch, res);
      }
    } else {
      next(await actionWith(
        {
          ...failureType,
          error: true
        },
        [action, getState(), res]
      ));
      if (typeof onFailure === 'function') {
        onFailure(dispatch, res);
      }
    }

My question to you: how is it looking so far? Thoughts?

@masad-frost
Copy link

Just use thunk and promises, way cleaner than callbacks.

@Sparragus
Copy link

@masad-frost Can you post an example?

@majodev
Copy link

majodev commented Feb 12, 2016

Hi, I can also confirm that dispatching "thunks" is working without any problem (v1.0.0-beta3), @Sparragus here is an example:

export function patchAsyncExampleThunkChainedActionCreator(values) {
  return async(dispatch, getState) => {
    const actionResponse = await dispatch({
      [CALL_API]: {
        endpoint: "...",
        method: "PATCH",
        body: JSON.stringify(values),
        headers: {
          "Accept": "application/json",
          "Content-Type": "application/json",
        },
        types: [PATCH, PATCH_SUCCESS, PATCH_FAILED]
      }
    });

    if (actionResponse.error) {
      // the last dispatched action has errored, break out of the promise chain.
      throw new Error("Promise flow received action error", actionResponse);
    }

    // you can EITHER return the above resolved promise (actionResponse) here...
    return actionResponse;

    // OR resolve another asyncAction here directly and pass the previous received payload value as argument...
    return await yourOtherAsyncAction(actionResponse.payload.foo);
  };
}

@Sparragus
Copy link

Oh wow, @majodev. THIS CHANGES EVERYTHING. :D

I didn't know that dispatch(action) actually returns the action itself. Thanks for the example. It definitely cleared things up. 👍

@audionerd
Copy link
Contributor

In the docs for redux-thunk there's a section on composition that explains this further:

// In fact I can write action creators that dispatch
// actions and async actions from other action creators,
// and I can build my control flow with Promises.

function makeSandwichesForEverybody() {
  return function (dispatch, getState) {
    if (!getState().sandwiches.isShopOpen) {

      // You don’t have to return Promises, but it’s a handy convention
      // so the caller can always call .then() on async dispatch result.

      return Promise.resolve();
    }

    // We can dispatch both plain object actions and other thunks,
    // which lets us compose the asynchronous actions in a single flow.

    return dispatch(
      makeASandwichWithSecretSauce('My Grandma')
    ).then(() =>
      Promise.all([
        dispatch(makeASandwichWithSecretSauce('Me')),
        dispatch(makeASandwichWithSecretSauce('My wife'))
      ])
    ).then(() =>
      dispatch(makeASandwichWithSecretSauce('Our kids'))
    ).then(() =>
      dispatch(getState().myMoney > 42 ?
        withdrawMoney(42) :
        apologize('Me', 'The Sandwich Shop')
      )
    );
  };
}

// This is very useful for server side rendering, because I can wait
// until data is available, then synchronously render the app.

store.dispatch(
  makeSandwichesForEverybody()
).then(() =>
  response.send(React.renderToString(<MyApp store={store} />))
);

@quangthe
Copy link

quangthe commented Jun 4, 2017

@majodev I tried to use your code, but the following statement return undefined
const actionResponse = await dispatch({...});

"redux-thunk": "^2.2.0"
"redux-api-middleware": "^1.0.3"

What am I missing here?

@RanzQ
Copy link

RanzQ commented Jul 6, 2017

@quangthe You need ES2017 or greater which supports async/await. Then make sure you put async in front of the function you want to use await in. Like in the example above:

return async(dispatch, getState) => {

It can be done with plain promises too:

return (dispatch, getState) => {
    return dispatch(someAction()).then(actionResponse => {
        /* handle actionResponse here */
    })
}

@quangthe
Copy link

quangthe commented Jul 7, 2017

@RanzQ Thanks for your answer, I would go with plain promise instead because of ES6 in my project.

@nason
Copy link
Collaborator

nason commented Mar 3, 2018

Whats the status of this one? Does redux-thunk give enough flexibility or are there use cases where we'd want this middleware to pass dispatch itself?

Anyone want to help with some docs on this subject?

@zandersays
Copy link

I am still getting an undefined from:
const actionResponse = await dispatch({[RSAA]: {...}});

I am using es2017, regardless, I get this from doing both async/await or using a plain promise.
Am I doing something incorrectly? I am following @majodev 's example, but I still get an undefined response. I can dispatch from inside the success type, but that feels incorrect.

anyone have any insight?

@stocky37
Copy link

stocky37 commented Apr 17, 2018

Not sure if this will be helpful, but using @majodev's example I've written a custom middleware that essentially wraps RSAA actions and handles onSuccess & onFailure callbacks:

export const apiMiddlewareWrapper = ({dispatch}) => next => (action) => {
	if(!isRSAA(action)) {
		return next(action);
	}

	const {onSuccess, onFailure, ...rsaaAction} = action;
	if(!onSuccess && !onFailure) {
		return next(rsaaAction);
	}

	return (async() => {
		const response = await next(rsaaAction);
		if(response.error && onFailure) {
			dispatch(onFailure);
		} else if(onSuccess) {
			dispatch(onSuccess);
		}
		return response;
	})();
};

@nason nason closed this as completed in #191 Jun 8, 2018
@nason
Copy link
Collaborator

nason commented Jun 8, 2018

@zandersays I've seen reports of this being caused by middleware order #179 (comment)

Does that help at all?

I'm going to close this issue out since there does seem to be an accepted answer and I've documented it in the readme. Feel free to keep this thread going or to open new issues to continue.

@zandersays
Copy link

@nason I figured out what the issue was.

On top off middleware order, any middleware that calls next(action) instead of return next(action) when it is finished won't return the promise.

This was hard to catch, because next still moves on to the next action or middleware but it doesn't pass on that promise.

Regardless, I got it figured out, and your direction helped me immensely. Thanks much.

@wanschi
Copy link

wanschi commented Apr 10, 2019

@zandersays That was the solution for me! Thank you for finding the error!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.