splitOption and splitOne combined stays behind one event

[arturaz]

I discovered a weird interaction between those two operators. See it in action:

https://scribble.ninja/u/arturaz/crhfsrdmulrwuvcotjevngtlsaoh

When you press + first time, first two labels does not update, whilst the next two work just fine.

Upon successive presses the first two labels shows a value of previous event.

Repro code in case scribble goes down:

  enum Bar {
    case A(value: Int)
  }
  case class Foo(bar: Option[Bar])
  val rx = Var(Foo(Some(Bar.A(3))))

  val splitOptionPlusMatchOneSignal = rx.signal.map(_.bar)
    .splitOption((_, bar) => 
      bar.splitMatchOne
        .handleType[Bar.A]((_, rx) => 
          span(child.text <-- rx.map(_.value.toString))
        )
        .toSignal
    )
    .map {
      case None => Signal.fromValue(None)
      case Some(signal) => signal.map(Some(_))
    }
    .flattenSwitch

  // Desugared version of `splitOptionPlusMatchOneSignal`
  val splitOptionPlusSplitOneSignal = rx.signal.map(_.bar)
    .splitOption((_, bar) => 
      bar.splitOne(_ => 0)((_, _, rx) => 
        span(child.text <-- 
          rx.map(_.asInstanceOf[Bar.A].value.toString)
        )
      )
    )
    .map {
      case None => Signal.fromValue(None)
      case Some(signal) => signal.map(Some(_))
    }
    .flattenSwitch

  val splitMatchOneSignal = rx.signal.map(_.bar)
    .mapSome { bar => Signal.fromValue(
      span(bar.asInstanceOf[Bar.A].value)
    ) }
    .map {
      case None => Signal.fromValue(None)
      case Some(signal) => signal.map(Some(_))
    }
    .flattenSwitch

  val splitOptionSignal = rx.signal.map(_.bar)
    .splitOption((_, bar) => 
      bar.map(bar => 
        span(bar.asInstanceOf[Bar.A].value.toString)
      )
    )
    .map {
      case None => Signal.fromValue(None)
      case Some(signal) => signal.map(Some(_))
    }
    .flattenSwitch

  def MyComponent(mods: Mod[HtmlElement]*): Element = {
    div(
      button("+", onClick --> { _ => rx.update { foo =>
        Foo(Some(Bar.A(foo.bar.get.asInstanceOf[Bar.A].value + 1)))
      } }),
      div(
        label("splitOptionPlusMatchOne: "), 
        child.maybe <-- splitOptionPlusMatchOneSignal
      ),
      div(
        label("splitOptionPlusSplitOneSignal: "), 
        child.maybe <-- splitOptionPlusSplitOneSignal
      ),
      div(
        label("splitMatchOne: "), 
        child.maybe <-- splitMatchOneSignal
      ),
      div(
        label("plain: "), 
        child.maybe <-- splitOptionSignal
      ),
      mods
    )
  }

Discussion in Laminar Discord: https://discordapp.com/channels/1020225759610163220/1020225760075718669/1341370171331182662

[arturaz]

@HollandDM I believe you were the one that wrote the macros? Any idea what could be happening here?

[arturaz]

@HollandDM the macro is fine, it is splitOne that is acting up.

[raquo]

So far I've reduced it to this:

https://scribble.ninja/u/raquo/cyutyarybiqietcmigwstvmxnpoq

  val rx = Var(0)

  // Bug
  val splitOptionPlusSplitOneSignal = rx.signal
    .splitOne(_ => "b")((_, _, vs) =>
      vs.splitOne(_ => "a")((_, _, rx) => 
        //rx.debugLog().observe(unsafeWindowOwner) // prints same values as Laminar renders to the UI
        span(
          text <-- rx.map(_.toString)
        )
      )
      // .map(identity) // here is fine though
    )
    .map(_.map(identity)) // <<< adding this triggers bug
    .flattenSwitch

  // No bug
  val splitOptionSignal = rx.signal
    .splitOne(_ => "c")((_, _, bar) => 
      bar.map(b => 
        span(b.toString)
      )
    )
    .map(_.map(identity))
    .flattenSwitch

My preliminarily tests show that Laminar is not necessary to trigger the issue, which is good, as that narrows it down a lot. I'll need to eliminate Laminar from the reproduction.

Under the hood, all the Signal split* operators use the same implementation, so probably the problem is either there, or in Signal's flattenSwitch. I suspect the latter, but we'll see. Rather perplexing, not quite sure what it could be yet, TBH.

I will look into this as soon as I can but it might take a week or two, it's not an easy bug, and I have a tough schedule right now. In the meantime, as a workaround, it seems that moving your signal.map to be inside the splitOne / splitOption bypasses this issue. If using splitOption, you can use its second ifEmpty param to specify the None case.

[raquo]

@arturaz Fantastic find, thanks! I was able to fix not one, but two unrelated issues thanks to this.

Please try 17.2.1-M1, it should fix the original issue.

Summary of the two issues:

• SplitChildSignal (the signal that the split callback gives you for each item) reads the internal state of SplitSignal (the output of the .split operator itself), but does not formally depend on it as an observable. This means that if SplitChildSignal has observers, but SplitSignal does not, SplitSignal is not updating its internal state, and so SplitChildSignal can potentially read stale state from SplitSignal.
• When flattenSwitch operator switches from signal1 to signal2, it first disconnects from signal1 before connecting to signal2. If both signal1 and signal2 have a common ancestor signal0, and signal0 has no other observers, this will cause signal0 to briefly stop and then re-start.

Together, these two issues were causing SplitSignal to briefly stop due to lack of observers when switching from one signal.map(identity) to another – even though it's the same signal, a new signal.map(identity) was created every time. Our flattenSwitch operator already had special logic for when signal1 == signal2, which is why removing the .map worked around the issue.

The fixes for the two issues are:

• When SplitChildSignal wants to read state from SplitSignal, it now checks that SplitSignal's state accounts for the latest data from SplitSignal's parent (parentLastUpdateId). If it sees that the data is stale, it triggers a pull on SplitSignal, updating its value and internal state, before reading its state again. SplitChildSignal does depend on the parent of SplitSignal, so it's safe for SplitSignal to pull its parent's value without any special ceremony.
• When flattenSwitch switches from one signal to another, it connects to the new signal before disconnecting from the old signal, preventing any common ancestor observables from being briefly stopped.

Actually, either of these fixes alone would have been sufficient to fix the specific misbehaviour in the reproduction, but both are needed to properly fix the underlying problems in the general case.

---

Migration: I guess it's possible that the flattenSwitch fix might expose a user's inadvertent reliance on this bug, but that's unlikely. The split bug seems exceptionally unlikely for anyone to depend on without realizing that it's a bug.