Add @dynamicMemberLookup to ObservableType and SharedSequenceConvertiblebType

[yuriferretti]

Hi guys,

One of the cool features that Combine brought us is to enable keypath based @dynamicMemberLookup on some types of sequences what makes simple attribute based transformation much easier like the following:

// conventional mapping
let row = tableView.rx.itemSelected.map { $0.row }

// using dynamicMemberLookup
let row = tableView.rx.itemSelected.row

So I added the @dynamicMemberLookup to ObservableType and SharedSequenceConvertiblebleType to allow RxSwift Sequences the same neat mapping.

Please let me now your thoughts about it.

[kzaher]

Please remove these changes.

[kzaher]

It seems that we don't need these either.

[kzaher]

Please remove all unnecessary changes for this PR.

[kzaher]

I haven't worked with Swift for a while now so I've missed some developments.

How does this relate to the Observable extensions. If one has Observable<Observable<Int>> and I call subscribe, how will this be resolved? Or if there is Observable<Int?> and map is performed.

How much compiler overhead does this add? Do we have some estimates? Can somebody run this changes on a larger codebase?

I kind of like the changes on the one hand, but on the other it is kind of confusing.

@freak4pc Thoughts?

[freak4pc]

Edited: This is a good change syntactically speaking, makes writing a tad more natural.

I don't have any data regarding performance / etc because this is still a relatively new feature, and we also need to see if this creates any effect on existing consumers (probably not).

Could you run the benchmark targets before and after this to provide some idea of the effect this makes? @yuriferretti

[freak4pc]

@yuriferretti Can you also share where in Combine is this used? I couldn't find this on top of Publisher types, for example.

[gringoireDM]

How does this relate to the Observable extensions. If one has Observable<Observable> and I call subscribe, how will this be resolved? Or if there is Observable<Int?> and map is performed.

@kzaher @dynamicMemberLookup with this implementation requires a Keypath having the generic type as root, meaning that it works just with vars, not methods. You can define Observable<Observable>, but it won't be useful.

This is a good change syntactically speaking, makes writing a tad more natural

I'm not sure I share your point of view here.

struct Person { 
    var name: String 
    var lastName
}

var obs: Observable<Person> ...
obs.name // valid and the type will be `Observable<String>`
//equivalent to
obs.map { $0.name }

Personally I feel this change not a good fit for dynamicMemberLookup. I might be wrong, but the idea to shortcut a map feels like driving a Ferrari in a courtyard.

I would see @dynamicMemberLookup as a good fit for a BehaviorSubject

let person = BehaviorSubject<Person>(value: ...)
person.name // String (the name of the current value)
// equivalent to
(try! person.value()).name

soon we will have Keypaths as function that will already improve map ergonomics:

person.map(\.name) //Observable<String>

[yuriferretti]

@gringoireDM

Personally I feel this change not a good fit for dynamicMemberLookup. I might be wrong, but the idea to shortcut a map feels like driving a Ferrari in a courtyard.

Could you elaborate more on that, what do you think would be a good fit?
All implementations of @dynamicMemberLookup I've seen so far are about making easier to access inner attributes of types. IMHO this is by far the most common use case for map.

I would see @dynamicMemberLookup as a good fit for a BehaviorSubject

let person = BehaviorSubject<Person>(value: ...)
person.name // String (the name of the current value)
// equivalent to
(try! person.value()).name

And how would to deal with it throwing an error in case the subject errors out? If you had a good workaround I can add it to the implementation

[yuriferretti]

@yuriferretti Can you also share where in Combine is this used? I couldn't find this on top of Publisher types, for example.

In this wwdc video Doug Gregor shows how the used @propertyWrappers + @dynamicMemberLookup to make bindings mapping easier.

[yuriferretti]

@kzaher

How does this relate to the Observable extensions. If one has Observable<Observable<Int>> and I call subscribe, how will this be resolved?

It doesn't conflict with observable extensions in any way, it can also provide access to them if they are var. The nested Observable case would allow access to Observable public attributes, but I think that are none.

Or if there is Observable<Int?> and map is performed.

After your question I started making some tests and I think the compiler gets lost sometimes when a long keyPath chain has an inner member that is optional. I will try to reproduce it more reliably and try some workarounds and If I didn't succeed I will file a bug to Swift and put PR on hold.

[yuriferretti]

@freak4pc

Edited: This is a good change syntactically speaking, makes writing a tad more natural.

I don't have any data regarding performance / etc because this is still a relatively new feature, and we also need to see if this creates any effect on existing consumers (probably not).

Could you run the benchmark targets before and after this to provide some idea of the effect this makes? @yuriferretti

Sure, I will send it to you when I reach my machine later today!

[gringoireDM]

@yuriferretti

Could you elaborate more on that, what do you think would be a good fit?
All implementations of @dynamicMemberLookup I've seen so far are about making easier to access inner attributes of types. IMHO this is by far the most common use case for map.

The use case for @dynamicMemberLookup is when you do have a value that your @dynamicMemberLookup class/struct is wrapping. In an Observable you actually don't.

By seeing the code person.name I would never think that its result is to apply a map to the stream to name. I think it's counter intuitive and at some extent confusing.

The way this is used in swiftUI (with @State for example) is to return a Binding of a property from an object that is already existing representing a state.

All the examples I've seen so far have the same behavior: existing wrapped value -> do something with their properties.

With Observable we don't have a wrapped value.

And how would to deal with it throwing an error in case the subject errors out? If you had a good workaround I can add it to the implementation

You could return an optional, then do the same for BehaviorRelay that can't error out (in which you would return a non optional value)

[M0rtyMerr]

Fully agree with @gringoireDM about counter intuitive side of this proposal

_{Sent with GitHawk}

[freak4pc]

Yup, my instinct is that - it's fun syntactically and saves some crud.

e.g. observable.map { $0.a.b.c } -> observable.a.b.c

BUT - I think it's too confusing as they aren't really properties of the Observable but of it's underlying Element type.

[gringoireDM]

Soon observable.map(\.a.b.c) will be available for free, and if you like to use a custom operator you can already have it today

prefix operator ^
prefix func ^<Root, Value>(keypath: KeyPath<Root, Value>) -> (Root) -> Value {
    return { $0[keyPath: keypath] }
}

With this code you can do observable.map(^\.a.b.c)

[yuriferretti]

@gringoireDM

The way this is used in swiftUI (with @State for example) is to return a Binding of a property from an object that is already existing representing a state.

All the examples I've seen so far have the same behavior: existing wrapped value -> do something with their properties.

It think you meant @Binding because state doesn't allow @dynamicMemberLookup. Please take a look at @Binding docs and you will see that @Binding does the same as my proposal does projecting an inner attribute from binding element as another binding, that's where my inspiration came from.

You could return an optional, then do the same for BehaviorRelay that can't error out (in which you would return a non optional value)

IMHO would be strange and counterintuitive to suddenly expose all attributes as optional values.

[yuriferretti]

@kzaher @freak4pc @gringoireDM
Thanks for your feedback and given your reactions I think it's worth maturing a little more the ideia given the new @dynamicMember usages that will start spawning everywhere and will make this more common sense.

[gringoireDM]

@yuriferretti yes, I meant @Binding.

@binding does the same as my proposal does projecting an inner attribute from binding element as another binding, that was my came from inspiration.

It's not the same. Binding is projecting an inner attribute that is already present, while you are transforming the stream. It's deeply different. Binding projects a property of an object that is currently wrapping. Observable is not wrapping an object.

IMHO would be strange and counterintuitive to suddenly expose all attributes as optional values.

Not really. the value() can fail, therefore it is not really surprising that a property fetched from the BehaviorSubject is optional.

Besides BehaviorRelay does not suffer of this issue. That would be a perfect example where @dynamicMemberLookup would be a good choice.

[yuriferretti]

@gringoireDM

It's not the same. Binding is projecting an inner attribute that is already present, while you are transforming the stream. It's deeply different. Binding projects a property of an object that is currently wrapping. Observable is not wrapping an object.

I'm sorry but I feel that you are trying to focus in the smaller picture in order to justify your vision and before continuing, please, understand that I'm not trying to impose nothing nor telling what's right or wrong here. I'm just trying to show where my inspiration came from.

I think both of us will agree that the most valuable aspect of Observable<Element> is the Element it emits the same way Value is for Binding<Value>. Said that, what binding did was to expose Value's attributes as bindings because it's a pretty common use case to bind them in other view components.
Now I think you can have a more clear perception about what I'm trying to achieve here.

Another way to see how both implementations have the same pouporse is:

struct Foo { 
   let bar: Bar
}

let foo: Observable<Foo> = ...

How would you create an Observable<Bar> from an Observable<Foo>?

[gringoireDM]

I'm sorry but I feel that you are trying to focus in the smaller picture in order to justify your vision and before continuing, please, understand that I'm not trying to impose nothing nor telling what's right or wrong here. I'm just trying to show where my inspiration came from.

I'm sorry you feel this way, but that's not the case. You can't have @dynamicMemberLookup if you don't have a member to lookup.

I think both of us will agree that the most valuable aspect of Observable is the Element it emits

No, we don't. The most valuable aspect of Observable<Element> is the stream of Elements. If I had only one Element, I wouldn't make it observable in the first place.

the same way Value is for Binding.

Binding has a reference to an existing Value. Observable doesn't. Observable might have a stream of Element, or it might have an error, or It might complete without even emitting an Element.

Said that, what binding did was to expose Value's attributes as bindings because it's a pretty common use case to bind them in other view components.

Correct, but as new Binding, not as a map of the trigger that ticks the binding to emit a change.

struct Foo {
    var bar: Bar
}

struct Baz {
    @Binding var fooInstance: Foo

    func myFunc() {
        let barBinding = $fooInstance.bar
        fooInstance.bar = newBar // <- this will cause barBinding to emit a new event
    }
}

How would you create an Observable from an Observable?

But you are not creating an Observable<Bar> you are mapping to Bar a stream of Foo. Which means that it will emit a new value just when a new Foo is emitted. If Foo is the same, and the bar instance of Foo changes, map won't emit anything new, which is not what happens with @Binding Binding wraps getter and setter of bar in a new Binding. You see how different the idea is?

Surely the type check will give you the right to think that you've made an observable of Bar, but truly you are not observing Bar changes on the current Foo.

[yuriferretti]

No, we don't. The most valuable aspect of Observable<Element> is the stream of Elements. If I had only one Element, I wouldn't make it observable in the first place.

Please point me where I wrote that I'm talking about an observable that emmits just a single time.

Binding has a reference to an existing Value. Observable doesn't. Observable might have a stream of Element, or it might have an error, or It might complete without even emitting an Element.

Sure and map isn't called for errors nor completion the same way this PR shortcut wouldn't. So I don't think this arguent is a valid comparison given both would react the same way.

But you are not creating an Observable<Bar> you are mapping to Bar a stream of Foo. Which means that it will emit a new value just when a new Foo is emitted. If Foo is the same, and the bar instance of Foo changes, map won't emit anything new, which is not what happens with @Binding Binding wraps getter and setter of bar in a new Binding. You see how different the idea is?

Surely the type check will give you the right to think that you've made an observable of Bar, but truly you are not observing Bar changes on the current Foo.

You are totaly mixing up things here. I said before that both Observable<Element> and Binding<Value> most important aspects are Element and Value, not that Binding works the same way as Observable, both types have very different usages.
Also I've never expected/said that changing an instance that an observable emmits would force it to emmit the modified instance, I know very well how an observable works.

I will simplify the things here to avoid more misinterpretations and please be aware that I'm not talking about the @Binding wrapper, I'm talking about @dynamicMemberLookup:

let foo: Binding<Foo> = .constant(Foo()) // no wrapper pure types
let dynamicBinding: Binding<Bar> = foo.bar //dynamicMember returns Binding<Bar>
let initBinding: Binding<Bar> = Binding(base: foo) // it would also work as the above but is more verbose

let obsFoo : Observable<Foo> = .just(Foo())
let dynamicBar: Observable<Bar> = obsFoo.bar //dynamicMember returns Observable<Bar>
let mapBar: Observable<Bar> = obsFoo.map { $0.bar } // same as above but more verbose

I'm not talking about when they will emit, update, complete or whatever. I'm talking about enabling a shorcut to make those kind of transformations more brief and noiseless.

It's not about doing the same job, it's about where the idea came from!
If you dont agree or don't like it no problem man! 🍻

[yuriferretti]

It seems that swift's typechecker has some optional chaining KeyPath related bugs that I'm stumbling upon right now as stated here.
I'm closing this PR and we can reopen it when they solve it