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

Saga -> Component communication (aka sagas and setState) #907

Open
brettstack opened this Issue Apr 12, 2017 · 33 comments

Comments

Projects
None yet
@brettstack

brettstack commented Apr 12, 2017

I want to set form loading state (spinner icon, disable input) when the user submits the form, and clear that state when the action completes or fails. Currently I am storing this state in my store, which involves creating an action creator and reducer, which is annoying for a few reasons:

  1. It's a lot more work than simply calling this.setState
  2. It's more difficult to reason about
  3. I don't really want to store local component state in my store

Essentially I want to do this from within my form component:

handleFormSubmit() {
  this.setState({ isSaving: true })
  this.props.dispatchSaveForm({ formData: this.props.formData })
    .finally(() => this.setState({ isSaving: false }))
}
@brettstack

This comment has been minimized.

Show comment
Hide comment
@brettstack

brettstack Apr 12, 2017

I haven't been able to find any discussion on this, but conceivably, I could do this:

handleFormSubmit() {
  this.setState({ isSaving: true })
  this.props.dispatchSaveForm({
    formData: this.props.formData,
    callback: () => this.setState({ isSaving: false })
  })
}

Any issues with this? Is there a better way?

brettstack commented Apr 12, 2017

I haven't been able to find any discussion on this, but conceivably, I could do this:

handleFormSubmit() {
  this.setState({ isSaving: true })
  this.props.dispatchSaveForm({
    formData: this.props.formData,
    callback: () => this.setState({ isSaving: false })
  })
}

Any issues with this? Is there a better way?

@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Apr 12, 2017

Member

Your action is losing its serialibility. Personally I prefer them being plain objects. The question is - what do u want (ui-wise) to do after saving is complete?

Member

Andarist commented Apr 12, 2017

Your action is losing its serialibility. Personally I prefer them being plain objects. The question is - what do u want (ui-wise) to do after saving is complete?

@brettstack

This comment has been minimized.

Show comment
Hide comment
@brettstack

brettstack Apr 12, 2017

Willing to lose serializability for simplicity here. In this example, I want the form to become disabled and show a spinning icon. Of course I can achieve this by storing state in the store, but it requires a ridiculous number of steps for something that should be so simple.

brettstack commented Apr 12, 2017

Willing to lose serializability for simplicity here. In this example, I want the form to become disabled and show a spinning icon. Of course I can achieve this by storing state in the store, but it requires a ridiculous number of steps for something that should be so simple.

@eirslett

This comment has been minimized.

Show comment
Hide comment
@eirslett

eirslett Apr 18, 2017

We use redux-wait-for-action as a compromise when we need do do hacks like this. it makes store.dispatch(...) return a Promise which you can .then() or .catch() on.

eirslett commented Apr 18, 2017

We use redux-wait-for-action as a compromise when we need do do hacks like this. it makes store.dispatch(...) return a Promise which you can .then() or .catch() on.

@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Apr 19, 2017

Member

If u want to bypass the store I think indeed some Promise-based solution is the best way to achieve your goal. Interesting problem nevertheless, wondering if this could be somehow tackled by us on the library level, but no ideas yet. Any thoughts @yelouafi ?

Member

Andarist commented Apr 19, 2017

If u want to bypass the store I think indeed some Promise-based solution is the best way to achieve your goal. Interesting problem nevertheless, wondering if this could be somehow tackled by us on the library level, but no ideas yet. Any thoughts @yelouafi ?

@ms88privat

This comment has been minimized.

Show comment
Hide comment
@ms88privat

ms88privat May 3, 2017

This problem is really old. Some years ago someone posted a similar solution to the callback style.

handleSubmit(data) {
    return new Promise((resolve, reject) => this.props.dispatchAction(data, resolve, reject))

This way you can even reject form SubmissionErrors.
It is working fine in my apps. Loosing serialibility is of course a drawback.

ms88privat commented May 3, 2017

This problem is really old. Some years ago someone posted a similar solution to the callback style.

handleSubmit(data) {
    return new Promise((resolve, reject) => this.props.dispatchAction(data, resolve, reject))

This way you can even reject form SubmissionErrors.
It is working fine in my apps. Loosing serialibility is of course a drawback.

@raarts

This comment has been minimized.

Show comment
Hide comment
@raarts

raarts Aug 5, 2017

I think the state does belong in the store, but if there's a way to address this:

but it requires a ridiculous number of steps for something that should be so simple.

that would be great.

raarts commented Aug 5, 2017

I think the state does belong in the store, but if there's a way to address this:

but it requires a ridiculous number of steps for something that should be so simple.

that would be great.

@norbertpy

This comment has been minimized.

Show comment
Hide comment
@norbertpy

norbertpy Oct 9, 2017

Any update on this? Some of the transient states do not really belong to redux store. Is there an easy way to do:

class App extends Component {
  state = {
    loading: false,
  }

  handleGetData = () => {
    this.setState({ loading: true });
    const data = this.props.getData(); // SAGA
    this.setState({ loading: false });
  }

  render () {
    const { loading } = this.state;
    return (
        <div>
          { loading && <span> loading... </span> }
          <button onClick={ this.handleGetData }> fetch </button>
       </div>
    );
  }
}

norbertpy commented Oct 9, 2017

Any update on this? Some of the transient states do not really belong to redux store. Is there an easy way to do:

class App extends Component {
  state = {
    loading: false,
  }

  handleGetData = () => {
    this.setState({ loading: true });
    const data = this.props.getData(); // SAGA
    this.setState({ loading: false });
  }

  render () {
    const { loading } = this.state;
    return (
        <div>
          { loading && <span> loading... </span> }
          <button onClick={ this.handleGetData }> fetch </button>
       </div>
    );
  }
}
@eirslett

This comment has been minimized.

Show comment
Hide comment
@eirslett

eirslett Oct 9, 2017

loading state as true/false is maybe a bad example, since it's widely considered correct practice to store that in Redux: http://redux.js.org/docs/advanced/AsyncActions.html

eirslett commented Oct 9, 2017

loading state as true/false is maybe a bad example, since it's widely considered correct practice to store that in Redux: http://redux.js.org/docs/advanced/AsyncActions.html

@norbertpy

This comment has been minimized.

Show comment
Hide comment
@norbertpy

norbertpy Oct 9, 2017

A form getting submitted has nothing to do with redux store. I wanna show a popup before the request and close it after it's done. There is no point writing reducer, action creator, ... for that tiny thing. The Prophet himself seems to agree.

norbertpy commented Oct 9, 2017

A form getting submitted has nothing to do with redux store. I wanna show a popup before the request and close it after it's done. There is no point writing reducer, action creator, ... for that tiny thing. The Prophet himself seems to agree.

@eirslett

This comment has been minimized.

Show comment
Hide comment
@eirslett

eirslett Oct 9, 2017

This is how we do it with redux-wait-for-action, give it a try:

// action creators
const getData = (payload) => ({
  type: 'GET_DATA',
  [WAIT_FOR_ACTION]: 'GET_DATA_SUCCESS',
  [ERROR_ACTION]: 'GET_DATA_FAIL'
});

const getDataSuccess = payload => ({
  type: 'GET_DATA_SUCCESS',
  payload
)};

const getDataFail = error => ({
  type: 'GET_DATA_FAIL',
  error
});

// saga
function * handleGetData (action) {
  try {
    const data = fetchDataFromSomewhere();
    yield put(getDataSuccess(data)); // assume there's an action creator
  } catch (error) {
    yield put(getDataFail(error)); // assume there's an action creator
  }
}

// Inside your UI component (I think setState() returns Promises, they are async, I can't remember)
await this.setState({ loading: true });
const result = await this.props.getData() // <-- will be mapped to store.dispatch(getData())
await this.setState({ loading: false });

eirslett commented Oct 9, 2017

This is how we do it with redux-wait-for-action, give it a try:

// action creators
const getData = (payload) => ({
  type: 'GET_DATA',
  [WAIT_FOR_ACTION]: 'GET_DATA_SUCCESS',
  [ERROR_ACTION]: 'GET_DATA_FAIL'
});

const getDataSuccess = payload => ({
  type: 'GET_DATA_SUCCESS',
  payload
)};

const getDataFail = error => ({
  type: 'GET_DATA_FAIL',
  error
});

// saga
function * handleGetData (action) {
  try {
    const data = fetchDataFromSomewhere();
    yield put(getDataSuccess(data)); // assume there's an action creator
  } catch (error) {
    yield put(getDataFail(error)); // assume there's an action creator
  }
}

// Inside your UI component (I think setState() returns Promises, they are async, I can't remember)
await this.setState({ loading: true });
const result = await this.props.getData() // <-- will be mapped to store.dispatch(getData())
await this.setState({ loading: false });
@norbertpy

This comment has been minimized.

Show comment
Hide comment
@norbertpy

norbertpy Oct 9, 2017

Thanks. But that's the same thing as writing redux ceremonial stuff.

Now I have two problems.

norbertpy commented Oct 9, 2017

Thanks. But that's the same thing as writing redux ceremonial stuff.

Now I have two problems.

@brettstack

This comment has been minimized.

Show comment
Hide comment
@brettstack

brettstack Oct 9, 2017

Right. The redux-wait-for-action approach still has a lot of extra code. This is what I'm currently doing:

// promise-dispatch.js
export default ({ dispatch, actionCreator, payload }) => new Promise((resolve, reject) => dispatch(actionCreator({ ...payload, resolve, reject })))
// In a React component
const response = await promiseDispatch({
  dispatch,
  actionCreator: widgetsPageLoad,
  payload: { foo: 'bar' }
})
this.setState({ loading: false })

Ideally this would just be

const response = await dispatch(widgetsPageLoad({ foo: 'bar' }))
this.setState({ loading: false })

brettstack commented Oct 9, 2017

Right. The redux-wait-for-action approach still has a lot of extra code. This is what I'm currently doing:

// promise-dispatch.js
export default ({ dispatch, actionCreator, payload }) => new Promise((resolve, reject) => dispatch(actionCreator({ ...payload, resolve, reject })))
// In a React component
const response = await promiseDispatch({
  dispatch,
  actionCreator: widgetsPageLoad,
  payload: { foo: 'bar' }
})
this.setState({ loading: false })

Ideally this would just be

const response = await dispatch(widgetsPageLoad({ foo: 'bar' }))
this.setState({ loading: false })
@snikobonyadrad

This comment has been minimized.

Show comment
Hide comment
@snikobonyadrad

snikobonyadrad Oct 10, 2017

@brettstack, @norbertpy If you want generators with ease and not coupled to redux store, you can use co or simply async/await. It'll be like:

// logic.js
import co from 'co';

export const getData = () => 
  new Promise(res => setTimeout(() => res(), 2000));

export const getRestOfTheData = () => 
  new Promise(res => setTimeout(() => res(), 3000));

export const getAll = co.wrap(function* () {
  const d = yield getData();
  const r = yield getRestOfTheData();
  return { d, r };
});

And then in your component:

import { getAll } from './logic';

class App extends Component {
  state = { loading: false }
  handleGetData = () {
    this.setState({ loading: true });
    getAll()
      .then(data => this.setState({ loading: false, data: data }))
      .catch(e => this.setState({ loading: false, error: 'error' }));
  }
  render () {
        return (...);
  }
}

snikobonyadrad commented Oct 10, 2017

@brettstack, @norbertpy If you want generators with ease and not coupled to redux store, you can use co or simply async/await. It'll be like:

// logic.js
import co from 'co';

export const getData = () => 
  new Promise(res => setTimeout(() => res(), 2000));

export const getRestOfTheData = () => 
  new Promise(res => setTimeout(() => res(), 3000));

export const getAll = co.wrap(function* () {
  const d = yield getData();
  const r = yield getRestOfTheData();
  return { d, r };
});

And then in your component:

import { getAll } from './logic';

class App extends Component {
  state = { loading: false }
  handleGetData = () {
    this.setState({ loading: true });
    getAll()
      .then(data => this.setState({ loading: false, data: data }))
      .catch(e => this.setState({ loading: false, error: 'error' }));
  }
  render () {
        return (...);
  }
}
@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Oct 10, 2017

Member

It would be possible I think to write some kind of sagaConnector HOC, which could glue ur trigger and success actions, inject appropriate handlers into ur wrapped component and effectively allow u to bypass redux's store and just 'react' directly with setState in ur component.

Should be fairly easy to implement this in user land using context API.

Member

Andarist commented Oct 10, 2017

It would be possible I think to write some kind of sagaConnector HOC, which could glue ur trigger and success actions, inject appropriate handlers into ur wrapped component and effectively allow u to bypass redux's store and just 'react' directly with setState in ur component.

Should be fairly easy to implement this in user land using context API.

@th0rv

This comment has been minimized.

Show comment
Hide comment
@th0rv

th0rv Nov 27, 2017

hey there! any updates on this point?

th0rv commented Nov 27, 2017

hey there! any updates on this point?

@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Nov 27, 2017

Member

Unfortunately not, it is also not a priority right now for the core team of redux-saga. I really wish somebody could pitch in and provide at least API draft, that way it would be easier to work on this for anyone.

Member

Andarist commented Nov 27, 2017

Unfortunately not, it is also not a priority right now for the core team of redux-saga. I really wish somebody could pitch in and provide at least API draft, that way it would be easier to work on this for anyone.

@salterok

This comment has been minimized.

Show comment
Hide comment
@salterok

salterok Feb 14, 2018

@brettstack @norbertpy I use helper function in one of my work projects. Seems like it solves your case.

There is middleware.run(saga, ...args) method described in docs which supposed to run dynamic saga.

So i do something like this:

// middleware.js
import createSagaMiddleware from "redux-saga";

const sagaMiddleware = createSagaMiddleware();

// 'sagaMiddleware' should be registered as middleware in 'createStore'
// otherwise you have no access to store 

export function execute(saga, ...args) {
    return sagaMiddleware.run(saga, ...args).done;
}

And in my component use exported execute function:

import * as React from "react";
import { execute } from "./middleware";

class ExampleComponent extends React.Component {
    async onLoadClicked() {
        this.setState({
            loading: true,
        });
        const result = await execute(mySagaHandler);
        this.setState({
            loading: false,
            result
        });
    }
    render() {
        return (...);
    }
}

Note that mySagaHandler can read/write to store and use other effects.

salterok commented Feb 14, 2018

@brettstack @norbertpy I use helper function in one of my work projects. Seems like it solves your case.

There is middleware.run(saga, ...args) method described in docs which supposed to run dynamic saga.

So i do something like this:

// middleware.js
import createSagaMiddleware from "redux-saga";

const sagaMiddleware = createSagaMiddleware();

// 'sagaMiddleware' should be registered as middleware in 'createStore'
// otherwise you have no access to store 

export function execute(saga, ...args) {
    return sagaMiddleware.run(saga, ...args).done;
}

And in my component use exported execute function:

import * as React from "react";
import { execute } from "./middleware";

class ExampleComponent extends React.Component {
    async onLoadClicked() {
        this.setState({
            loading: true,
        });
        const result = await execute(mySagaHandler);
        this.setState({
            loading: false,
            result
        });
    }
    render() {
        return (...);
    }
}

Note that mySagaHandler can read/write to store and use other effects.

@ninjarails

This comment has been minimized.

Show comment
Hide comment
@ninjarails

ninjarails Feb 21, 2018

I am much interested in this topic but it's fruitless. redux-saga is designed for side effects. it can track each event but can't track every event.

ninjarails commented Feb 21, 2018

I am much interested in this topic but it's fruitless. redux-saga is designed for side effects. it can track each event but can't track every event.

@a-type

This comment has been minimized.

Show comment
Hide comment
@a-type

a-type Mar 7, 2018

I did some exploration into this problem with redux-facet. Specifically, a kind of hack which wraps / intercepts saga generator effects: https://github.com/Bandwidth/redux-facet/blob/master/src/facetSaga.js

The idea in this library is that every action gets tagged with a meta.facetName value so that it can be traced back to the component which dispatched it.

The facetSaga wrapper intercepts all put effects and references the original action which triggered the saga, copying its meta.facetName value onto the outgoing actions.

This makes it (kind of) easy to keep track of the thread from the REQUEST action to the COMPLETE (or FAILED) action.

The same technique might be useful in resolving a promise on the original action.

For instance, a generator wrapper could be written like so...

const smartActionCreator = (data) => {
  let resolve;
  let reject;
  // there's probably a way cleaner way to do this:
  const promise = new Promise((res, rej) => { resolve = res; reject = rej; });
  return {
    type: 'SOME_ACTION',
    payload: data,
    meta: { resolve, reject, promise },
  };
};

function trackCompletion(saga) {
  return function*(action) {
    // ignore 'dumb' actions without promises
    if (!action.meta.promise) {
      yield* saga(action);
      return;
    }

    const { resolve, reject } = action.meta;

    const iterator = saga(action);
    let lastResult;

    while (true) {
      let next;
      if (lastResult instanceof Error) {
        // an error was thrown, reject the action promise
        reject(lastResult);
        next = iterator.throw(lastResult);
      } else {
        next = iterator.next(lastResult);
      }

      if (next.done) {
        // the saga has completed!
        resolve(next.value);
        return next.value;
      }

      // keep generating values
      lastResult = yield next.value;
    }
  };
}

// usage

// using the saga wrapper
function* handleAction(action) { /* ... */ }
function* () {
  yield takeEvery('SOME_ACTION', trackCompletion(handleAction));
}

// dispatching the action and awaiting saga completion
const { meta: { promise } } = dispatch(smartActionCreator(data));
await promise;
// saga is complete.

Code has not been tested, just spitballing

This could probably be streamlined by doing the async action stuff in middleware; there are many solutions for that.

I'm thinking of ways that the redux-saga library could more directly support the kind of pattern seen above without having to do a generator-wrapper (though I don't think the wrapper is too complex by itself).

Perhaps a way to add middleware to the redux-saga library itself? Some place we could put the code above so that it would be applied to every saga? Of course, not every saga has an action as a parameter, so that might not make much sense. Perhaps this wrapper function is good enough.

a-type commented Mar 7, 2018

I did some exploration into this problem with redux-facet. Specifically, a kind of hack which wraps / intercepts saga generator effects: https://github.com/Bandwidth/redux-facet/blob/master/src/facetSaga.js

The idea in this library is that every action gets tagged with a meta.facetName value so that it can be traced back to the component which dispatched it.

The facetSaga wrapper intercepts all put effects and references the original action which triggered the saga, copying its meta.facetName value onto the outgoing actions.

This makes it (kind of) easy to keep track of the thread from the REQUEST action to the COMPLETE (or FAILED) action.

The same technique might be useful in resolving a promise on the original action.

For instance, a generator wrapper could be written like so...

const smartActionCreator = (data) => {
  let resolve;
  let reject;
  // there's probably a way cleaner way to do this:
  const promise = new Promise((res, rej) => { resolve = res; reject = rej; });
  return {
    type: 'SOME_ACTION',
    payload: data,
    meta: { resolve, reject, promise },
  };
};

function trackCompletion(saga) {
  return function*(action) {
    // ignore 'dumb' actions without promises
    if (!action.meta.promise) {
      yield* saga(action);
      return;
    }

    const { resolve, reject } = action.meta;

    const iterator = saga(action);
    let lastResult;

    while (true) {
      let next;
      if (lastResult instanceof Error) {
        // an error was thrown, reject the action promise
        reject(lastResult);
        next = iterator.throw(lastResult);
      } else {
        next = iterator.next(lastResult);
      }

      if (next.done) {
        // the saga has completed!
        resolve(next.value);
        return next.value;
      }

      // keep generating values
      lastResult = yield next.value;
    }
  };
}

// usage

// using the saga wrapper
function* handleAction(action) { /* ... */ }
function* () {
  yield takeEvery('SOME_ACTION', trackCompletion(handleAction));
}

// dispatching the action and awaiting saga completion
const { meta: { promise } } = dispatch(smartActionCreator(data));
await promise;
// saga is complete.

Code has not been tested, just spitballing

This could probably be streamlined by doing the async action stuff in middleware; there are many solutions for that.

I'm thinking of ways that the redux-saga library could more directly support the kind of pattern seen above without having to do a generator-wrapper (though I don't think the wrapper is too complex by itself).

Perhaps a way to add middleware to the redux-saga library itself? Some place we could put the code above so that it would be applied to every saga? Of course, not every saga has an action as a parameter, so that might not make much sense. Perhaps this wrapper function is good enough.

@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Mar 9, 2018

Member

The idea itself is ok, I'd like to enable connecting such pieces as components and sagas more tightly for some use cases, but a builtin mechanism just for this doesn't seem right - unless we figure out better API. Need to think about something more universal.

Member

Andarist commented Mar 9, 2018

The idea itself is ok, I'd like to enable connecting such pieces as components and sagas more tightly for some use cases, but a builtin mechanism just for this doesn't seem right - unless we figure out better API. Need to think about something more universal.

@a-type

This comment has been minimized.

Show comment
Hide comment
@a-type

a-type Mar 12, 2018

I agree, I don't think it's a good idea to make this a built-in behavior.

I've been doing some more thinking on this problem. I think the approach may be flawed from the start. I'm starting to believe that a combination of 1) more case-specific sagas and 2) more usage of saga composition may provide a cleaner answer.

Consider the original form-submit use case. As boilerplate-y as it is to store loading state in Redux, if we consent to do so, we can write a pretty readable saga to achieve our aims...

// generic state reducer for 'raw' profile data. Can be combined in selectors
// to provide data to any view which deals with profiles
const defaultProfileState = null;
const profileReducer = (state = defaultProfileState, { type, payload }) => {
  switch (type) {
    case 'PROFILE/NEW_DATA': return payload.data;
    default: return state;
  }
};

// view state reducer for any state related to the profile edit form
const defaultProfileEditState = { loading: false, error: null };
const profileEditFormReducer = (state = defaultProfileEditState, { type, payload }) => {
  switch (type) {
    case 'PROFILE_EDIT_FORM/SUBMIT': return { ...state, loading: true };
    case 'PROFILE_EDIT_FORM/RESET': return defaultProfileEditState;
    case 'PROFILE_EDIT_FORM/SHOW_ERROR': return { ...state, error: payload.message };
    default: return state;
  }
};

// generic, reusable data saga which encapsulates updating a profile,
// including dispatching data-related actions to update the raw data in the store.
// returns the updated profile.
// this saga isn't directly connected to any `take` effects. It is only called
// by view-related sagas.
function* profileUpdateSaga(profileData) {
  // let's assume this throws an error on 4xx/5xx
  const apiResponse = yield call(sdk.updateProfile, profileData);
  // put a data-centric action to update the raw data in the store
  yield put(profileActions.newData(apiResponse.body);
  return apiResponse.body;
}

// view-specific saga which delegates profile update to the general saga,
// but also adds effects which relate directly to the profile edit form view.
function* profileEditFormSaga({ payload }) {
  try {
	const profile = yield call(profileUpdateSaga, payload.data);
	// we can freely include view-specific concerns here in a readable way
	yield put(profileEditFormActions.reset());
	// even other side-effects like routing, since this saga will only ever represent
	// the experience on the profile edit form view
	yield call(navigation.goTo, '/home'); // mock / generic navigation service
  } catch (err) {
    yield put(profileEditFormActions.showError(err.message));
  }
}; 

function* watchProfileEditFormSubmit() {
  yield takeLatest('PROFILE_EDIT_FORM/SUBMIT', profileEditFormSaga);
}

This may seem like redux-saga 101... cause it is... but I know I've personally gone really far down the rabbit hole in search of a way to effectively achieve this kind of pattern where we can have generic, reusable logic for common data and still have very specific and easy-to-follow logic for views. I think the desire to manage loading state within the component itself is coming from the same need which this pattern addresses: we want to have a place which can plainly manage highly specialized user flows which come from specific views without polluting our generic logic with edge-cases. Saga composition, based on my explorations, still seems like the best way to do this (even if it means we must move some state to Redux like loading or error alerts). It just seems like once you use redux-saga, it needs to be the exclusive way that you handle asynchronous operations. As a middleware which has no concept of the view, it can't easily be sewn together with view-based logic like other approaches (thunks, etc) can. Personally, I'm cool with that.

a-type commented Mar 12, 2018

I agree, I don't think it's a good idea to make this a built-in behavior.

I've been doing some more thinking on this problem. I think the approach may be flawed from the start. I'm starting to believe that a combination of 1) more case-specific sagas and 2) more usage of saga composition may provide a cleaner answer.

Consider the original form-submit use case. As boilerplate-y as it is to store loading state in Redux, if we consent to do so, we can write a pretty readable saga to achieve our aims...

// generic state reducer for 'raw' profile data. Can be combined in selectors
// to provide data to any view which deals with profiles
const defaultProfileState = null;
const profileReducer = (state = defaultProfileState, { type, payload }) => {
  switch (type) {
    case 'PROFILE/NEW_DATA': return payload.data;
    default: return state;
  }
};

// view state reducer for any state related to the profile edit form
const defaultProfileEditState = { loading: false, error: null };
const profileEditFormReducer = (state = defaultProfileEditState, { type, payload }) => {
  switch (type) {
    case 'PROFILE_EDIT_FORM/SUBMIT': return { ...state, loading: true };
    case 'PROFILE_EDIT_FORM/RESET': return defaultProfileEditState;
    case 'PROFILE_EDIT_FORM/SHOW_ERROR': return { ...state, error: payload.message };
    default: return state;
  }
};

// generic, reusable data saga which encapsulates updating a profile,
// including dispatching data-related actions to update the raw data in the store.
// returns the updated profile.
// this saga isn't directly connected to any `take` effects. It is only called
// by view-related sagas.
function* profileUpdateSaga(profileData) {
  // let's assume this throws an error on 4xx/5xx
  const apiResponse = yield call(sdk.updateProfile, profileData);
  // put a data-centric action to update the raw data in the store
  yield put(profileActions.newData(apiResponse.body);
  return apiResponse.body;
}

// view-specific saga which delegates profile update to the general saga,
// but also adds effects which relate directly to the profile edit form view.
function* profileEditFormSaga({ payload }) {
  try {
	const profile = yield call(profileUpdateSaga, payload.data);
	// we can freely include view-specific concerns here in a readable way
	yield put(profileEditFormActions.reset());
	// even other side-effects like routing, since this saga will only ever represent
	// the experience on the profile edit form view
	yield call(navigation.goTo, '/home'); // mock / generic navigation service
  } catch (err) {
    yield put(profileEditFormActions.showError(err.message));
  }
}; 

function* watchProfileEditFormSubmit() {
  yield takeLatest('PROFILE_EDIT_FORM/SUBMIT', profileEditFormSaga);
}

This may seem like redux-saga 101... cause it is... but I know I've personally gone really far down the rabbit hole in search of a way to effectively achieve this kind of pattern where we can have generic, reusable logic for common data and still have very specific and easy-to-follow logic for views. I think the desire to manage loading state within the component itself is coming from the same need which this pattern addresses: we want to have a place which can plainly manage highly specialized user flows which come from specific views without polluting our generic logic with edge-cases. Saga composition, based on my explorations, still seems like the best way to do this (even if it means we must move some state to Redux like loading or error alerts). It just seems like once you use redux-saga, it needs to be the exclusive way that you handle asynchronous operations. As a middleware which has no concept of the view, it can't easily be sewn together with view-based logic like other approaches (thunks, etc) can. Personally, I'm cool with that.

@dcbrwn

This comment has been minimized.

Show comment
Hide comment
@dcbrwn

dcbrwn Mar 24, 2018

Stumbled across this topic while googling for related problem. Currently I solved the need for communication between components and sagas like this https://github.com/dcbrwn/redux-semaphore.
Bunch of helper methods encapsulating boilerplate help to save a few keystrokes. For example:

const ENTITY_FETCH_REQUEST = 'ENTITY_FETCH_REQUEST';
const ENTITY_FETCH_SUCCESS = 'ENTITY_FETCH_SUCCESS';
const ENTITY_FETCH_FAIL = 'ENTITY_FETCH_FAIL';

async function fetchAndWait(requestAction, dispatch) {
  dispatch(requestAction);
  const actionTypePrefix = requestAction.type.slice(0, -('REQUEST'.length));
  const failActionType = actionTypePrefix + 'FAIL';
  const successActionType = actionTypePrefix + 'SUCCESS';
  return await semaphore(
    action => action.type === successActionType && action.payload.requestAction === requestAction,
    action => action.type === failActionType && action.payload.requestAction === requestAction);
}

// And somewhere in component

async _handleReloadClick(event) {
  event.stopPropagation();
  this._isLoading = true;

  try {
    await fetchAndWait(fetchSomeData(), this.dispatch);
  } catch (error) {
    this.dispatch(addNotification('Failed to fetch data'));
  }

  this._isLoading = false;
}

dcbrwn commented Mar 24, 2018

Stumbled across this topic while googling for related problem. Currently I solved the need for communication between components and sagas like this https://github.com/dcbrwn/redux-semaphore.
Bunch of helper methods encapsulating boilerplate help to save a few keystrokes. For example:

const ENTITY_FETCH_REQUEST = 'ENTITY_FETCH_REQUEST';
const ENTITY_FETCH_SUCCESS = 'ENTITY_FETCH_SUCCESS';
const ENTITY_FETCH_FAIL = 'ENTITY_FETCH_FAIL';

async function fetchAndWait(requestAction, dispatch) {
  dispatch(requestAction);
  const actionTypePrefix = requestAction.type.slice(0, -('REQUEST'.length));
  const failActionType = actionTypePrefix + 'FAIL';
  const successActionType = actionTypePrefix + 'SUCCESS';
  return await semaphore(
    action => action.type === successActionType && action.payload.requestAction === requestAction,
    action => action.type === failActionType && action.payload.requestAction === requestAction);
}

// And somewhere in component

async _handleReloadClick(event) {
  event.stopPropagation();
  this._isLoading = true;

  try {
    await fetchAndWait(fetchSomeData(), this.dispatch);
  } catch (error) {
    this.dispatch(addNotification('Failed to fetch data'));
  }

  this._isLoading = false;
}
@ovaldi

This comment has been minimized.

Show comment
Hide comment
@ovaldi

ovaldi commented Apr 5, 2018

Mark!

@Andarist

This comment has been minimized.

Show comment
Hide comment
@Andarist

Andarist Apr 5, 2018

Member

New library got released that might help some of you solving this problem - https://github.com/erikras/redux-promise-listener

Member

Andarist commented Apr 5, 2018

New library got released that might help some of you solving this problem - https://github.com/erikras/redux-promise-listener

@klis87

This comment has been minimized.

Show comment
Hide comment
@klis87

klis87 Apr 5, 2018

Contributor

Also I am creator of library https://github.com/klis87/redux-saga-requests for simplification of AJAX requests - promises like those with Redux-thunk are built-in there. Disadvantage is that promisification is just for AJAX request, but usually this is what most people need anyway :)

Contributor

klis87 commented Apr 5, 2018

Also I am creator of library https://github.com/klis87/redux-saga-requests for simplification of AJAX requests - promises like those with Redux-thunk are built-in there. Disadvantage is that promisification is just for AJAX request, but usually this is what most people need anyway :)

@BlueAccords

This comment has been minimized.

Show comment
Hide comment
@BlueAccords

BlueAccords May 7, 2018

Somewhat related, but there's a similar issue here #161 that discusses trying to return a promise from an action dispatch in case anyone stumbles upon this issue and wants to look at possible solutions to their problem.

The referenced issue is specific to redux-saga + redux-form integration but still has some solutions worth looking at imo.

Additionally, this library https://github.com/diegohaz/redux-saga-thunk might be of interest as well as another potential solution.

BlueAccords commented May 7, 2018

Somewhat related, but there's a similar issue here #161 that discusses trying to return a promise from an action dispatch in case anyone stumbles upon this issue and wants to look at possible solutions to their problem.

The referenced issue is specific to redux-saga + redux-form integration but still has some solutions worth looking at imo.

Additionally, this library https://github.com/diegohaz/redux-saga-thunk might be of interest as well as another potential solution.

@URvesh109

This comment has been minimized.

Show comment
Hide comment
@URvesh109

URvesh109 May 8, 2018

/**
* Lifecycle method
*/
async componentDidMount() {
try {
this.setState({loading: true});
await this.props.getCities();
const {cities} = this.props;
this.setState({loading: false, tempCities: cities});
} catch (e) {
this.setState({loading: false});
}
}

I was using redux-saga for my app, then I realise to achieve above I have to use library for this. Is there any simple way to achieve above behaviour in redux-saga?

URvesh109 commented May 8, 2018

/**
* Lifecycle method
*/
async componentDidMount() {
try {
this.setState({loading: true});
await this.props.getCities();
const {cities} = this.props;
this.setState({loading: false, tempCities: cities});
} catch (e) {
this.setState({loading: false});
}
}

I was using redux-saga for my app, then I realise to achieve above I have to use library for this. Is there any simple way to achieve above behaviour in redux-saga?

@vedmant

This comment has been minimized.

Show comment
Hide comment
@vedmant

vedmant May 13, 2018

I just started with saga and this is first problem that popped up almost immediately. Using redux to store totally everything is a real pain cause I need also to store every field error messages for every form, I need to dispatch actions to clear error messages every time components unmounts and mounts to make sure that previous error is not shown before form is sent. I also use persistence and I need to exclude all that loading and errors states for every form from persistence. And this is awkward, I do everything to simulate component stare using redux state writing like 5x time more code.

vedmant commented May 13, 2018

I just started with saga and this is first problem that popped up almost immediately. Using redux to store totally everything is a real pain cause I need also to store every field error messages for every form, I need to dispatch actions to clear error messages every time components unmounts and mounts to make sure that previous error is not shown before form is sent. I also use persistence and I need to exclude all that loading and errors states for every form from persistence. And this is awkward, I do everything to simulate component stare using redux state writing like 5x time more code.

@saboya

This comment has been minimized.

Show comment
Hide comment
@saboya

saboya May 14, 2018

And this is why redux isn't for everyone. It does encourage managing ALL of your state in one place, including loading states and such.

While it's understandable to look for a solution outside the flow due to the awkwardness of the solution that redux provides to those issues, it's inherently wrong, and it's completely outside the scope of this project to solve this "problem".

redux-saga is a clean and declarative way to implement side-effects, and that's it.

saboya commented May 14, 2018

And this is why redux isn't for everyone. It does encourage managing ALL of your state in one place, including loading states and such.

While it's understandable to look for a solution outside the flow due to the awkwardness of the solution that redux provides to those issues, it's inherently wrong, and it's completely outside the scope of this project to solve this "problem".

redux-saga is a clean and declarative way to implement side-effects, and that's it.

@klis87

This comment has been minimized.

Show comment
Hide comment
@klis87

klis87 May 14, 2018

Contributor

Actually I think Redux encourages keeping GLOBAL state in 1 place, I read in numerous places thats it is totally fine to use for instance React state for local things, for example for form messages/errors. This is what I do, and many libraries like Formik as well.

Contributor

klis87 commented May 14, 2018

Actually I think Redux encourages keeping GLOBAL state in 1 place, I read in numerous places thats it is totally fine to use for instance React state for local things, for example for form messages/errors. This is what I do, and many libraries like Formik as well.

@vedmant

This comment has been minimized.

Show comment
Hide comment
@vedmant

vedmant May 14, 2018

@saboya I'd say that you are talking about redux-saga isn't for everyone. Cause use redux with redux-thunk and can easily get promise returned from the dispatch:

handleLogin() {
  this.setState({loading: true})
  this.props.login(this.state.form)
    .then(() => this.navigate('DashboardScreen'))
    .catch(err => this.setState({error: err.response.data})
    .then(() => this.setState({loading: false})
}

And save a lot of coding without the need of implementing multiple additional actions, especially when this things just don't belong to global state as they have to be cleared after component unmount.

vedmant commented May 14, 2018

@saboya I'd say that you are talking about redux-saga isn't for everyone. Cause use redux with redux-thunk and can easily get promise returned from the dispatch:

handleLogin() {
  this.setState({loading: true})
  this.props.login(this.state.form)
    .then(() => this.navigate('DashboardScreen'))
    .catch(err => this.setState({error: err.response.data})
    .then(() => this.setState({loading: false})
}

And save a lot of coding without the need of implementing multiple additional actions, especially when this things just don't belong to global state as they have to be cleared after component unmount.

@saboya

This comment has been minimized.

Show comment
Hide comment
@saboya

saboya May 14, 2018

Keeping state in one place is what redux encourages.. It's up to each developer to decide if they belong there or not.

redux-thunk returns Promise from dispatch and that allows some stuff, sure. But if you landed here in redux-saga it's because you probably realized that it doesn't scale to large-scale apps. Each solution has its pros and cons, but trying to turn redux-saga into something it's not isn't going to solve design problems.

saboya commented May 14, 2018

Keeping state in one place is what redux encourages.. It's up to each developer to decide if they belong there or not.

redux-thunk returns Promise from dispatch and that allows some stuff, sure. But if you landed here in redux-saga it's because you probably realized that it doesn't scale to large-scale apps. Each solution has its pros and cons, but trying to turn redux-saga into something it's not isn't going to solve design problems.

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