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

SplitChildSignal does not re-sync when restarted #120

Open
TomasStanek opened this issue Feb 25, 2024 · 5 comments
Open

SplitChildSignal does not re-sync when restarted #120

TomasStanek opened this issue Feb 25, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@TomasStanek
Copy link

Hello everybody,
I've encountered an unexpected behavior of a signal created by the split operator.

First the code:

import com.raquo.laminar.api.L._


object SplitSignalBehavior {

  def render() = {
    // set of visible keys
    val visibleSetVar: Var[Set[Int]] = Var(Set.empty)

    def toggleVisibility(i: Int) = {
      val s = visibleSetVar.now()

      val s2 =
        if (s.contains(i)) s - i
        else s + i

      visibleSetVar.set(s2)
    }


    var j = 0

    // example of a sequence with _1 key and _2 value
    def computeSequence(): Vector[(Int, Int)] = {
      (1 to 3).iterator.map(i => (i, j)).toVector
    }

    val seqVar = Var(computeSequence())

    // increment j and recompute sequence
    def increment() = {
      j += 1
      seqVar.set(computeSequence())
    }

    div(
      button(
        "increment",
        onClick --> Observer { _: Any => increment() }
      ),
      table(
        children <-- {
          seqVar.signal.split(_._1) { case (key, _, badSig) =>
            val goodSig = seqVar.signal.map(_.find(_._1 == key).get)

            tr(
              td(
                button(
                  key.toString,
                  onClick --> Observer { _: Any => toggleVisibility(key) }
                )
              ),
              td(
                child <-- visibleSetVar.signal.map(_.contains(key)).map { isVisible =>
                  if (isVisible)
                    div(
                      child.text <-- badSig.map(_._2.toString), // keeps an old value
                      "|",
                      child.text <-- goodSig.map(_._2.toString) // works as expected
                    )
                  else
                    div("hidden")
                }
              )
            )
          }
        }
      )
    )
  }
}

The behavior (latest laminar):
Each row of the table has a show/hide toggle button which shows a hypothetical component.
When a component is shown (attached to the dom) and the increment button is pressed, everything works as expected (both numbers in the row are updated).
When a component is hidden (not attached to the dom), the increment button is pressed, then the component is shown, only the value from goodSig is the current one and the value from badSig is old.
In other words the values comming from the badSig are updated only when the element is mounted.

Behavior of different laminar versions:
Versions 15.0.0, 16.0.0 and 17.0.0-M6 exhibit the described behavior
My local version 0.14.2 with applied fix for #95 exhibits the same behavior.
Version 0.14.2 from maven behaves such that none of the values are updated when hidden (although the behavior of the goodSig and badSig differs in terms of the first mount).

Best regards,
Tomas Stanek

@yurique
Copy link
Sponsor Contributor

yurique commented Feb 25, 2024

I tried reproducing it here:
https://scribble.ninja/u/yurique/dhzaiieqevjkokiyajlxyrhilmh

And it does reproduce.
Also, if you uncomment the badSig.signal.debugLogEvents() - it starts to behave correctly.

@raquo raquo closed this as completed in 57b0938 Feb 26, 2024
@raquo
Copy link
Owner

raquo commented Feb 26, 2024

Thanks for reporting with a nice reproduction!

In 0.14.x, this is the expected behaviour, however in v15 and up, this child signal should have synced up with the parent signal when the former was re-started (when its isVisible changed from false to true). The fact that this does not happen is indeed a bug.

The root of this problem in v15 and up is that SplitChildSignal (the type of your badSig) did not have proper re-syncing implemented, and the standard re-syncing logic did not work for it, because technically, internally, its parent is a special timing stream, not the signal that the users would expect semantically.

The fix is not binary compatible with Airstream v17.0.0-M6 but is binary compatible with Laminar v17.0.0-M6, and probably with most other libraries that depend on Airstream v17.0.0-M6.

I will publish Airstream v17.0.0-M8 a bit later, but for now, you can try to publishLocal the latest Airstream master, and try using it with Laminar v17.0.0-M6 and other dependencies that work with it.

@raquo raquo reopened this Feb 26, 2024
@raquo raquo added the bug Something isn't working label Feb 26, 2024
@raquo raquo changed the title Behavior of split signal attached to mounted/unmounted dom element SplitChildSignal does not re-sync when restarted Feb 26, 2024
@TomasStanek
Copy link
Author

Thank you for your quick response.
I have tried the local version and it solves the issue partially.

Steps to reproduce:
open the page
click button 1 to show the component
click increment
click button 1 to hide the component
click increment
click button 1 to show the component
-- component 1 has both numbers consistent -- correct so far
click button 2 to show the component
-- component 2 shows 0|2 (same for component 3)

raquo added a commit that referenced this issue Feb 28, 2024
… it is available.

The initial value that we used to get before can get stale if the child signal is started only after the parent signal emits some events. This is a fix for Tomas followup comment in #120.
@raquo
Copy link
Owner

raquo commented Feb 28, 2024

Ah, indeed, the problem was a bit more complicated than I thought, and my initial fix was not quite right. I have reworked the solution now, and with the latest commit, it works as expected.

@TomasStanek
Copy link
Author

The latest commit works for me.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants