ThoughtWorksInc / Binding.scala

Reactive data-binding for Scala
https://javadoc.io/page/com.thoughtworks.binding/binding_2.13/latest/com/thoughtworks/binding/index.html
MIT License
1.59k stars 108 forks source link

Null cache exception in FlatMap #188

Closed basimkhajwal closed 5 years ago

basimkhajwal commented 5 years ago

I'm currently updating a project that was written under Binding.scala 10.0.3 (and compiled/ran fine) to 11.8.1. Updating the library caused a null error to surface that previously wasn't there.

Specifically, in line 408 of Binding.scala, oldCache is null.

Looking through the changes to the relevant code since 10.0.03, I can see that this commit added a cache which is initialised to null.

Either there was some invariant that meant that oldCache should never be null (in which I need to fix my code) or a null check needs to be made.

Atry commented 5 years ago

Could you provide a reproduction code, e.g. a link to https://scalafiddle.io/ ?

basimkhajwal commented 5 years ago

Working code under 10.0.3: https://scalafiddle.io/sf/bzRRp4g/0 Error under 11.8.1: https://scalafiddle.io/sf/HcOpljv/0

On a side note, I can see the approach that I took isn't very nice so I wanted to ask if there was a better to do what I intend (keep a cache that can both be mutated externally and whenever a cache miss occurs)

Atry commented 5 years ago

According to the Scaladoc for the value method: https://javadoc.io/page/com.thoughtworks.binding/binding_2.11/latest/com/thoughtworks/binding/Binding$$Vars.html#value:scala.collection.mutable.Buffer[A]

This method must not be invoked inside a @dom method body or a Binding { ... } block..

Atry commented 5 years ago

It does not make sense to change a Var or Vars in a data binding expression that depends on itself. It's like a barber paradox.

As a workaround, you can delay the change by wrapping the value calls into a Future:

import com.thoughtworks.binding.Binding._
import com.thoughtworks.binding._

def expensiveOperation(i: Int): Object = "Dummy"

// Cache the operations performed, a Var was needed
// because it could be mutated elsewhere
val objectCache: Vars[(Int,Object)] = Vars.empty

// Index mutated through UI
val selectedIdx: Var[Int] = Var(0)

// Value which is then used to render DOM
val objectValue: Binding[Object] = Binding {
  val idx = selectedIdx.bind
  val cache = objectCache.bind

  cache.find { case (i,_) => i == idx} match {
    case Some((_,obj)) => obj
    case None => {
      val newObject = expensiveOperation(idx)
      import concurrent.ExecutionContext.Implicits._
      concurrent.Future {
        objectCache.value += ((idx, newObject))
      }
      newObject
    }
  }
}

Binding {
  println(objectValue.bind)
}.watch()

https://scalafiddle.io/sf/HcOpljv/1

Even if you delay the change, recursive data binding expression is still likely buggy, because it may cause endless recalculation.

Atry commented 5 years ago

I know AngularJS automatically delay changes, allowing recursive data binding. This approach causes AngularJS's infamous stability problem.

In Binding.scala, recursive data binding requires explicit delay execution with a Future or setTiemout. You take the risk when you mean it.

basimkhajwal commented 5 years ago

Right, I understand.

One thing I'm still unsure about is the typical pattern for input elements which goes like:

import org.scalajs.dom.html

@dom
def inputElement(data: Var[String]): Binding[html.Input] = {
  <input oninput={e:Event => data.value = e.target.asInstanceOf[html.Input].value}
    value={data.bind} />
}

This is still a recursive binding that works however it still invalidates the principle that value_= should never be called within a @dom expression.

Moving the oninput to an external function kind of resolves it but it still effectively does the same thing (recursive) so I'm still unsure as to how to properly handle this situation (if there is an alternative).

Atry commented 5 years ago

The oninput handler and the Future block are functions other than the @dom function. You know, their return types are not Binding, and they will be not triggered by Var or Vars changes.

Basim Khajwal notifications@github.com于2019年7月20日 周六下午12:52写道:

Right, I understand.

One thing I'm still unsure about is the typical pattern for input elements which goes like:

import org.scalajs.dom.html

@dom def inputElement(data: Var[String]): Binding[html.Input] = { <input oninput={e:Event => data.value = e.target.asInstanceOf[html.Input].value} value={data.bind} /> }

This is still a recursive binding that works however it still invalidates the principle that value_= should never be called within a @dom expression.

Moving the oninput to an external function kind of resolves it but it still effectively does the same thing (recursive) so I'm still unsure as to how to properly handle this situation (if there is an alternative).

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ThoughtWorksInc/Binding.scala/issues/188?email_source=notifications&email_token=AAES3OVAJZ5OVAKNDPTVZN3QANUG7A5CNFSM4IFM5H52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NU4DA#issuecomment-513494540, or mute the thread https://github.com/notifications/unsubscribe-auth/AAES3OU4GZSQYPHEIYTGY2DQANUG7ANCNFSM4IFM5H5Q .

-- 杨博 (Yang, Bo)

Atry commented 5 years ago

Recursive code layout does not matter. What matters here is to avoid recursive Var changes.

Yang, Bo pop.atry@gmail.com于2019年7月20日 周六下午1:54写道:

The oninput handler and the Future block are functions other than the @dom function. You know, their return types are not Binding, and they will be not triggered by Var or Vars changes.

Basim Khajwal notifications@github.com于2019年7月20日 周六下午12:52写道:

Right, I understand.

One thing I'm still unsure about is the typical pattern for input elements which goes like:

import org.scalajs.dom.html

@dom def inputElement(data: Var[String]): Binding[html.Input] = { <input oninput={e:Event => data.value = e.target.asInstanceOf[html.Input].value} value={data.bind} /> }

This is still a recursive binding that works however it still invalidates the principle that value_= should never be called within a @dom expression.

Moving the oninput to an external function kind of resolves it but it still effectively does the same thing (recursive) so I'm still unsure as to how to properly handle this situation (if there is an alternative).

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ThoughtWorksInc/Binding.scala/issues/188?email_source=notifications&email_token=AAES3OVAJZ5OVAKNDPTVZN3QANUG7A5CNFSM4IFM5H52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NU4DA#issuecomment-513494540, or mute the thread https://github.com/notifications/unsubscribe-auth/AAES3OU4GZSQYPHEIYTGY2DQANUG7ANCNFSM4IFM5H5Q .

-- 杨博 (Yang, Bo)

-- 杨博 (Yang, Bo)