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: Adapt method is disposed before being completed #184

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

mashe
Copy link
Contributor

@mashe mashe commented Apr 4, 2022

Description

replace .flatMapLatest with .flatMap to avoid adapt to be disposed before being completed
related issue #183

Checklist

  • this PR is based on develop or a 'develop related' branch
  • the commits inside this PR have explicit commit messages
  • the Jazzy documentation has been generated (if needed -> Jazzy RxFlow)

@mashe mashe requested a review from twittemb April 4, 2022 13:56
@mashe mashe changed the title #183 Adapt method is disposed before being completed Fix: Adapt method is disposed before being completed Apr 4, 2022
@mashe mashe self-assigned this Apr 5, 2022
@mashe
Copy link
Contributor Author

mashe commented Apr 14, 2022

@twittemb Hey! Do you have any thoughts about this?

@twittemb
Copy link
Collaborator

twittemb commented Apr 14, 2022

Hi @mashe, sorry for the delay.

Could you tell me a bit more about your use case, what kind of long operation do you want to perform in the adapt function ?

I'm just wondering if adapt is well suited to handle long operations. Initially the goal was to be able to present an intermediate View before navigating (like a view for authentication).

If you perform a long operation in the function, how is the user feedback ? you display a progress spinner ?

thanks.

PS: I'm just trying to remember exactly why it was flatMapLatest in the first place :-)

@mashe
Copy link
Contributor Author

mashe commented Apr 14, 2022

@twittemb in my particular case I am using adapt to dismiss presented ViewController before presenting another one. This is a long operation. And another Step is received while dismissing that should update SplitViewController selection.

Since adapt returns Single, the only difference between .flatMap and .flatMapLatest is to skip an uncompleted Step or not. I do not think it is designed to skip Steps in mind. From my experience .flatMapLatest is used by default and I would write exactly the same way. So it looks pretty safe to use .flatMap instead, or am I missing something?

@mashe
Copy link
Contributor Author

mashe commented Apr 19, 2022

@twittemb any new thoughts?

@mashe
Copy link
Contributor Author

mashe commented Apr 27, 2022

@twittemb ping

Copy link
Collaborator

@twittemb twittemb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@twittemb twittemb merged commit 1f1ec3c into RxSwiftCommunity:main Apr 27, 2022
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.

2 participants