Allowing set on disposeBag

[tylerwilson]

I ran into an issue where we are subscribing to changes of a UITextField in a UITableViewCell instance. Because the cells can be re-used, we should dispose in the prepareForReuse - we cannot wait for a deinit. Otherwise, we end up with multiple subscribers on a UITextField which really confuses things.

I first attempted to implement via a

    override func prepareForReuse() {
        super.prepareForReuse()
        
        disposeBag = DisposeBag()
    }

But then we get the Swift error "Cannot assign to property: 'self' is immutable". There are a number of articles about this issue, and from what I have seen, the simplest and cleanest solution is to modify the HasDisposeBag protocol like so:

public protocol HasDisposeBag: class {

    /// a unique Rx DisposeBag instance
    var disposeBag: DisposeBag { get set }
}

This allows the setting of the disposeBag within the UITableViewCell and thus keep the subscribers to the correct set.

There may be other solutions available, but if this is a good solution, it would be great to see it changed in the original source (I am currently using a fork I made with just this one change).

Thank you

[ashfurrow]

Hey! Yeah that makes sense.

Can you update to the latest NSObject-Rx? The property is currently read/write so this might just work. Otherwise, totally welcome to a PR 😄

[tylerwilson]

FYI, I was using the latest version, and the set still failed with the error above. I am unsure why it fails for me but no in your unit tests. I am using Xcode 9 with Swift set to 3.2

Making the change to the protocol has side effects; that is, it would then only work with class types, not value types. I am unsure if you want to accept that limitation or not.

Here is a good article that goes into some detail on this issue.

[ashfurrow]

I see. This case is actually covered by our tests:

https://github.com/RxSwiftCommunity/NSObject-Rx/blob/58cd60b8dc7fc52f99423968ad339b0166eb3101/Demo/DemoTests/DemoTests.swift#L8-L13

But I noticed that the NSObject is a var. I think we've hit some sort of edge case of Swift's type resolution or something. But! I managed to find a workaround in our own implementation:

https://github.com/RxSwiftCommunity/NSObject-Rx/blob/58cd60b8dc7fc52f99423968ad339b0166eb3101/NSObject%2BRx.swift#L13-L16

Can you try using the following implementation in your cell subclass?

    override func prepareForReuse() {
        super.prepareForReuse()
        var mutatingSelf = self
        mutatingSelf.rx.disposeBag = DisposeBag()
    }

[ashfurrow]

(Sidenote below.)

I though I remembered solving a similar problem, of reusing cells getting rid of their subscriptions. I used the takeUntil() operator to terminate the signal ...

https://github.com/artsy/eidolon/blob/52ae03071efe2c71af2b857831a87e4c00a36c0e/Kiosk/Auction%20Listings/ListingsViewController.swift#L175

... whenever the preparingForReuse signal fired:

https://github.com/artsy/eidolon/blob/52ae03071efe2c71af2b857831a87e4c00a36c0e/Kiosk/ListingsCollectionViewCell.swift#L75

Both approaches have their merits – if I can clarify anything in our implementation, let me know 👍

[bobgodwinx]

@ashfurrow I just took a look at this new way.. can you elaborate on the advantages of your style? it looks pretty awesome to me.. I've been doing it like this

 private var bag = DisposeBag()
   
 override func prepareForReuse() {
        super.prepareForReuse()
        bag = DisposeBag()
    }

[ashfurrow]

I like how explicit it is, and how it removes an aspect of state from my cell class. It's largely a personal preference – either is fine, though in your code I'd make sure to call bag.dispose() before re-assigning it, to be on the safe side.

[hongmhoon]

This is my solution.

override func prepareForReuse() {
    super.prepareForReuse()
    rx.clearDisposeBag()
}

with

public extension Reactive where Base: AnyObject {
    public func clearDisposeBag() {
        synchronizedBag {
            objc_setAssociatedObject(base, &disposeBagContext, nil, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
        }
    }
}