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

Deduplicate request if isFetching in Saga world #830

Closed
chengcyber opened this issue Feb 19, 2017 · 9 comments
Closed

Deduplicate request if isFetching in Saga world #830

chengcyber opened this issue Feb 19, 2017 · 9 comments
Labels

Comments

@chengcyber
Copy link

HI, guys. I got a problem to use saga instead of thunk functions.

Imaging a simple fetch action. I want fetch something and no more duplicate request if isFetching.

In thunk world, I can totally control my whole process of dispatch, which mean the 'FETCH_REQUEST' is also controlled.

const fetch = () => (dispatch, getState) => {
  
  // is fetching, no more process
  if (isFetching(getState())) Promise.resolve();
  
  dispatch({
    type: 'FETCH_REQUEST'
  })

  api.fetch
    .then(
      res => dispatch({type: 'FETCH_SUCCESS'}),
      err => dispatch({type: 'FETCH_FAILURE'})
    )
}

but in saga world, the 'FETCH_REQUEST' is a entry of work saga,

// action generator, a plain object
const fetch = () => ({
  type: 'FETCH_REQUEST'
})

// work saga to handle 'FETCH_REQUEST'
function* fetchSaga() {
  try {
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS'});
  } catch(err) {
    yield put({type: 'FETCH_FAILURE'});
  }
}

// watcher
function* rootSaga() {
  yield takeEvery('FETCH_REQUEST', fetchSaga);
}

Here is the problem, I try to "filter" my every 'FETCH_REQUEST' in my work saga to control workflow, but it will be harder in real world application, since other arguments specified in fetch function.

Can i have some trick, to control the 'FETCH_REQUEST' thing, just like thunk examples???
Anything helpful is welcomed 😀

@lovio
Copy link

lovio commented Feb 19, 2017

You can make a convention in your app.

LOAD_XXX is a set of actions to trigger api.fetch

And

XXX is object contains of three states of an api request: REQUEST, SUCCESS and FAILURE just like the example of real-world.

import createAction from 'redux-actions';

export const loadProducts = createActions('LOAD_PRODUCTS');

export const products = {
  request: createAction('PRODUCT_REQUEST'),
  success: createAction('PRODUCT_SUCCESS'),
  failure: createAction('PRODUCT_FAILURE'),
}

Here are some gists for your information

@UchihaVeha
Copy link

UchihaVeha commented Feb 19, 2017

instead if (isFetching(getState())) u can use if (select(isFetching))

function* fetchSaga() {
  try {
    const isFetching = yield select(isFetchingSelector);
    if(!isFetching){
       yield put({type: 'FETCH_START'});
       const res = yield call(api.fetch);
       yield put({type: 'FETCH_SUCCESS'});
    }   
  } catch(err) {
    yield put({type: 'FETCH_FAILURE'});
  }
}

After action 'FETCH_START' set isFetching to true in store. Don't forget after action 'FETCH_SUCCESS' and 'FETCH_FAILURE' set isFetching to false

@Andarist
Copy link
Member

Also it might be a good to decouple your saga from the state shape itself, im alway trying to do this, unless it would provide to copying reducers' logic in your sagas.

You could create a higher order saga for this, which would look something like this:

function* takeOneAndBlock(pattern, worker, ...args) {
  const task = yield fork(function* () {
    while (true) {
      const action = yield take(pattern)
      yield call(worker, ...args, action)
    }
  })
  return task
}

and use it like this:

function* fetchRequest() {
  try {
    yield put({type: 'FETCH_START'});
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS'});
  } catch (err) {
    yield put({type: 'FETCH_FAILURE'});
  }
}

yield takeOneAndBlock('FETCH_REQUEST', fetchRequest)

In my opinion this way is far way more elegant and also its behaviour can be easily customized depending on your needs.

@chengcyber
Copy link
Author

@Andarist
Thanks for replying.
I attempt this saga way, but i fail since some specified in request.

for exmaple, I want to fetch todo list, I got three todo list, 'all' 'active' 'completed',
they are specified in action.filter when dispatch ({type: 'FETCH_TODO_REQUEST, filter})
When i use takeOneAndBlock, I can just take one request specified with filter.
but, what i want is take each request once, and block duplicate request when is fetching each request specified with 'all' 'active' 'completed'.

I think a lot from your code.

function* customTake(pattern, worker, ...args) {
  const task = yield fork(function* () {
    let currentTask = {};
    while (true) {
      const action = yield take(pattern)
      const { filter } = action;
      if ( !currentTask[filter] ) {
           currentTask[filter] = yield fork(worker, ...args, action)  // ** 1 **
           currentTaks[filter] = null;  // ** 2 **
      }
    }
  })
  return task
}
function* fetchRequest() {
  try {
    yield put({type: 'FETCH_START'});
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS'});
  } catch (err) {
    yield put({type: 'FETCH_FAILURE'});
  }
}


yield customTake('FETCH_REQUEST', fetchRequest)

code line ** 1 **
if use call will pause the whole saga, which is not i want. so currentTask[filter] is used to manage each request.
code line ** 2 **
When i use fork, i can not block the function generator, and reset my currentTask[filter] to null, after one request forked

It is a large conflict in my design if the recommend saga way is used...

So, I try to control my request out of saga, write the business logic directly in container component.
With isFetching as a prop of component, I can control request now.

I wonder there is a better practice to implement what i want in Saga way?
Since I start to treat Saga as a brilliant way to control side-effects of actions, and this kind of things is not strong point of Saga.

@chengcyber
Copy link
Author

Thanks for replying

To @UchihaVeha
I tried these way to get state from my store, but i notice the logic trick here, if 'FETCH_REQUEST' is dispatched, 'isFetching' is always true, since reducer set isFetching as soon as this action dispatched.

So, i must use another action.type such as 'LOAD_PRODUCT' @lovio pointed.

To @lovio
It will work, but it changes the design if i transform my thunk design to saga design. which is not elegant...

@Andarist
Copy link
Member

Well, this can really be solved in numerous ways.

  1. with passing your map into your worker by reference, so it can reset in the end (mutation making this solution not elegant)
function* customTake(pattern, worker, ...args) {
  let currentTasks = {};
  const task = yield fork(function* () {
    while (true) {
      const action = yield take(pattern)
      const { filter } = action;
      if ( !currentTasks[filter] ) {
        currentTasks[filter] = yield fork(worker, ...args, currentTasks, action)
        currentTasks[filter] = null
      }
    }
  })
  return task
}

function* fetchRequest(currentTasks, action) {
  try {
    yield put({type: 'FETCH_START'});
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS'});
  } catch (err) {
    yield put({type: 'FETCH_FAILURE'});
  } finally {
    currentTasks[action.filter] = null
  }
}


yield customTake('FETCH_REQUEST', fetchRequest)
  1. inter saga communication with puts/takes
function* customTake(pattern, worker, ...args) {
  let currentTasks = {};
  const task = yield fork(function* () {
    yield takeEvery(pattern, function* () {
      if (currentTasks[filter]) {
        return
      }
      yield call(worker, ...args, action)
    })

    yield takeEvery([FETCH_SUCCESS, FETCH_FAILURE], function* (action) {
      currentTasks[action.filter] = null
    })
  })
  return task
}

function* fetchRequest(currentTasks, action) {
  try {
    yield put({type: 'FETCH_START', filter: action.filter});
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS', filter: action.filter});
  } catch (err) {
    yield put({type: 'FETCH_FAILURE', filter: action.filter});
  }
}

yield customTake('FETCH_REQUEST', fetchRequest)
  1. using my previous example
function* takeOneAndBlock(pattern, worker, ...args) {
  const task = yield fork(function* () {
    while (true) {
      const action = yield take(pattern)
      yield call(worker, ...args, action)
    }
  })
  return task
}

function* fetchRequest() {
  try {
    yield put({type: 'FETCH_START'});
    const res = yield call(api.fetch);
    yield put({type: 'FETCH_SUCCESS'});
  } catch (err) {
    yield put({type: 'FETCH_FAILURE'});
  }
}

yield takeOneAndBlock(action => action.type === 'FETCH_REQUEST' && action.filter === 'all', fetchRequest)
yield takeOneAndBlock(action => action.type === 'FETCH_REQUEST' && action.filter === 'active', fetchRequest)
yield takeOneAndBlock(action => action.type === 'FETCH_REQUEST' && action.filter === 'completed', fetchRequest)

and possibly many more, just using 3 top of my head

@chengcyber
Copy link
Author

@Andarist Thanks a lot! The solustions are impressive. which gives me a way to control the entry of worker saga. instead of control the most first request like 'FETCH_REQUEST' in redux-thunk way.

@gadiGuesty
Copy link

actionChannel doesn't resolve that?

@nullaus
Copy link

nullaus commented Mar 19, 2021

actionChannel doesn't resolve that?

I'm wondering the same thing. It appears that it does.

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

No branches or pull requests

6 participants