Skip to content
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

Reduce Disposable's sphere of influence #143

Closed
andersio opened this issue Dec 5, 2016 · 2 comments
Closed

Reduce Disposable's sphere of influence #143

andersio opened this issue Dec 5, 2016 · 2 comments
Labels

Comments

@andersio
Copy link
Member

andersio commented Dec 5, 2016

Originally: ReactiveCocoa/ReactiveCocoa#2044

Currently, Disposable is used in two key areas in the public API:

  1. Resource collection and cleanup.
    Signal generator, SignalProducer start side effect.

  2. Work cancellation.
    SignalProducer work cancellation, Signal observer detaching, Scheduler work cancellation.

While there is likely no sensible alternative for work cancellation tokens, the newly introduced Lifetime seems a nice contender to the use of Disposable for resource management in Signal and SignalProducer.

For example, we may have instead:

let producer = SignalProducer { observer, lifetime in
    // Attach a resource to the lifetime of the produced `Signal`.
    lifetime += signal.observe(observer)
}

let signal = Signal { observer, lifetime in
    // Attach a resource to the lifetime of the produced `Signal`.
    lifetime += upstream.observe(observer)
}

The key change here is that the "garbage collector" passed to the SignalProducer start handler are no longer disposable. CompositeDisposable as the public interface allows it to be disposed in the start handler, while it is supposed to be disposed only upon external interruption.

Edit: API changes

This would break sources that somehow relies on this though. I am not sure about the impact, but it feels like a fringe case.

The SignalProducer change would be breaking due to conflicting signature with existing initializer. Even if we replicate the CompositeDisposable API on Lifetime, it breaks whenever the second argument type is not inferred but explicitly specified.

The Signal change, however, is purely addictive to ReactiveSwift 1.0. The Signal change would be breaking due to closure return type not being able to be inferred from multi-line body.


As an extension, we may also introduce a new variant of observe, which associates an observation with the given Lifetime (or trigger signal).

property.signal.observe(during: reactive.lifetime) { event in
    // ...
}

The main advantage over take(until:) would be avoiding an extra Signal just to forward the events when used at the tail of a signal composition, at a cost of expanding the reach of the observation APIs.

@mdiep
Copy link
Contributor

mdiep commented Jan 29, 2017

I don't think it makes sense to add resource collection to Lifetime.

We have 3 jobs:

  • Cancellation
  • Cleanup
  • Lifetime

I think cleanup is more closely related to cancellation than lifetime.

@andersio
Copy link
Member Author

@mdiep The cleanup for producers does not differentiate between cancellation and termination. The collected resources are disposed of when the signal is terminated regardless how it terminates.

So it is actually a synonym of observing the termination of a Signal, just without affecting the Signal's lifetime like how a normal observer does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants