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

Store.cancelAll() #335

Closed
wants to merge 1 commit into from
Closed

Store.cancelAll() #335

wants to merge 1 commit into from

Conversation

hfossli
Copy link

@hfossli hfossli commented Dec 18, 2020

Related to a discussion (coming in some minutes) on Swift Forums

@hfossli
Copy link
Author

hfossli commented Dec 18, 2020

Related to this discussion
https://forums.swift.org/t/how-can-the-composable-architecture-really-become-composable/43045/2

@hfossli
Copy link
Author

hfossli commented Jan 11, 2021

What do you think? Is this a good approach? If I write tests for this, can it be merged?

@hfossli hfossli mentioned this pull request Jan 13, 2021
@stephencelis
Copy link
Member

Hey @hfossli! Sorry for not weighing in. Your solutions have definitely been on our radar, and we've been thinking about the problem and various solutions, but we haven't quite decided what direction to take yet. Thanks for sharing, though! We'll try to take a closer look soon.

@hfossli-agens
Copy link

Thanks! That means a lot to hear :) It is definetely a tricky one. Looking forward to hear your thoughts. 🥰

@ferologics
Copy link
Contributor

Bump; I could really make use of something like this now. I assumed that when the state of the optional reducer is nil it's effects will be cancelled, but that's not the case for some reason.

@stephencelis
Copy link
Member

@hfossli 👋 We've been thinking about this problem for awhile and because we haven't had a solution we've been happy with, we've left all PRs open to think about.

Our Concurrency Beta provides one new solution, though, that we'd love your opinion on. Namely, it begins to return a task handle from ViewStore.send, which can be awaited and cancelled in the View.task modifier.

.task { await viewStore.send(.onAppear) }

When the view goes away, it cancels this task and all effects spun up by the associated action will be cancelled, as well.

@johnrogers
Copy link

@hfossli 👋 We've been thinking about this problem for awhile and because we haven't had a solution we've been happy with, we've left all PRs open to think about.

Our Concurrency Beta provides one new solution, though, that we'd love your opinion on. Namely, it begins to return a task handle from ViewStore.send, which can be awaited and cancelled in the View.task modifier.

.task { await viewStore.send(.onAppear) }

When the view goes away, it cancels this task and all effects spun up by the associated action will be cancelled, as well.

Glad to know there is a solution in the works. We are using TCA in a fairly large scale project at the moment (and loving it!) but have run into the same issues being documented here with long-living effects not being cancelled.

@stephencelis
Copy link
Member

@hfossli It's taken awhile but we've finally arrived at some cancellation helpers we're happy with, so we're going to close these related PRs out. I think we're a bit worried about store.cancelAll() being too blanketed, but are pretty happy with the following:

  • Async cancellation via await viewStore.send(.task).finish()
  • Cancellation tied to navigation APIs (this experiment is ongoing but we have some promising results so far)

Thanks again for taking the time to open these PRs and contribute to the conversation! If you have anything you'd like to discuss about our solutions, we'd love to hear about it. Please start a GitHub discussion!

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

5 participants