pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.49k stars 1.45k forks source link

Cancellation in long-living effects works globally for all stores. #1989

Closed kacpermazurkiewicz closed 1 year ago

kacpermazurkiewicz commented 1 year ago

Description

It seems that for the long-living effects where we set the cancellable id to eg. CancelID.self where: enum CancelID: Hashable {} and then call a canceling effect like .cancel(id: CancelID.self) the cancellation is global, not local for the store from within it was called. So it affects all stores that use the CancelID.self.

An example: We have an instance of an example TCA component StoreOf<ExampleComponent> that in the reducer has a long-running effect to listen to some changes (let's name this instance a store1) and we have another instance of the same component StoreOf<ExampleComponent> in another screen that does exactly the same but it is initialized independently (eg. let's call it store2) and we send to the store1 the action which cancels the long-running effect using the CancelID.self - then in both stores (store1 and store2), the effect will be canceled.

Checklist

Expected behavior

I would expect that the cancellation effect will have an effect on the store from which it was called.

Actual behavior

When we have two separate instances of the store eg. let store1: StoreOf<ExampleComponent> let store2: StoreOf<ExampleComponent>

and I cancel the long-living effect in store1 by calling eg. ViewStore(store).send(.cancelLongRunningEffect) then both stores - store1 and store2 will receive the cancellation.

Steps to reproduce

An example was recreated in the following repository: https://github.com/kacpermazurkiewicz/TheComposableArchitectureGlobalCancellationIssue

In short:

Step 1. Create an example TCA Component with a long-living effect and a way for cancelling it - https://github.com/kacpermazurkiewicz/TheComposableArchitectureGlobalCancellationIssue/blob/main/ComposableArchitectureIssueCancellableID/TCAContentView/ContentView%2BReducer.swift

Step 2. Initialize two independent stores of the created component and pass it to some view: https://github.com/kacpermazurkiewicz/TheComposableArchitectureGlobalCancellationIssue/blob/main/ComposableArchitectureIssueCancellableID/ComposableArchitectureIssueCancellableIDApp.swift

Step 3. Send cancellation effect to one of the stores (in my case I used the Button view to trigger the action by tapping on it) https://github.com/kacpermazurkiewicz/TheComposableArchitectureGlobalCancellationIssue/blob/main/ComposableArchitectureIssueCancellableID/ContentView.swift

Result: You will notice that both instances of the store will cancel its long-living effect.

The Composable Architecture version information

0.52.0

Destination operating system

iOS 16

Xcode version information

Xcode 14.2

Swift Compiler version information

swift-driver version: 1.62.15 Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51)
tgrapperon commented 1 year ago

Hey @kacpermazurkiewicz! This is a known limitation. Cancellation identifiers are not automatically contextualized, and you should append a bit of information yourself if you want to differentiate them (you can even use a kind of path @Dependency for that, but again, it requires some manual intervention to append the path components). The upcoming navigation update could improve the situation in this area.

kacpermazurkiewicz commented 1 year ago

Hey @tgrapperon! Thank you for the quick answer! At the moment we will go with the proposed workaround.

Great to hear that the improvement is ongoing 👍 I have to say that it wasn't so obvious while working on the component and we realized that the separate component was affected. Maybe it is worth mentioning in the documentation that the cancellation identifiers are global (at least at the moment)? 🤔 The documentation was the first place that I checked while searching for some information. Intuitively I guess that everyone treats 'Store' as an isolated component.

Since the limitation is known - should I close this gh issue?

mbrandonw commented 1 year ago

Hi @kacpermazurkiewicz, as @tgrapperon explained this definitely is a known behavior and so not really considered a bug. We definitely should improve the documentation for this.

The navigation tools coming will help with this as far as composed features are concerned, but separate stores will continue to share the same global bag of cancellation IDs, and so will not directly fix the problem you are encountering.

However, I will say that we do have changes planned for shortly after the navigation tools are released that will properly quarantine effects to their respective stores, which means it will eventually be fixed.

I think we can close this issue for now, but if you wanted to open a discussion to discuss further that would be fine!

kacpermazurkiewicz commented 1 year ago

Thank you @mbrandonw and @tgrapperon for your quick answers! As the limitation is known - for now, I am closing the issue.