Include KeyPath subscripts on Property types

[mdiep]

I think it'd make sense to include a way to create a new Property or BindingTarget from a Property with a KeyPath.

struct User {
  var name: String
}

let property = MutableProperty(User(name: "a"))
property[\.name] <~ someBindingSource

This is awkward to express otherwise. You can't really use <~ and are forced to observe the values manually.

I'd suggest making the subscript return a property, except that (1) this might make the ownership difficult to work out and (2) I'm not sure if we can express the Property/MutableProperty relationship in Swift's type system. But if it is possible, then it'd be nice.

[sharplet]

Interesting: on the left hand side of <~ you want (the semantics of) BindingTarget, and on the right, Property. This feels kind of lens-y to me — maybe we need a kind of mutable property (it's a protocol, right?) that is just a view into another, rather than owning its own storage?

[andersio]

If the intention is just to support bindings, property.reactive[\.name] <~ stream already works, just that it is part of ReactiveCocoa AFAICR.

It can be a retaining view with a custom binding target implementation. There was a very similar proposal before for DynamicProperty. ReactiveCocoa/ReactiveCocoa#3162

[sharplet]

How's this for a start? This is specifically not retaining the base property, so that it has the same lifetime.

final class MutablePropertyView<Base, Derived>: MutablePropertyProtocol {
    typealias Value = Derived

    let lifetime: Lifetime
    let producer: SignalProducer<Derived, NoError>
    let signal: Signal<Derived, NoError>

    private weak var base: MutableProperty<Base>?
    private let cache: Property<Derived>
    private let keyPath: WritableKeyPath<Base, Derived>

    init(base: MutableProperty<Base>, keyPath: WritableKeyPath<Base, Derived>) {
        self.base = base
        self.keyPath = keyPath

        cache = base.map(keyPath)
        lifetime = base.lifetime
        producer = cache.producer
        signal = cache.signal

        base.lifetime.observeEnded { _ = self }
    }

    var value: Derived {
        get { return cache.value }
        set { base?.modify { $0[keyPath: keyPath] = newValue } }
    }
}

extension MutableProperty {
    subscript<U>(keyPath: WritableKeyPath<Value, U>) -> MutablePropertyView<Value, U> {
        return MutablePropertyView(base: self, keyPath: keyPath)
    }
}

I'm not sure I have the bandwidth to drive the implementation this, but I had need for something like this in my current project so thought I'd share in case someone wants to work on it.

[andersio]

I don't see much of a concern for a view to strongly retain its backing property.

[sharplet]

So I tried a version where the view retains the base property, but that causes problems with this kind of thing:

property[\.name] <~ someBindingSource

In this expression, unless something retains the view, the whole binding will immediately complete. So in my example above, the base property retains the view until the end if its lifetime. If you then make the view strongly reference the base property, you have a leak. I haven't figured out a way to break this cycle yet.

So, it's possible to conform MutablePropertyProtocol and weakly reference the base property, because if the reference is nil you can just discard updates. This makes sense, because the view doesn't own the storage. However, ComposableMutableProperty is another thing: in order to implement modify(), you have to execute the closure, which gets difficult if your reference to the underlying storage has gone away and you don't have a value to pass in.

[sharplet]

Ok, I figured out how to get it to conform to ComposableMutablePropertyProtocol: https://gist.github.com/sharplet/4f23ac034d94c0e94b476d8a4ca5565c.

[sharplet]

I should probably just open a PR with what I have at this point, so we can more easily discuss the finer points of the implementation.

[andersio]

In this expression, unless something retains the view, the whole binding will immediately complete.

The view can supply a manual binding target — it has the access to the backing property to derive a binding target from it, instead of relying on the default implementation.

[sharplet]

The view can supply a manual binding target — it has the access to the backing property to derive a binding target from it, instead of relying on the default implementation.

Ok, so I think I understand what you mean now: the view retains the base property, but the binding target does not, so when the view immediately is deallocated, the binding target continues with the same lifetime as the base property.

However, I quickly discovered in my use case that I wanted the ability to keep these views around, and created a type-erased wrapper:

let username: AnyMutableProperty<String> = AnyMutableProperty(user[\.name)

This is really useful, but definitely complicates the object graph. Given the precedent of Property(mutableProperty) not capturing its underlying property, I found that I initially expected an AnyMutableProperty wrapper to behave the same way. But it's not clear to me what the ownership semantics should be, and I'd want to be careful that we don't make it too easy to leak memory. Do you have any thoughts on how this could work?

[andersio]

If it is meant to be an existential wrapper, it should be strong after all as to align with the semantics of generalised existential when it lands in Swift eventually. i.e. let temperature: MutablePropertyProtocol where Value == Decimal should be semantically equivalent to let temperature: AnyMutableProperty<Decimal>.

Property.init isn't meant to be such. Property.init(capturing:) is, and it is meant to be transitional since we assumed that generalised existential was going to land in Swift 3.x. It didn't work out though.

[andersio]

I'd like to add that having views to retain its base object is not quite a rare pattern. The Standard Library slices use the same semantic.

TBH I would be in favour of ditching Property.init(_:) to undo the confusing precedence, if the eventual goal is to replace Property<Value> with a generic type alias to the PropertyProtocol existential.

Alternatively, we could redesign our property types as a class cluster (e.g. the KeyPath cluster). Then we would be able to enjoy the subtyping relationships in today's Swift.

[iby]

Have a related issue going on in #704 where trying to find the best way to consolidate related property bindings, signals and producers. The current mutable property model doesn't look very appealing and I'm trying to understand a couple of things.

As a concept properties sound great. However, in practice ReactiveCocoa doesn't use them at all where they would fit naturally and reverts to binding targets, signals and producers instead, which cause naming inconsistencies (discussed here and here).

I find it rather strange to have a specific tool in the framework, but instead use three other tools to do the job it's designed to do… Do you think having lighter properties which don't require being retained would help breaking this "tradition"?

My second questions is related. Why must the Property be a class? I don't fully understand how signal affect this:

ReactiveSwift/Sources/Property.swift

Lines 14 to 16 in 2a4ffea

	/// Only classes can conform to this protocol, because having a signal
	/// for changes over time implies the origin must have a unique identity.
	public protocol PropertyProtocol: class, BindingSource {

My guess is that this was designed upon retaining the value and not considering that this can be a wrapper or proxy. E.g., it can be composited of an KeyPath existing and signal and be a lightweight struct, like here. Not the universal use case, but the same can be done with @objc dynamic properties, especially when ReactiveCocoa/ReactiveCocoa#3491 gets merged.

[mdiep]

Why must the Property be a class?

It's this bit:

implies the origin must have a unique identity

For something to be observable, it must have reference semantics. (Since values can't change in an observable way.) You can create a thin wrapper as a struct, but it's not a value in any meaningful sense since it'd be backed by an object that was providing identity.

[iby]

Doesn't it depend on how you look at the observable though? It doesn't need to be when it's acting as a "reactive" proxy for AnyObject's property and not storing the actual value. A broad example using KeyPath (the same could be done with getter/setter blocks, not sure why…):

struct PropertyProxy<Base: AnyObject & ReactiveExtensionsProvider, Value>: BindingSource, BindingTargetProvider
{
    init(_ base: Base, _ keyPath: ReferenceWritableKeyPath<Base, Value>, _ signal: Signal<Value, NoError>) {
        self.base = base
        self.keyPath = keyPath
        self.signal = signal
    }

    let base: Base
    let keyPath: ReferenceWritableKeyPath<Base, Value>
    let signal: Signal<Value, NoError>
    var lifetime: Lifetime { return Lifetime.of(self.base) }
    var producer: SignalProducer<Value, NoError> { return self.signal.producer.prefix(value: self.value) }
    var bindingTarget: BindingTarget<Value> { return self.base.reactive[self.keyPath] }

    var value: Value {
        get { return self.base[keyPath: self.keyPath] }
        nonmutating set { self.base[keyPath: self.keyPath] = newValue }
    }
}

Again, this is not to say that this is how things must be, but feels like a great approach to KeyPath properties with minimal initialization and it would be great if PropertyProtocol extensions were compatible with it.

Simply dropping AnyObject conformance wouldn't work, instead, could we have a common base protocol for these stuff? Maybe have protocol Bindable: BindingSource, BindingTargetProvider to provide things like map, etc.?

[mdiep]

Why not make PropertyProxy a class? All of its properties are classes. What do you gain by it being a struct?

[iby]

Okay. My life just divided in before and after I checked this in playground… The struct initialization is outperformed by the same class. Sure this might differ with optimizations, etc.

I was basing my belief that structs will outperform classes. Since the proxying property is temporary disposable type it felt like a better idea sticking with structs. If there's no dramatic difference then there's no point. And it also won't be invoked in 10k-iteration loops either, so… No more questions! ✌️

[RuiAAPeres]

Hello. 👋 Thanks for opening this issue. Due to inactivity, we will soft close the issue. If you feel that it should remain open, please let us know. 😄