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

RFC: signal.peekNow() #130

Open
raquo opened this issue Dec 30, 2022 · 6 comments
Open

RFC: signal.peekNow() #130

raquo opened this issue Dec 30, 2022 · 6 comments

Comments

@raquo
Copy link
Owner

raquo commented Dec 30, 2022

75ucgr

Problem

Airstream's design is focused on safety, and among other safety mechanisms, we want to prevent users from accessing the current value of signals that are currently stopped, because in those cases their value could be stale or undefined.

As signals can start and stop at arbitrary times as determined by external factors (via the ownership system), this constraint can't be expressed in types.

So, by necessity, the only way the user could prove to Airstream that the signal is started, is for the user to add an observer to it – if the signal wasn't started before, it is now. To add an observer, the user needs access to an owner, which Laminar can provide via any onMount* method (for signals with a global lifespan, users can also use unsafeWindowOwner, but we're talking about signals that belong to elements):

def childComponent(numSignal: Signal[Int]): Div =
  div(
    onMountCallback { ctx =>
      val observedNumSignal = numSignal.observe(ctx.owner)
      val childVar = Var(initial = observedNumSignal.now())
      println(s"current number: ${childVar.now()}")
    },
    "Hello world",
    child.text <-- childVar // does not compile – no access to childVar here
  )

def parentComponent(): Div = {
  val numVar = Var(0)
  div(
    "Child: ",
    component(numVar.signal.map(_ + 1))
  )
}

val app = parentComponent()

The snippet above prints the signal's current value every time the div is mounted into the DOM. As you see, simply accessing .now() on a signal requires two lines of boilerplate, and picking annoying names like observed<existingSignalName>. Another annoyance is that we can only do this inside onMountCallback, and that means that the scope of childVar is limited to that callback, so it's even more annoying to use that Var to render children or bind events.

In this simple example, you could replace onMountCallback with onMountInsert and return child.text <-- childVar from there, but this does not work if your code gets more complex (e.g. if you also need to send some onClick events into childVar) – I mean you can still do that, but the boilerplate starts exploding.

Currently, to solve that, I often let the parent component pass the owner to the child:

def childComponent(numSignal: Signal[Int], owner: Owner): Div =
  val observedNumSignal = numSignal.observe(ctx.owner)
  val childVar = Var(initial = observedNumSignal.now())
  println(s"current number: ${childVar.now()}")
  div(
    "Hello world",
    child.text <-- childVar
  )

def parentComponent(): Div = {
  val numVar = Var(0)
  div(
    "Child: ",
    onMountInsert { ctx =>
      component(numVar.signal.map(_ + 1), ctx.owner)
    }
  )
}

val app = parentComponent()

This gives us access to childVar in the entirety of the child component, so we can pipe it to child.text now. However, the boilerplate remains. We can shave off some more of it by creating observed signals on the parent side:

def childComponent(numSignal: StrictSignal[Int]): Div =
  val childVar = Var(initial = numSignal.now())
  println(s"current number: ${childVar.now()}")
  div(
    "Hello world",
    child.text <-- childVar
  )

def parentComponent(): Div = {
  val numVar = Var(0)
  div(
    "Child: ",
    onMountInsert { ctx =>
      component(numVar.signal.map(_ + 1).observe(ctx.owner))
    }
  )
}

val app = parentComponent()

So far, this has been mostly satisfactory to me personally, although it is not perfect – using onMountInsert in the parent means that the child element is re-created every time the parent is unmounted and re-mounted, and also you can't get a reference to the child element outside of onMountInsert – this is usually not needed, but if your child component returns not a simple element, but an instance of a class that exposes special properties / streams / etc., then it will be hard for you to access those properties outside of onMountInsert.

To reduce the number of onMountInsert-s, I sometimes have several layers of ancestor components pass down the same owner to their children – however that is getting into sketchy territory in regards to lifecycle & memory management. That ancestral owner will only kill subscriptions when the ancestor it belongs to (some "root"-like element that is high in the hierarchy) is unmounted, and so it is only safe to use in children that are rendered statically, not dynamically. If any children inside child <-- / children <-- use that ancestral owner, the child's resources bound to that owner won't be released until the whole ancestor has been unmounted, even if the child in question has been unmounted long before that.

Desired feel

In my ideal world, I should be able to do this without worry:

def childComponent(numSignal: Signal[Int]): Div =
  val childVar = Var(initial = numSignal.peekNow()) // peekNow() instead of now
  println(s"current number: ${childVar.now()}")
  div(
    "Hello world",
    child.text <-- childVar
  )

def parentComponent(): Div = {
  val numVar = Var(0)
  div(
    "Child: ",
    component(numVar.signal.map(_ + 1))
  )
}

val app = parentComponent()

and everything should work just fine. The new peekNow() method would simply look at numSignal's private current value, and return it, except it should throw if numSignal is currently stopped – this isn't a replacement for managing observable lifecycles after all, it's simply a way to look at some value that you know is already there.

Case in point – in this particular code snippet, peekNow() will actually fail because while numVar.signal being a Var's signal is always-started, the derived numVar.signal.map(_ + 1) is not started when we call peekNow() on it. In fact, it is never started in our code at all. So, peekNow() will throw. This could have been avoided if parentComponent was also using numVar.signal.map(_ + 1) in its own code, but that is not the case.

I am fine paying the cost of exceptions in principle, but I'm left wondering if there will be enough use cases for such a peekNow() method. Maybe some day I'll implement it and use it my personal projects to see how it feels.

Request for comments

I would like to understand how Laminar users deal with this problem – do you feel the need to use .observe() often, and is it annoying? Why do you usually use .observe() – to initialize a Var with some other signal's current value (e.g. in web forms), or for something else?

@sherpal
Copy link

sherpal commented Dec 30, 2022

I have personally never done that kind of "trick", and certainly I don't remember that I used observe in the past.

For what it's worth, if I had to implement the particular use case you mention myself, I would do something like

def childComponent(initialValue: Int): Div =
  val childVar = Var(initial = initialValue)
  println(s"current number: ${childVar.now()}")
  div(
    "Hello world",
    child.text <-- childVar
  )

def parentComponent(): Div = {
  val numVar = Var(0)
  div(
    "Child: ",
   child <-- numVar.signal.map(_ + 1).take(1).map(childComponent)
  )
}

val app = parentComponent()

where that take(1) would be an extension method that I would need to find how to implement...

@raquo
Copy link
Owner Author

raquo commented Dec 30, 2022

Some notes / ideas, mostly for myself

We were talking with @armanbilge about this today, from the perspective of lifecycle management in observable / streaming systems.

He takes different tradeoffs in Calico, and it was interesting to learn those patterns and think about their possible applications in Laminar.

Very rudimentarily, one pattern we could employ in current Laminar is something like this:

def component(signal1: Signal[String], signal2: Signal[B]) =
  div.observed(signal1, signal2) { (s1, s2, owner, thisRef) =>
    // Here s1 and s2 are strict, and `owner` is available to make derivatives like s1.map(...) also strict
    List(
      "Hello " + s1.now().toUpperCsae,
      child <-- s2.map(...)
    )
  }

def observed(signal1: Signal[A], signal2: Signal[B])(render: (StrictSignal[A], StrictSignal[B], Owner, Div) => List[Modifier]) = {
  onMountInsert { ctx =>
    thisNode => div(inContext { thisNode => render(signal1, signal2, ctx.owner, thisNode) } )
  }
} 

div(
  "Parent here",
  component(signal1, signal2)
)

As presented, this is essentially an abstraction over my onMountInsert approach above. Laminar and Airstream lack the necessary properties to make this pattern more useful.

More generally, we talked about Airstream's observables being lazy (as a downside – managing their lifetimes, even though it's done automatically, can still be annoying at times as evidences by this very issue), and Calico's concept of elements-as-resources that allows their observables to be strict and yet not leak memory.

In Laminar you can mount and unmount a component, and this will stop and then restart the subscriptions, while retaining the same DOM element, and reviving the subscriptions in a reasonable and safe. It took me a lot of work to get to a point where this is working relatively smooth, but I think the upcoming version of Laminar nails down this pattern. Yet this pattern isn't really an end-goal, it's more of a necessity due to our observables being lazy. And the observables are lazy because otherwise they would leak in certain cases (see the linked discord thread, or #33), at least as long as we have direct element creation using div().

I think we should investigate borrowing some wisdom from Calico's approach. That would be an immense undertaking, and would need a lot of thought put into it to even just fully understand the tradeoffs, let alone iron out all the bugs, but it's good to talk it out even if I'm not going to do it anytime soon, or at all.

Consider this:

  • If Laminar elements were lazy and had no "initial value", signals inside of those elements could evaluate their initial value strictly. We can't have both elements and signals strictly evaluated (because Fix memory management gotcha #33), so we're switching one from the other.
  • As Arman said, dealing with lifecycles of observables (and resources in general) is a bigger problem than dealing with indirect DOM element references, so this might be a good tradeoff.

Then, we need:

class Element(tag: Tag):

  • Same as today, 1-to-1 link to real DOM element
  • Has a single owner (provided by the parent) (instead of a dynamicOwner that creates multiple owner-s)
  • Subscriptions in this element use this element's owner
  • Thus, subscriptions are initialized immediately upon element creation, NOT when it's mounted
  • And the subscriptions are killed when the owner decides, not when this element is mounted.

class Resource[A](Owner => A):

  • Call .build() to create an Owner and produce an A from it.
  • For example, you can build an element this way.

Tag:

  • div(mods) creates a Resource[Element](owner => div.create(owner, mods))
  • div.withOwner(owner => mods) is the same but gives access to the owner if needed
  • // div.fromOwner(owner)(mods) creates an Element(!) by creating a Resource and calling build(owner) on it

So essentially:

val user: Signal[User] = ???
val el: Resource[Element] = div.withOwner( owner =>
  // if we use withOwner, we can use class-based components like TextInput and their props as usual
  val nameInputVar = Var("world")
  val formattedName = nameInputVar.signal.map(_.toUppercase).own(owner)
  val textInput = TextInput(owner, formattedName)
  textInput.inputNode.amend(
    onInput.mapToValue --> nameInputVar,
    value <-- nameInputVar
  )
  dom.console.log("Initial name: " + formattedName.now())
  List(
    "Hello, ",
    b(child.text <-- formattedName),
    textInput.node,
    br(),
    // onMountCallback equivalent (onMountBind and onMountInsert are not needed - just put that stuff here)
    // onMountUnmountCallback is also possible with... "bimap" or whatever FP lords would  call it.
    div(cls := "widget").map((thisRef, owner) => setupThirdPartyComponent(thisRef.ref))
    br(),
    // child <-- Signal[Resource] provides a new Owner to the new child every time it emits
    // that owner will die on the next event, or when the parent element's owner dies.
    child <-- user.map(renderUserResource(_, formattedName)),
    // If we wanted to render individual child elements, we would need to provide some
    // owner, but there isn't any. the parent's owner would be unsuitable here, but that's the
    // only owner available to the user. Does this mean child <-- should not accept Signal[Element]?
    child <-- user.map(renderUserElement(_, formattedName), owner = ???)
    // this one works just like our regular split, creating an element and an owner context
    // allows for efficient rendering of user info as usual.
    // do we need a similar implementation that requires a resource callback instead of element callback?
    child <-- user.splitOne(_.id)(renderUserSplit(formattedName))
  )
)

class TextInput(owner: Owner, formattedName: StrictSignal[String]) {
  
  val inputNode: Element = input(cls := "input").build(owner)
  
  val node: Element = div(
    "Initial name: " + formattedName.now(),
    inputEl
  ).build(owner)
}

def renderUserResource(user: User, formattedName: StrictSignal[String]): Resource[Element] = {
  // ??? this userFormattedName still can't be strict, right?
  // this can't be owned by the parent owner, because then we're leaking memory just like current Airstream would
  // so we need some special syntax for this... Calico has flatMap but I'm looking for something lighter
  val userFormattedName = formattedName.map(user.formatAgain)
  div(
    user.name + " (",
    "Initial: " + userFormattedName.now(), // can't do this, need `div.withOwner ( owner =>` to make into StrictSignal
    child.text <-- userFormattedName,
    ")"
  )
}

def renderUser(user: User, formattedName: StrictSignal[String], owner: Owner): Element = {
  div(
    user.name + " (",
    child.text <-- formattedName.map(userformat),
    ")"
  ).build(owner)
}

def renderUserSplit(id: String, userSignal: StrictSignal[User], owner: Owner)(formattedName: StrictSignal[String]): Element = {
  div(
    "Initial name: " + userSignal.now().name,
    child.text <-- userSignal.map(_.name),
    child.text <-- formattedName.map(userformat),
  ).build(owner)
}

For now, I still don't see how to get rid of laziness without paying the boilerplate price of flatmapping (the div.observed I started with) to bind derived observables to one of the child elements / resources. I don't like this syntax very much because it breaks the regular freeform-style that we currently enjoy in Laminar – we don't force you to define resources in a certain location or in a certain order. I guess calling .own() is slightly better visually, but then, we're stuck with strictSignal.map(foo) needing to be lazy. And if it remains lazy (until we call .own()), how is that fundamentally different from our lazy signals and calling .observe(owner) on them to get a strict signal?

Ask Arman later about helloCounter.map(_.toString) in calico example code – is that lazy or strict? And if that's strict, would it cause a memory leak if it was passed to a dynamic child resource? (screw that wording, give a specific example).

I think (but not sure) that Calico's observable transformations like .map can only have pure functions in them, and they don't have shared execution – maybe that somehow allows for a different memory model? I would understand if it was pull-based, but it appears to be push-based, meaning that parent observable needs to have references to child observables in order to propagate events. So, pure or not, it should still have GC-preventing links in the structure. Hrm.

@armanbilge
Copy link

I think (but not sure) that Calico's observable transformations like .map can only have pure functions in them, and they don't have shared execution – maybe that somehow allows for a different memory model?

Yes, that's right. Only pure functions, and responsibility for executing them actually falls to each subscriber to the signal. Mapping a signal basically only provides a "view", it does not allocate any state to cache an intermediate value.

I would understand if it was pull-based, but it appears to be push-based, meaning that parent observable needs to have references to child observables in order to propagate events.

Well, it's a bit of push-pull 🤔 subscribing to a signal returns something of Resource-like shape. That means the listener is responsible for managing the lifecycle of its subscription (typically by binding this to the lifecycle of the element its being used with.

Furthermore, once a signal fires an event (push), it actually clears out all of its listeners. It's up to listeners to resubscribe (pull) whenever they are interested and ready for events again. This is implemented such that if there was at least one event during that window where a listener was unsubscribed, it will be notified immediately upon re-subscription.

Ask Arman later about helloCounter.map(_.toString) in calico example code – is that lazy or strict?

I guess you mean this one, used in the routing example?

https://github.com/armanbilge/calico/blob/7adad4b3a61e1d701b596a791ee473eb71b581e7/docs/router.md?plain=1#L45

In that case, the SignallingRef is created when the entire app is created.

Then we setup a subscription here, which is bound to the lifecycle of the p(...) (which itself is bound to all its parents).

https://github.com/armanbilge/calico/blob/7adad4b3a61e1d701b596a791ee473eb71b581e7/docs/router.md?plain=1#L79-L80

As described by the push-pull dynamics above, I think I would say it's lazy? If nobody is listening to a signal (pull), then it is not firing events (push). However, it is still "alive" in the sense that you can .get (read peakNow()) it at any time to get its current value. If there are any transformations e.g. .map(_.toString) these would be applied on-demand.

Hope that helps :)

@armanbilge
Copy link

Regarding this one:

def renderUserResource(user: User, formattedName: StrictSignal[String]): Resource[Element] = {
  // ??? this userFormattedName still can't be strict, right?
  // this can't be owned by the parent owner, because then we're leaking memory just like current Airstream would
  // so we need some special syntax for this... Calico has flatMap but I'm looking for something lighter
  val userFormattedName = formattedName.map(user.formatAgain)
  div(
    user.name + " (",
    "Initial: " + userFormattedName.now(), // can't do this, need `div.withOwner ( owner =>` to make into StrictSignal
    child.text <-- userFormattedName,
    ")"
  )
}

Can't this be expressed something like this?

def renderUserResource(user: User, formattedName: StrictSignal[String]): Resource[Element] = Resource { owner =>
  val userFormattedName = formattedName.map(user.formatAgain) // use owner here ?
  div(
    user.name + " (",
    "Initial: " + userFormattedName.now(),
    child.text <-- userFormattedName,
    ")"
  ).build(owner)
}

@raquo
Copy link
Owner Author

raquo commented Dec 30, 2022

As described by the push-pull dynamics above, I think I would say it's lazy? If nobody is listening to a signal (pull), then it is not firing events (push). However, it is still "alive" in the sense that you can .get (read peakNow()) it at any time to get its current value. If there are any transformations e.g. .map(_.toString) these would be applied on-demand.

Great, that was my suspicion, thanks for confirming.

Can't this be expressed something like this?

Right, yes, we can just wrap it in another Resource, makes sense, thanks!

@raquo
Copy link
Owner Author

raquo commented Apr 20, 2023

Just a note to self – if we ever allow something like the peekNow() described in the original post, I will probably need to hide it behind a feature import, similar to unitArrows in Laminar 15. I still see people wanting to use .now() on signals where they shouldn't, and don't want to make it too easy to go down the wrong path.

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

3 participants