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

stream receiver for Observable[VDomModifier] #199

Merged
merged 1 commit into from
Jul 6, 2018

Conversation

cornerman
Copy link
Member

@cornerman cornerman commented May 21, 2018

Added a Modifier for supporting an Observable[VDomModifier]: ModifierStreamReceiver.

This is support stream arbitrary modfiers and not just nodes and single attributes. The ChildStreamReceiver and ChildrenStreamReceiver are replaced with the ModifierStreamReceiver. Furthermore, the StaticNodeRender was removed. The AttributeStreamReceiver still makes sense, because it can be made unique per attribute title.

Questions:

  • How about streaming a Key? This does not really make sense with snabbdom or does it?

fixes #198 #187

@cornerman cornerman changed the title [WIP] stream receiver for Observable[Property] stream receiver for Observable[VDomModifier] May 21, 2018
@cornerman cornerman force-pushed the modifier-stream branch 3 times, most recently from 5e4469d to 93de1c6 Compare May 22, 2018 19:23
@mariusmuja
Copy link
Collaborator

I didn't have time to look over this yet, but after a quick glance I'm wondering if we want the library to handle nested Observables.

I think it might be better to not do so and require the client code to flatten the observables before passing them to outwatch. For example, this PR uses switchMap to flatten nested observables, but why not use mergeMap (or concatMap). Which is the correct behaviour? Probably depends on the application.

@cornerman
Copy link
Member Author

I didn't have time to look over this yet, but after a quick glance I'm wondering if we want the library to handle nested Observables.

For the usage, I find this really convenient: I can just combine VDomModifiers whether they are streamed or not. Additionally I can have a Seq of modifiers streamed into a node which leads to less updates than having multiple attribute streams. Currently, I have to introduce an intermediate dom element (StaticVNode) to wrap multiple modifiers, which is not always what I want.

For example, this PR uses switchMap to flatten nested observables, but why not use mergeMap (or concatMap). Which is the correct behaviour? Probably depends on the application.

I think switchMap is really intuitive in the context of VDomModifiers. Think of this example:

val modifierHandler = Handler.create[String]()
val modifier: VDomModifier = div(cls <-- modifierHandler)
val otherModifier: VDomModifier = ???

val booleanHandler = Handler.create[Boolean]()
booleanHandler.map {  enabled =>
  div(
    "status: ", if (enabled) modifier else otherModifier
  )
}

Then (if we used e.g. mergeMap):

booleanHandler.unsafeOnNext(true) // this will render modifier
modifierHandler.unsafeOnNext("hans") // will render modifier with class=hans
booleanHandler.unsafeOnNext(false) // will render otherModifier
modifierHandler.unsafeOnNext("pete") // will trigger the update of the old observable from modifier and render class=pete.

Rendering the class from modifier within otherModifier is definitely wrong, and doing this multiple times would lead to multiple updates.

I think switchMap is essentially the same behaviour you got when we were streaming StaticVNode which could itself have streamed children - they are also replaced. If you want more control, you can still managed the observables yourself.

Do you have an example where switchMap leads to an unintuitive behaviour?

@mariusmuja
Copy link
Collaborator

For the usage, I find this really convenient: I can just combine VDomModifiers whether they are streamed or not. Additionally I can have a Seq of modifiers streamed into a node which leads to less updates than having multiple attribute streams.

I like the idea of supporting Seq of modifiers streamed into a node and I see your point on switchMap having the same behaviour as static nodes with children observables being replaced.

What I'm not so convinced about yet is the usefulness of having observables of VDomModifiers streamed into a node.

@cornerman
Copy link
Member Author

cornerman commented May 23, 2018

What I'm not so convinced about yet is the usefulness of having observables of VDomModifiers streamed into a node.

I like that I can treat all modifiers equally, often I need to make one part reactive, and now this leads to nesting of elements.

Another advantage is that I can better separate reactive components and behaviours. Say, I am having a simple component with a handler:

def component = Handler.create[String].flatMap { handler =>
   span("Like: ", handler, onClick("yes!") --> handler)
}

// usage
div(component)

This is nice and reuseable, but what if I want control over the outer element? What I want is to render this behaviour into an element:

def component = Handler.create[String].flatMap { handler =>
   VDomModifier(handler, onClick("yes!") --> handler)
}

// usage
div("Love: ", component)

Or with streaming a VDomModifier:

def component = Handler.create[VDomModifier]("?").flatMap { handler =>
   VDomModifier(handler, onClick(VDomModifier("yes!", color := "green")) --> handler)
}

It could even include more observables and would still work. Of course, one should watch out what you are combining but that is already the case now.

@mariusmuja
Copy link
Collaborator

Yeah, that's nice! But this example doesn't use higher-order observables. Did you encounter instances where using higher order observables was needed?

I agree that it's nice to treat all VDomModifies equally, but maybe a solution would be to adjust the Modifier hierarchy? I'm a bit reluctant to introducing this complexity without a well defined use case.

@cornerman
Copy link
Member Author

but maybe a solution would be to adjust the Modifier hierarchy?

Indeed, we could adjust the Modifier hierarchy, maybe with a StaticModifier. But I would like to see whether we can come up with something that works for all modifiers :)

I extended the component in the example to have a handler of VDomModifier. Does that make sense?

@mariusmuja
Copy link
Collaborator

I extended the component in the example to have a handler of VDomModifier. Does that make sense?

Yes, but that still doesn't use higher order observables.

@cornerman
Copy link
Member Author

Yes, but that still doesn't use higher order observables.

If I wrap the usage in an observable, I would have nested streamed modifiers. I thought that was a higher order observable - isn't it?

@mariusmuja
Copy link
Collaborator

If I wrap the usage in an observable, I would have nested streamed modifiers. I thought that was a higher order observable - isn't it?

Yes, it's obviously possible to construct an example of usage. What I'm wondering about is a practical use case for this, when having nested observables of VDomModifiers part of a node is the most obvious or the easiest / most intuitive way of solving the problem at hand.

@fdietze
Copy link
Member

fdietze commented Jul 6, 2018

What I like about outwatch, is that you can throw almost everything at it, and it will render it. This PR makes this even more flexible. Even if it's hard to find an artificial use-case of nested observables right now, we do not know the architectures other people have.

this replaces Child-/ChildrenStreamReceiver with ModifierStreamReceiver
@cornerman cornerman merged commit 2c61e04 into outwatch:master Jul 6, 2018
@cornerman cornerman deleted the modifier-stream branch July 6, 2018 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StreamReceiver for VDomModifier
4 participants