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

Migrate from redux-saga to redux-logic #1934

Closed
4 of 25 tasks
outoftime opened this issue Nov 1, 2019 · 3 comments
Closed
4 of 25 tasks

Migrate from redux-saga to redux-logic #1934

outoftime opened this issue Nov 1, 2019 · 3 comments
Assignees

Comments

@outoftime
Copy link
Contributor

outoftime commented Nov 1, 2019

redux-saga is a very powerful tool but it also introduces a completely different approach to async programming than the one we use in all other JavaScript code, and requires anyone who uses it ramps up on a whole new vocabulary of effects. For new contributors who come in without experience with redux-saga, it bends the learning curve dauntingly.

redux-logic, while undeniably less popular than redux-saga, accomplishes the same basic goals using a much simpler approach. Logics are written using regular async/await code; if we need to deal with more complex scenarios, we can fall back to the underlying RxJS, which has a similar learning curve to redux-saga but is a much more general-purpose tool. This, anyway, should be rare.

It should be noted that redux-saga can do things that redux-logic can’t, in particular around cancellation of in-flight asynchronous actions. However dealing with cancellation correctly is quite tricky, and anyway we haven’t come across any situations where we need to.

Code structure

While our current saga code is organized into modules of related sagas, which are then consolidated into a single default export, logics should be one per module.

Note also that our sagas are often named after the action that triggers them; however, this presumes a 1:1 mapping between actions and sagas (or actions and logics) that does not actually obtain. So, let’s name logics after what they do, not what action(s) they consume.

Testing

As part of this transition, we should write the tests for the new logics in Jest, rather than Karma where the Saga tests live. Thus this should help us move toward #1443 as well.

Sagas to migrate

sagas/user.js

  • linkGithubIdentity
  • logout
  • startAccountMigration
  • unlinkGithubCredentials

sagas/errors.js

  • validateCurrentProject
  • updateProjectSource
  • toggleLibrary (note that this probably does not need to be its own logic—it should just be subsumed into validateCurrentProject)

sagas/ui.js

  • userDoneTyping
  • popOutProject
  • exportProject
  • projectSuccessfullySaved

sagas/clients.js

  • createSnapshot
  • exportProject
  • applicationLoaded

sagas/compiledProjects.js

  • validatedSource

sagas/projects.js

  • applicationLoaded
  • createProject
  • changeCurrentProject
  • projectExported
  • updateProjectSource,
  • userAuthenticated,
  • toggleLibrary,
  • loadAndBeautifyProjectSource,
  • archiveProject

sagas/manageUserState.js

This is definitely the most complex saga in the codebase.

  • manageUserState
@outoftime
Copy link
Contributor Author

This was worked on in #1655, #1672, and #1701

@joshling1919
Copy link
Contributor

Down to pick up the errors saga first! Will start looking at the previous PRs that @jwang1919 previously, as well as reading up on redux-saga and redux-logic.

@outoftime
Copy link
Contributor Author

Having done a fair amount of thought and research into this, I think the motivation here was good but the specific approach we decided on—migrating to redux-logic—is probably not the right one. In particular, redux-logic represents kind of an uncanny valley, with more power than we really need for most use cases that could practically be written as thunks, and not really powerful enough for a handful of complex use cases that are handled by redux-saga. Also, redux-logic is a pretty big contributor to bundle size (unlike thunk and saga).

My inclination would be to migrate most of the logics to thunks, and work on migrating most of the rest of the sagas to thunks as well, but keep redux-saga in the stack for the handful of cases where we need to represent more complex async/concurrency logic than thunks can do.

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

No branches or pull requests

3 participants