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

Moving an element from one parent to another can cause re-mounting #163

Open
raquo opened this issue Aug 5, 2024 · 0 comments
Open

Moving an element from one parent to another can cause re-mounting #163

raquo opened this issue Aug 5, 2024 · 0 comments
Labels
bug needs design The solution is not clear, or I am not very happy with it

Comments

@raquo
Copy link
Owner

raquo commented Aug 5, 2024

Background

In Laminar you can move an element from one parent to another, e.g. by dynamically including / excluding it from a pair of child <-- and children <-- contents. The design intent behind this feature is for the element to retain its mounted status, so that its subscriptions remain active.

(Note: the problem described here does not apply when moving elements within a single children <-- block).

Problem

However, in some cases, the element is unmounted from its current location before it's mounted to the new location, and so the element's subscriptions are deactivated and re-activated. That includes running onUnmount / onMount hooks. This is undersirable, both for predictability and performance reasons, since the main reason to move elements like this is to either retain complex element state, or to get max performance.

Reproduction by @felher (discord, scribble):

      val onTop = Var(true)
      val numMounts = Var(0)
      val elem = div("element", onMountCallback(_ => numMounts.update(_ + 1)))

      div(
        child.text <-- numMounts.signal.map(_.toString),
        button("toggle", onClick --> (_ => onTop.update(!_))),
        div(
          "top: ",
          child <-- onTop.signal.map({
            case true => elem
            case false => "disabled"
          })
        ),
        div(
          "bottom: ",
          child <-- onTop.signal.map({
            case true => "disabled"
            case false => elem
          })
        )
      )

In this reproduction, when onTop.signal emits, the first child <-- effect is triggered, unmounting elem. This happens before the second child <-- effect begins executing.

Solution?

I'm not yet sure what the solution should be.

I guess one approach would be to delay the deactivation of the element's subscriptions until the end of the current transaction. But this means that the element's subscriptions will remain active even after the element has been removed from the DOM, even if very briefly. Any subscriptions bound to the element being unmounted will survive for a bit longer, and will still be active after the element has been removed from the DOM.

  • This would be a breaking change, because if any of those subscriptions would have fired in the same transaction as the event that unmounted that element, previously, they wouldn't fire, and now, they would. I think.
  • I'm not 100% sure, but I think that currently such subscriptions don't have a chance to observe the element while it's unmounted from the DOM. If that's true, then implementing such a fix would start allowing events / effects in the same transaction to access the element when it's already removed from the real DOM tree, and this could break some of them, e.g. if they try to access the element's dimensions, that are only accessible while the element is mounted.

So I'm not sure if that would be a good solution. Needs some thinking, and deeper investigation.

Perhaps this could be opt-in somehow, like an element could be marked as "movable" to opt-in to the delayed unmounting logic? Well, maybe that could be the way to gradually roll this out, to reduce disruption. Although that option might be hard for users to discover.

Workaround

In some cases you may be able to use Airstream's delaySync operator to make sure that the event fired by the "sender" of the element is fired prior to the event fired by the "receiver". In this reproduction, both of those are onTop.signal, which is ok, but the solution won't work because the element can be moved in both directions (well, it will work for only one of those directions).

@raquo raquo added bug needs design The solution is not clear, or I am not very happy with it labels Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs design The solution is not clear, or I am not very happy with it
Projects
None yet
Development

No branches or pull requests

1 participant