Fix retain cycle #30
Fix retain cycle #30
Conversation
|
Actually, still seeing the issue (though I'm pretty sure the change I made in this PR should still eliminate one retain cycle.) Documenting the issue here: #31 |
|
😱 good catch @sprynmr
I'll go ahead and merge this (giving that its true that this should take at least 1 retain cycle) Thanks for the BTW. I'm following Ash Furrow's idea and I'm offering access to the repo, etc to anyone that submits a |
|
Hey sure. At some point I think it would make sense to move to a protocol first approach for this, similar to the open PR #25. Not sure exactly what that would look like (maybe that PR is dead on), but something I was going to look at at some point here. |
|
Yes; definitely the protocol approach was always on my mind but at the time of the creation of this library i lacked enough knowledge to make it happen as I wanted it to be. I need to check #25 deeply because it could be the solution or might too close to only need very few tweaks 😄 I'll send you an invite to the org right now BTW |
Since `self` was being sent to the subjects with a buffer of 1, it was holding itself in the subjects indefinitely. The weakSelf I added in #30 only affects the closure itself, not the memory semantics of the subject storing itself. Looking back at RVMViewModel, they didn't have these as a replay subject, so I changed them to publish subjects to match their semantics. The throttling observable can keep it's subject since it's not sending values of self. I *think* this is correct, but please correct me on any of this. :)
Noticed a retain cycle when using RxViewModel.