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
notifications
as Signal.
#64
Conversation
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.
Awesome. Can't wait to do the same with UIControl
in RAC.
A few comments!
var observedObject: AnyObject? = NSObject() | ||
let producer = center.reactive.notifications(forName: nil, object: observedObject) | ||
observedObject = nil | ||
it("should be freed if it is not reachable and no observers is attached") { |
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.
Are
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.
Shouldn'tno observers
be singular?
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.
"observers" is plural.
You could also say "no observer is attached".
@@ -24,10 +24,10 @@ class FoundationExtensionsSpec: QuickSpec { | |||
let center = NotificationCenter.default | |||
|
|||
it("should send notifications on the producer") { | |||
let producer = center.reactive.notifications(forName: .racFirst) |
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.
Signal
|
||
disposable.dispose() | ||
} | ||
expect(signal).toNot(beNil()) |
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.
👍
|
||
disposable.dispose() |
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.
We need another test similar to this one to verify that the signal is not freed if there are other observers.
4622af2
to
4929a20
Compare
4929a20
to
d11bca4
Compare
} | ||
|
||
it("should be not freed if it still has one or more active observers") { |
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.
Thanks!
🎉 |
Signal
starts right away, so the weak capturing ofobserver
was removed.