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

Compatibility with redux-thunk middleware #605

Open
dentuzhik opened this issue Jan 29, 2019 · 4 comments
Open

Compatibility with redux-thunk middleware #605

dentuzhik opened this issue Jan 29, 2019 · 4 comments

Comments

@dentuzhik
Copy link

dentuzhik commented Jan 29, 2019

I'm not sure if it's a feature a bug and it might be a usage question, but it seems that it better belongs to Github, then SO.

We have started to use redux-observable since version 1.0.0, so by this time all the docs mentioning compatibility with thunks have already been eradicated, and my search across Github have found only a bunch of deprecations and some very old issues. #13

Our app has used redux-thunk middleware mostly related to API calls, so when integrating redux-observable, we used something like this:

const middleware = [thunk, epicMiddleware];

Later on I discovered that this way dispatching thunks JUST WORKS, and after reading source code and trying to source docs I am still unsure whether this is expected behaviour or some bug (feature) being exploited accidentally.

Code looks like this:

import { of, timer } from 'rxjs';
import { switchMap } from 'rxjs/operators';
import { ofType } from 'redux-observable';

export const getDataAction = function () {
  return async dispatch => {
    try {
      dispatch({
        type: REQUEST_DATA_START,
      });

      const response = await getDataAPI();

      dispatch({
        type: REQUEST_DATA_SUCCESS,
        payload: response,
      });

      return response;
    } catch (err) {
      dispatch({
        type: REQUEST_DATA_FAILURE,
        payload: err,
        error: true,
      });

      throw err;
    }
  };
};

export const startPollingAction = () => ({ type: START_POLLING });
export const stopPollingAction = () => ({ type: STOP_POLLING });

export const somePollingEpic = action$ =>
  action$.pipe(
    ofType(START_POLLING),
    switchMap(() => {
      return timer(0, config.PollingInterval).pipe(
        switchMap(() => {
          return of(getDataAction());
        }),
        takeUntil(action$.pipe(ofType(STOP_POLLING))),
      );
    }),
  );

Putting debugger inside getDataAction does indeed approve that thunk gets properly executed and everything works as expected.

The only problem with this approach which I was able to find, that it is impossible to handle errors via catchError (because I assume that since it happens after store.dispatch, redux-observable does not know anything about it. Which is also not a large issue, since it's possible to create another epic if necessary to handle REQUEST_DATA_FAILURE.

So is it "idiomatically correct" to use this behaviour, or this is something that shouldn't be relied on, and may not work in future versions?

And if it's not correct is there a way to keep these two operations atomic and independently testable? For example thunk could be used independently in some other place, which relies on the fact that it's a promise, and as far as I know this is not currently possible with pure epics.

Thank you in advance!
Let me know if I should delete this issue and re-post to SO, or maybe file a PR for entry in docs.

@AylanBoscarino
Copy link

This issue deserves some more attention.
I'm facing the same situation but I'm using TypeScript and I cant use correctly the type Epic in my epics because it's first, and hence second, generic arguments must extend the Action type.

However the code is working as described above and I can create "type safe" epics by typing its arguments and its return without using the Epic type. I just would rather a way of typing ThunkActions in the Epic generics.

@dentuzhik
Copy link
Author

dentuzhik commented May 23, 2019

and I can create "type safe" epics by typing its arguments and its return without using the Epic type

@AylanBoscarino we are also using TS now, can you share code same where you have fully type safe epics with thunks?

Many thanks.

@AylanBoscarino
Copy link

AylanBoscarino commented May 23, 2019

Sure but I'm not totally sure they are completely type safe tho, hence the quotes.

I'm creating epics like this:

const stepsActivities = (
  action$: ActionsObservable<GenericAction<ActivitiesState>>,
  _state$: StateObservable<StoreState>,
  _dependencies: DependableServices,
): Observable<
  ThunkAction<
    Promise<void>,
    StoreState,
    DependableServices,
    GenericAction<ActivitiesState>
  >
> =>
  action$.pipe(
    ofType(ACTIVITIES_FETCH_STEPS),
    flatMap(() => [
      activitiesStepsPeriod('1 day'),
      activitiesStepsPeriod('1 week'),
      activitiesStepsPeriod('1 month'),
    ]),
  );

The function activitiesStepsPeriod is a ThunkAction dispatcher that do some side effects and dispatch an action:

export function activitiesStepsPeriod(
  period: timePeriod,
): ThunkAction<
  Promise<void>,
  StoreState,
  DependableServices,
  GenericAction<types.ActivitiesState>
> {
  return async (dispatch, getState, { activitiesService }) => {
    activitiesService.getStepCount(period, steps => {
      
      switch (period) {
        case '1 day':
          dispatch({
            type: types.ACTIVITIES_STEP_DAILY,
            payload: { dailySteps: steps },
          });

        case '1 week':
          dispatch({
            type: types.ACTIVITIES_STEP_WEEKLY,
            payload: { weeklySteps: steps },
          });

        case '1 month':
          dispatch({
            type: types.ACTIVITIES_STEP_MONTHLY,
            payload: { monthlySteps: steps },
          });
      }
    });
  };
}

How are you creating the epics in your project?

@safareli
Copy link

Created PR to fix this #780

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

Successfully merging a pull request may close this issue.

3 participants