scala-js / scala-js-dom

Statically typed DOM API for Scala.js
Other
315 stars 160 forks source link

LockOptions doesn't use val for fields, making overriding not possible #833

Closed jypma closed 5 months ago

jypma commented 5 months ago

Most traits in scala-js-dom extending js.Object use val for their members, so they can be overridden selectively. See e.g. IDBCreateObjectStoreOptions. This pattern is also recommended by the scalajs documentation:

trait MyObject extends js.Object {
  val foo: Int = js.native
  val bar: String = js.native
}

This allows a user to selectively override only some fields, e.g.

new MyObject {
  override def foo = 5
}

However, dom.LockOptions currently uses var. This means that this object needs to be instantiated as new LockOptions {}, and then the mutable fields can be changed.

This makes LockOptions inconsistent with the other objects. However, changing it to val would be a binary incompatible change, so I'm not sure of the proper way forward.

sjrd commented 5 months ago

The var idiom is the new, better one. We should update the Scala.js docs if they still appear to recommend the val idiom by default.

(abstract val is good for non-optional members; optional ones are better served with a var ... = js.undefined)

jypma commented 5 months ago

Ah, thanks a lot for clarifying! I guess it does make sense, JS objects are mutable either way. I suppose that inverts my original question ;-)