2

I use Combine in viewModels to update the views. But if I store the AnyCancellable objects into a set of AnyCancellable, the deinit method is never called. I use the deinit to cancel all cancellables objects.

struct View1: View {

    @ObservedObject var viewModel:ViewTextModel = ViewTextModel()
    @Injected var appActions:AppActions

    var body: some View {
        VStack {
            Text(self.viewModel.viewText)

            Button(action: {
                self.appActions.goToView2()
            }) {
                Text("Go to view \(self.viewModel.viewText)")
            }
        }
    }
}

class ViewTextModel: ObservableObject {
    @Published var viewText: String

    private var cancellables = Set<AnyCancellable>()

    init(state:AppState) {
        // initial state
        viewText = "view  \(state.view)"
        // updated state
        state.$view.removeDuplicates().map{ "view \($0)"}.assign(to: \.viewText, on: self).store(in: &cancellables)
    }

    deinit {
        cancellables.forEach { $0.cancel() }
    } 
}

Each time the view is rebuilt, a new viewmodel is instantiated but the old one is not destroyed. viewText attribute is updated on each instance with state.$view.removeDuplicates().map{ "view \($0)"}.assign(to: \.viewText, on: self).store(in: &cancellables)

If I don't store the cancellable object in the set, deinit is called but viewText is not updated if the state's changed for the current view.

Do you have an idea of ​​how to manage the update of the state without multiplying the instances of the viewmodel ?

Thanks

Dokoeur
  • 23
  • 1
  • 4
  • Interesting, I have the same problem (`deinit` not called) and I'm using `sink` (not `assign`). – User Sep 21 '20 at 17:27
  • My problem turned to be a memory leak in the view: https://stackoverflow.com/questions/59303789/swiftui-memory-leak-when-referencing-property-from-closure-inside-form-navigatio – User Sep 21 '20 at 18:36

1 Answers1

2

You could use sink instead of assign:

state.$view
    .removeDuplicates()
    .sink { [weak self] in self?.viewText = $0 }
    .store(in: &cancellables)

But I question the need for Combine here at all. Just use a computed property:

class ViewTextModel: ObservableObject {
    @Published var state: AppState

    var viewText: String { "view \(state.view)" }
}

UPDATE

If your deployment target is iOS 14 (or macOS 11) or later:

Because you are storing to an @Published, you can use the assign(to:) operator instead. It manages the subscription for you without returning an AnyCancellable.

state.$view
    .removeDuplicates()
    .map { "view \($0)" }
    .assign(to: &$viewText)
    // returns Void, so nothing to store
rob mayoff
  • 375,296
  • 67
  • 796
  • 848
  • Using ```sink``` instead of ```assign``` solves my problem thanks. The example here is deliberately simple to explain my problem. I prefer to use a viewmodel to prepare the data according to the state of the application. – Dokoeur Mar 18 '20 at 10:01
  • assign(to:on:) returns an AnyCancellable.. – tapmonkey Feb 19 '22 at 17:43
  • Yes, `assign(to:on:)` returns an `AnyCancellable`, which you then have to somehow store without creating a retain cycle. That was the problem with the code in the original question. – rob mayoff Feb 19 '22 at 18:48