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

fix: floating promises and fork usage on login saga #5589

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

diegolmello
Copy link
Member

@diegolmello diegolmello commented Feb 21, 2024

Proposed changes

Fixes:

  • Two floating promises that would keep loginSuccess saga hanging
  • Redux-Saga's fork used instead of put

As far as I could tell, there was no visible bug.
takeLatest would cancel the loginSuccess running anyway, while the fork was declared at the top of the saga, so there was enough time to dispatch their own saga.

Issue(s)

How to test or reproduce

  • Open simulator with Reactotron connected
  • Do a login
  • See how LOGIN_SUCCESS saga would never finish (fixed on this PR)

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

CORE-91

@diegolmello diegolmello merged commit fb9c271 into develop Feb 23, 2024
1 of 2 checks passed
@diegolmello diegolmello deleted the fix.floating-promises-saga branch February 23, 2024 11:53
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 this pull request may close these issues.

None yet

2 participants