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

Vars should be splittable #112

Closed
raquo opened this issue Oct 18, 2023 · 4 comments
Closed

Vars should be splittable #112

raquo opened this issue Oct 18, 2023 · 4 comments
Labels
enhancement New feature or request need to find time https://falseknees.com/297.html
Milestone

Comments

@raquo
Copy link
Owner

raquo commented Oct 18, 2023

Problem

Currently, we can split Signal[List[Model]] into Signal[Model] x N, but we can not split a Var[List[Model]] into DerivedVar[Model] x N. So, for example in Laminar if we're rendering a dynamic list of children from a Var, .split gives us signals for each item, but we get nothing to help with with writing an updated model into the Var.

Context: https://discord.com/channels/1020225759610163220/1020225760075718669/1164014490925404292

Solution

To create a DerivedVar, we need zoomIn / zoomOut functions, and an owner.

  1. We would need the split operator to create an id-specific owner for each model that it adds, and provide that owner to the render callback. That owner would kill its subs when the split operator removes the corresponding id from the list, and when the resulting Signal[List[Output]] is stopped. Note: we must make sure that the owners aren't created until the resulting Signal[List[Output]] is started, otherwise we could get a memory leak like Fix memory management gotcha Laminar#33

  2. We can already derive the zoomIn and zoomOut functions: it's basically "find this item by id", and "update the item by id" in the list. So, providing the key would be enough to derive those behind the scenes, I think. Will need to add a couple methods to the Splittable interface. Note: also consider cases when the key is not unique, or it's something like identity, for example when using operators like splitOne / splitOption

  3. Finally, having all this, we can create a new split operator on Var that would wrap Var.signal.split, and use its owner to create the derived var. Well, perhaps we could make it more efficient later, to avoid discarding the signal, but usage-wise, this will get us what we need.

  4. We can also add a similar .split method on EventBus, in theory. The problem is that we don't have a Var to create a derived var from, so we will need some kind of (EventBus, initialValue) -> DerivedVar conversion? It could be too annoying to implement though, not sure about it.

Advantages

onMountInsert is the currently sanctioned way to get an owner, but in case of splitting it's annoying because we already "know" that the rendered item will be mounted imminently, whereas onMountInsert is designed for cases when we don't know that, or when we can't really convey that information.

Moreover, onMountInsert is error prone because it requires the developer to decide on which element to put it, and if they put it on the wrong one, they can get a memory leak. So, providing an owner via split will help a lot to eliminate such errors, even when we're splitting regular signals, not necessarily vars.

Exposing the owner this way will also let us do all sorts of things that need an owner, not just derived vars but also signal.observe() for example.

TODO: Callback arguments

For regular signal splitting,owner would become the fourth render callback argument (id, initial, signal, owner). That seems like too many already. Perhaps it's time to shove all those arguments into a structure like SplitContext(key, initial, signal, owner), similar to how we have MountContext(thisNode, owner) in onMountInsert.

Two potential downsides:

  • I don't like the generic naming of key – I can't name it id, because it's not necessarily id, it could be something else especially in case of splitOne. Previously, the user would choose the argument name that fits their actual use case, now they'll need to live with a generic name.
  • Ideally I would like to keep the old (owner-less) syntax workable too, because the Laminar video has arguably the best explanation of the split operator, and it uses that syntax.

TODO: Lazy derived vars

I feel like if we have this functionality, users will quickly start wanting a way to zoom into a Var before handing it off to split. For example:

val stateVar = Var(State(foo, bar: Bar(bazs: List[Baz])))
val splittableVar = stateVar.zoom(_.bar.bazs)((state, bazs) => state.copy(bar = state.bar.copy(bazs = bazs))
val splittableVar.split(_.id)((bazId, initialBaz, bazDerivedVar) => ...)

However, right now this would be too annoying to do, because zoom requires an Owner. And that's because it provides a .now() method, and because it's not lazy. So perhaps we can create a lazyZoom method which would simply create a Signal[B] and an Observer[B] and bundle them in a single data structure. I'd call it LazyDerivedVar but it really isn't a Var anymore, so not sure.

If we did have such a LazyDerivedVar, I wonder what kind of API it would be able to expose. Signal, yes, writer, yes, but what about updating? That would require reading the signal's value, and that requires an owner, so I guess not? But, what if we called .observe(owner) on such a "lazy var" to create a non-lazy DerivedVar(parentVar, zoomIn, zoomOut, owner)? Then it would work, I guess, but we would lose shared execution – those zoomIn functions would be called separately for each item.

So, to retain shared execution semantics, I think when splitting a LazyDerivedVar, we would need to create a shared DerivedVar inside the split operator, before deriving it further for each item. However, what owner would it use? We would need a custom owner that matches the lifecycle of the resulting Signal[List[Output]]. And again, be careful to avoid instantiation of that derived Var until the signal gets started.

TODO: Lazy derived event buses

I'm guessing we'll have a similar problem when splitting event buses? So like, LazyDerivedBus? Ugghh. Without a current value, not much there is to be lazy about. Think about type hierarchy and naming before I start on this.

TODO: Other versions?

We use .split for dynamic collections of children, but we also have e.g. splitOne, splitOption, and onMountInsert for other use cases. Do we need to update any other methods with a similar signature, to include the owner? Are there ergonomic wins to be had? Well, splitOne and splitOption are fairly straightforward to update, but I mean, besides that?

Summary

Seems doable, and seems like it will be a nice improvement to ergonomics, but need to find the time to actually implement it.

@raquo raquo added enhancement New feature or request need to find time https://falseknees.com/297.html labels Oct 18, 2023
@filonik-cmu
Copy link

filonik-cmu commented Oct 18, 2023

Just wanted to voice my support. I feel like this functionality would be extremely useful for working nested hierarchies of data and components. Without it, derived vars currently feel a bit like second class citizens compared to signals in Airstream/Laminar.

@phfroidmont
Copy link
Contributor

I use this pattern of splitting vars quite a bit. Since this only comes up in the context of creating child components, creating a few helpers limited to that scope made things much easier (maybe I should make a PR to Laminext @yurique ?).

extension [A](seqVar: Var[Seq[A]])
  def splitRender[Key](key: A => Key)(project: Var[A] => HtmlElement)(using CanEqual[Key, Key])
    : Mod[HtmlElement] =
    onMountInsert(ctx =>
      given Owner = ctx.owner
      children <-- seqVar.signal.split(key)((_, _, signal) =>
        val zoomedVar = Var(signal.observe.now())
        project(zoomedVar)
          .amend(signal --> zoomedVar, zoomedVar --> seqVar.updaterById(key))
      )
    )

extension [A](optionVar: Var[Option[A]])
  def splitRenderOption(project: Var[A] => HtmlElement, ifEmpty: => HtmlElement)
    : Mod[HtmlElement] =
    onMountInsert(ctx =>
      given Owner = ctx.owner
      child <-- optionVar.signal.splitOption(
        (_, signal) =>
          project(
            optionVar.zoom(_.getOrElse(signal.observe.now()))((_, value) => Some(value))
          ),
        ifEmpty = ifEmpty
      )
    )

As for zooming, I actually implemented something very similar to the LazyDerivedVar you describe (that I called ValueController). Internally it was a wrapper for Signal[State], Observer[State => State] and Lens[State, State2] from Monocle which allows support for an updater method. Zooming further is just a matter of chaining the lenses. I can dig up the actual implementation if you think that's any help.

When the zoom method was added to Laminar, I decided to use it instead. Even if the ergonomics of zoom are not ideal it felt better than maintaining my bespoke abstraction.

@rgwilton
Copy link
Contributor

Just to add a +1 that I was finding it hard to get Laminar to work as I expected. I was trying to follow the similar structure of zooming into subparts of a Var and passing those onto rendering components that would zoom further. This worked, but meant that I couldn't really use split.

So, then I tried to use split instead, but found it hard to then zoom into the writer because I wanted the current context to zoom into.

raquo added a commit that referenced this issue Jul 11, 2024
@raquo raquo added this to the 17.1.0 milestone Jul 11, 2024
@raquo
Copy link
Owner Author

raquo commented Jul 11, 2024

You can now preview this feature in Airstream v17.1.0-M1. It can be used with Laminar 17.0.0. The new method names on Var are split and splitMutate – the latter for mutating mutable collections in-place in a Var.

@raquo raquo closed this as completed Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request need to find time https://falseknees.com/297.html
Projects
None yet
Development

No branches or pull requests

4 participants