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

Support nesting of dynamic inserters #157

Open
raquo opened this issue May 6, 2024 · 1 comment
Open

Support nesting of dynamic inserters #157

raquo opened this issue May 6, 2024 · 1 comment

Comments

@raquo
Copy link
Owner

raquo commented May 6, 2024

Background

To render dynamic data in Laminar, you use dynamic inserters like children <-- childrenObservable. Currently (v17), the children <-- receiver accepts Source[Collection[Component]], where:

  • Source is basically Observable
  • Collection is List, js.Array, or similar
  • Component is something that can be converted to a Laminar node (via RenderableNode)

Problem

The current functionality works fine to render many types that look like a sequence of elements, it even supports mutable sequences, but the RenderableNode interface places a significant restriction on what kind of components are considered element-like: any supported Component must have a corresponding Laminar element, and that element must be always available, and remain referentially the exact same element, i.e. you can't decide that the Component shall render an entirely different element at any point in its lifecycle. Such changing components are expected to expose Observable[Element] rather than a Component in Laminar – which works fine, but does not fit within the RenderableNode's idea of a Component.

In practice, this means that you can't put things like child <-- ... or onMountInsert(...) as one of the "components" in the observable in children <-- observable.

For example, I want to be able to do this (using the new children(...) <- boolObservable syntax of v17):

div(
  children(
    div("hello"),
    child(div("world")) <-- boolVar1,
    onMountInsert { ctx =>
      val nestedVar = someSignal.observe(ctx.context)
      div("blah", text <-- nestedVar)
    }
  ) <-- boolVar2
)

Or this:

div(
  children <-- observable.split(_.id) { (id, initial, childSignal) => 
    onMountInsert { ctx =>
      val childVar = parentVar.zoom(...)(ctx.owner)
      div(text <-- childVar)
    }
  }
)

Currently neither of this is not possible, because child(div("world")) <-- boolVar1 and onMountInsert { ... } do not fit the definition of Component in RenderableNode. This is because onMountInsert { ... } can not implement RenderableNode's asNode() method: it requires a MountContext – evidence of the parent element having been mounted – to create its element / node. Moreover, it may create a new element (or multiple) on every mount, whereas the RenderableNode contract expects the component to retain the same element at all times, and only one element.

Ultimately, RenderableNode's design is driven by the needs of the children <-- observable diffing algorithm. The meat of it is located in the updateChildren method of ChildrenInserter. That algorithm needs two main pieces of data:

    prevChildren: JsMap[dom.Node, ChildNode.Base],
    nextChildren: laminar.Seq[ChildNode.Base],

As you see, it has no concept of "Component" – we convert those to laminar elements (ChildNode.Base) before handing them off to this algorithm. Historically, that's because the concepts of RenderableNode and Component came into existence much later than this algorithm.

And so, this algorithm in its current state can not deal with components that can not be converted to a single static element. The question is whether we can generalize it without making it too complicated.

Motivation

On the surface, the issue can be avoided by simply wrapping your dynamic inserters in a static element like div, e.g.:

div(
 children <-- observable.split(_.id) { (id, initial, childSignal) => 
   div( // static wrap over dynamic contents
     onMountInsert { ctx =>
       val childVar = parentVar.zoom(...)(ctx.owner)
       text <-- childVar
     }
   )
 }
)

In simple cases like the above snippet, you may not even need to pay the performance overhead of the extra div, and even in more complex cases, that would often be inconsequential.

However, I believe the advantages in flexibility and reduced cognitive overhead could well be worth it. Especially now that the new-style children(...) <-- boolSignal API pretty much invites users to nest inserters inside children(...) as shown in the first snippet in this issue.

Potential solution: keep track of Inserter-s?

Laminar Inserter-s like onMountInsert { ... }, child <-- ..., and children <-- ... keep track of their own contents (DOM nodes). They also know where their content ends in the DOM, for example child <-- knows that it can only render at most one content node, and children <-- remembers how many nodes it has rendered last time (and in fact remembers the nodes themselves).

So, if children <-- observable accepted such inserters as part of the list in observable, the diffing algorithm could potentially coordinate with these inserters, and skip over the DOM nodes managed by them. For that, the algorithm would need to keep track of inserters instead of tracking the DOM nodes. ChildNode would probably need to extend Inserter, and we would need more subtypes of Inserter to apply correct behaviour in different cases.

This sounds fairly straightforward in principle, but the implementation would need to be very careful, especially to ensure that we correctly count the nodes managed by inserters even in the face of various external disturbances such as external code removing elements managed by nested inserters. Laminar has some resilience against such disturbances built-in, but I'm not sure if it will stand up to such nesting. We would also need to carefully watch performance.

Composability and Resource type equivalence

Supporting the nesting of inserters could be a good alternative solution to #148, and more generally, to the problem of lifecycle management discussed in the comments of #130. Would our reworked Inserter be able to stand in for a proper Resource type? I'm not yet sure. At least, it should solve the practical problems that users have been voicing, in a manner that is architecturally compatible with existing Laminar applications, and does not complicate the apparent API. But, I think the increased prevalence of Inserter type may induce demand for more functionality on that type, for example, if users start writing components that have onMountInsert as their top "element", they would return Inserter-s instead of elements, and so users would probably want some kind of amend method similar to what elements have. But... that wouldn't be possible because we don't know what kinds of nodes the inserter inserts. Those may not even be elements, they could be text nodes, for which amend functionality is not defined. Should Inserter be more typeful then? Inserter[N <: ChildNode.Base]? What about inserters that can potentially insert multiple nodes? Do they get a special type? Would their type param be a useless ChildNode.Base if they contain even one text node? What about sentinel comment nodes, ignore them I guess?

I don't know the answers to these questions yet. I think that over-relying on Inserter may be detreimental because it's too flexible – so while it's easy to create and hand off to Laminar without boilerplate, it's too opaque to be composable, so e.g. you wouldn't be able to call amend on it. That means that this type can't / shouldn't be the typical output of your reusable components.

I assume that this problem is mostly about the onMountInsert use case, as this is primarily the case that suffers from opaqueness. If your Inserter is e.g. child <-- observable, well you can just return the observable instead, and the consumer would be able to map it or something before calling the child <-- on it, if they wanted to. But a similar decomposition is not possible with onMountInsert.

So, while the proposed solution would still be useful in its own right, at least to render child <-- and children <-- in a nested way, I'm not sure that it would be a good solution for onMountInsert. I think that one needs more thought.

@HollandDM
Copy link
Contributor

I would love this feature so much. My Airstream/Laminar codes right now uses a lot of split's, and creating dummy element for them, although worked, feel very cumbersome to me.

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

No branches or pull requests

2 participants