-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Break retain cycle with Action.unsafeCocoaAction #2910
Conversation
public var unsafeCocoaAction: CocoaAction { | ||
return associatedObject(self, key: &unsafeCocoaActionKey, initial: { host in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private var unsafeCocoaActionKey
should be removed as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -176,12 +177,15 @@ class ActionSpec: QuickSpec { | |||
let result = action.apply(0).first() | |||
expect(result?.value) == 1 | |||
expect(values).toEventually(equal([ true, false, true ])) | |||
|
|||
_ = cocoaAction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, what is this assignation here for ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's to keep cocoaAction
from deallocating before the end of the test—because when it deallocates, the producer will complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
👍 |
This kind of change should not be hidden behind a simple "fixed retain cycle" in the release notes. While this code works fine in RAC 4.1, it compiles and fails silently in RAC 4.2: someButton.addTarget(someAction.unsafeCocoaAction, action: CocoaAction.selector) I feel like this change kind of defeats the purpose of having |
Sorry for the inconvenience! Let's move this discussion to #2983. I'll try to comment there soon, but it may to happen this week because of WWDC. |
Fixes #2233.