When should I use a saga?

If you’ve ever worked on a project that uses redux-saga then you may have have a discussion similar to this one I had today.

During code review with Willem Handreck today we had a discussion about the appropriate place for a piece of logic related to a component in our React app. What followed was a 300 word comment in the pull request. Here’s a slightly longer version with added context so it makes more sense.

Our React app is a CMS allowing teams to manage videos that get displayed in a mobile app. A recent feature request came in to be able to archive a video.

We went through the usual process of planning and design and moved onto implementing it.

One particular section of code came under discussion, related to how we deal with a user clicking the archive button and how we manage that flow.

The UI presents a standard browser alert element with a warning in it and if the user clicks the continue button, then we go ahead and archive the video.

Here’s where we started…

We have this component.

<ArchiveButton onArchive={onArchive({ featuredVideo, key })} />

The onArchive function looks like this…

const mapDispatchToProps = (dispatch) => {
  onArchive: ({ featuredVideo, key }) => (event) => {
    event.preventDefault()
    confirmArchive({ featuredVideo, key }) && 
      dispatch(VideoActions.videoArchiveStart(key))
  }
}

This code works but I had some concerns about doing the work in-line in the redux connect function.

We use sagas as a tool to handle side-effects and control flow in small, manageable (testable) functions.

To determine if this is a good place for the logic I ask 3 questions:

1. Does it make it easy for someone to understand? ie. the simpler a function is, the easier it will be to understand.
2. Is it easy to test?
3. Is it easy to change or extend in the future?

When I look at this code through the lens of those questions, here’s what I see…

The onArchive function is simple, but it is doing 2 things. It could be simpler if it only did one thing, ie. dispatching an action representing that we want to trigger the archive flow.

It is not as easy to test in its current form due to the way we call the functions within mapDispatchToProps. We would probably end up mocking things that we don’t care about in our tests, ie. dispatch and the props object.

Extracted out into a separate function, this becomes easier and we remove the need for the mocks.

Lastly, how would we change or extend this in the future? In its current form, making a change or extending this could be done comfortably. My only concern is this overall container object growing in complexity in relation to those changes.

Moving into a saga allows us to isolate the actions of the form from the detail of connecting it to the store. I like to keep my container components free of logic if I can. I view them more as functions that know about other functions, so they connect the UI components with functions that provide data.

With that in mind, I would modify the onArchive function to be something like:

onArchive: (videoId) => 
  dispatch(VideoActions.videoArchiveStart(videoId, key))

Then in a saga, archiveVideo.js …

const start = function * ({ key, videoId }) {
 if(yield call(confirmAction, videoId)) {
   yield call(doTheArchiveWork, key)
 }
}
const confirmAction = function * (videoId) {
 // the code to display the alert
}
export default function * () {
 yield takeLatest(‘VIDEO_ARCHIVE_START’, start)
}

The beauty of this is that we are better able to read this code, it’s easier to test and as a consequence it will be easier to change.