Feature Request: Add a generic try / catch saga wrapper

[tomekkleszcz]

Feature Request: Add a generic try / catch saga wrapper

[tomekkleszcz]

@chmac I've created three effect creators for now:

• takeEveryPromiseAction
• takeLeadingPromiseAction
• takeLatestPromiseAction

I thought overriding redux-saga effect creators' names isn't a good idea, so I've suffixed the names with PromiseAction to distinguish creators. The take and takeMaybedoes do not create a new saga, so they are not needed.

You can check these creators in your project by entering

"redux-saga-promise-actions": "git://github.com/tomekkleszcz/redux-saga-promise-actions.git#feature/effect_creators"

into the package.json file.

Example usage:

export function* signIn(action: ReturnType<typeof actions.signIn.request>) {
    const resp = yield call...

    yield put(actions.signIn.success(resp.data));
    return resp.data.refreshToken;
}

export const auth = [takeEveryPromiseAction(actions.signIn.request, signIn)];

Please let me know if this suits your needs, so I may work on this feature further.

[tomekkleszcz]

@chmac Yes, for now, these effect creators accept only promises actions. Other actions cannot be passed as a pattern, because of the type restrictions.

[tomekkleszcz]

@chmac I wonder if it is possible to implement this feature that way, it would not require any yield call(implementPromiseAction... in the saga. What I'm thinking instead is to create three effect creators:

• takeEvery
• takeLeading
• takeLatest

They will watch for actions and behave just like their redux-saga counterparts, but they will resolve or reject promise actions by default. Please let me know, what do you think about this idea and whether you would like it to be implemented in the library or not.

[chmac]

@tomekkleszcz That's a really interesting idea. Very interesting. Then I'd be able to swap all my take* for this library's versions and suddenly all my sagas are safely wrapped. Sounds excellent!

There's quite a few take* APIs in redux-saga, but maybe there's a simple way to wrap them all with a factory. 🤔

take(pattern)
takeMaybe(pattern)
take(channel)
takeMaybe(channel)
takeEvery(pattern, saga, ...args)
takeEvery(channel, saga, ...args)
takeLatest(pattern, saga, ..args)
takeLatest(channel, saga, ..args)
takeLeading(pattern, saga, ..args)
takeLeading(channel, saga, ..args) 

Seems like they all have a very similar pattern, so maybe the factory works well.

[chmac]

@tomekkleszcz AWESOME! That's outstanding. It looks perfect for my use case. I'll check it tomorrow, thanks a lot.

One immediate point of feedback on the name. When I read the function names, I expect them to only "take" promise actions, and ignore others. I wonder if that's what they do, or if they can be used everywhere and "wrap" promise actions. I'll think overnight if I can come up with any suggestions on naming.

[chmac]

@tomekkleszcz Makes sense. Then the naming is great! :-) I'll install today and report back.

Thanks a lot, you've really helped me out here. I've spent a few days now trying to figure out ho to structure my sagas so I can both dispatch them with actions, and from within other sagas and wait for their completion. All the while keeping my types accurate. Excited to test out this code. 👍

[chmac]

Great, got the code running, and I created a wrapper to automate the whole process of creating actions and linking them.

import { SagaIterator } from "redux-saga";
import { createPromiseAction, PromiseAction } from "redux-saga-promise-actions";
import * as effectCreators from "redux-saga-promise-actions/dist/effectCreators";

export type TakeEffectType = keyof typeof effectCreators;

export const createAsyncPromiseSaga = <P, R, E>({
  prefix,
  effect,
  takeType,
}: {
  prefix: string;
  effect: (action: PromiseAction<P, R>) => SagaIterator<R>;
  takeType: TakeEffectType;
}) => {
  const actionCreators = createPromiseAction(
    `${prefix}/request`,
    `${prefix}/success`,
    `${prefix}/failure`
  )<P, R, E>();

  const { request, success, failure } = actionCreators;

  const takeEffect = effectCreators[takeType];

  function* saga() {
    yield takeEffect(request.toString(), effect);
  }

  return {
    request,
    success,
    failure,
    saga,
  };
};

I couldn't get the typing correct without doing request.toString() if I just passed request, I got a type error about args and arg not being compatible. My head started exploding with all the types, so I ended up settling for the toString() workaround.

The import from /dist/effectCreators is a bit inelegant, but I haven't been able to figure out how to get imports to be all inside the dist/ directory, so I don't have a suggestion on how to fix that. Hmm, the redux-typed-saga package somehow manages it with a second package.json file. I could look into that and see if we can get the same running here if you like.

I guess it's also not such a big deal, we could export effectCreators from the index file as a simpler alternative?

[chmac]

@tomekkleszcz Been playing with this a bit now. Realising that I imagined the API working slightly differently. I imagined that I'd do the following:

const actions = createPromiseAction(...)<...>();
...
takeEveryPromiseAction(actions, effect);

Then I imagine that actions.failure will be dispatched at the same time that the promise is resolved.

But now I look into it further, I also had the same expectation of the resolvePromiseAction() and rejectPromiseAction() APIs. I expected they would do 2 things, resolve the promise and dispatch the success or failure action accordingly.

Hmmm. 🤔 This is not how the API currently works, and therefore not how your ~2k monthly download users are using the API. I guess it doesn't make sense to add the action dispatch inside the takeEveryPromiseAction() effect, what do you think?

[tomekkleszcz]

@chmac I could add an optional flag named eg. dispatchOnResolve when creating a promise action. If it is provided, the success and failure will be dispatched when resolvePromiseAction and rejectPromiseAction will be called.

I will also create the react-native-promise-actions/effects submodule to help you clean imports.

[tomekkleszcz]

@chmac You can now import from react-native-promise-actions/effects instead of redux-saga-promise-actions/dist/effectCreators. Please pull the newest changes from the feature/effect_creators branch.

[chmac]

@tomekkleszcz This is PHENOMENAL. Thanks so much. Love it. Will pull now and report back shortly.

❤️ ❤️ ❤️

[chmac]

@tomekkleszcz The flag idea makes a lot of sense. Excellent idea. 👍 👍 👍

Sorry for the comment noise, got too excited and posted before having read everything properly!

[tomekkleszcz]

@chmac I gave up the idea with the flag. Instead, I came up with a different solution that makes more sense to me.

When using takeEveryPromiseAction, takeLeadingPromiseAction, or takeLatestPromiseAction providing a promise action is required instead of request action.

In the following case, success and failure actions are dispatched automatically based on if any error has been caught.

export function* signIn(action: ReturnType<typeof actions.signIn.request>) {
    const resp = yield call...
    return resp.data;
}

export const auth = [takeEveryPromiseAction(actions.signIn, signIn)];

If you decide you need more control over dispatching success and failure actions here is an equivalent of the the previous example:

export function* signIn(action: ReturnType<typeof actions.signIn.request>) {
    try {
        const resp = yield call...
        yield put(actions.signIn.success(resp.data));
        yield call(resolvePromiseAction, action, resp.data);
    } catch(err) {
        yield put(actions.signIn.failure(err));
        yield call(rejectPromiseAction, action, err);
    }
}

export const auth = [takeEvery(actions.signIn.request, signIn)];

Please let me know if that solution works for you. When this pull request will be merged any of the existing features in the master branch won't be changed.

[chmac]

@tomekkleszcz Makes a lot of sense. Also to keep the existing API clean. 👍

Your new version is working great for me. I also was able to update my creator helper to support the new layout.

Thanks a million. Will do some more in depth testing tomorrow and report back.

[tomekkleszcz]

@chmac Any thoughts about current implementation? :)

[chmac]

@tomekkleszcz Thanks for the reminder. Here's what I ended up using:

https://github.com/GenerousLabs/share/blob/main/expo/src/utils/saga.utils.ts

I really struggled to get the typing to flow. That was my big goal, so that I can await a saga, and know what the type of the response will be without having to tell TypeScript. I tried using the takeEveryPromiseAction() effect creators, but they caused a different set of typing issues.

The other topic that came up for me was sanitizing the error. I have a sanitize function which strips out all the non serializable bits and returns a reduced error which we can log more easily, or even save to redux state.

I'm hoping that I get some progress on the type issue with the typed-redux-saga project. If that works, this whole setup will be outstanding. But as of right now, I'm not actually using the new APIs. I feel bad after all the work you put in, I wanted to use it, but types and error parsing were stumbling blocks.

What about putting all this code into an examples folder and pointing to it from the docs? Then we can try to get others involved in the discussion about what's useful. I'm hesitant to say that my specific use case with errors and so on should get in the way of the API you've designed.

Also happy to chat directly, voice, video, or text, if that's easier.

[tomekkleszcz]

Writing examples is a brilliant idea! I will recheck if the current implementation works fine and then merge the #4 pull request. I will also create an issue dedicated to writing examples. Maybe I will find some typing related issues while writing those examples.