Consistently set defaults for `new`-based constructors

[yurique]

MutationObserverInit is defined as a trait with vars and initial values, thus doing this won't compile:

new MutationObserverInit {
  val childList: UndefOr[Boolean] = someChildList
  ...

As a slight inconvenience, because the fields are initialized in the trait, the override modifier is required.
But what doesn't work is overriding the vars: scalac complains mutable variable cannot be overridden.

[sjrd]

This is intentional. The intended way to use it is as follows:

new MutationObserverInit {
  childList = someChildList
}

[yurique]

Ahh, indeed! My bad! Feeling kinda stupid now =)
Thank you, Sébastien!

[yurique]

I think this was what had thrown me off:

// Before (deprecated)
val response = dom.experimental.ResponseInit(myStatus, myStatusText, myHeaders)

// After
val response = new dom.ResponseInit {
  var status = myStatus
  var statusText = myStatusText
  var headers = myHeaders
}

[armanbilge]

Hmm, is the use of var not needed there? I grabbed that example from one of my own projects:
https://github.com/http4s/http4s-dom/blob/b900066aec70b39286925684b9501e66b36c11fe/dom/src/main/scala/org/http4s/dom/ServiceWorker.scala#L92-L95

Update: I tried removing it, but it didn't compile, so idk 🙃

[yurique]

@armanbilge Here's how I understand it :)

ResponseInit is defined like this (vars are abstract):

trait ResponseInit extends js.Object {
  var status: Int
  var statusText: ByteString
  var headers: HeadersInit
}

and this compiles (overriding-implementing the vars from the trait):

  val response = new org.scalajs.dom.ResponseInit {
    var status     = 200
    var statusText = "OK"
    var headers    = scala.scalajs.js.Dictionary.empty[String]
  }

without putting vars there it won't compile because those need to be overriden (implemented).

By contrast, MutationObserverInit is defined like this (there are default implementations for the vars):

trait MutationObserverInit extends js.Object {
  var childList: js.UndefOr[Boolean] = js.undefined
...

and scalac won't allow overriding them (vars) – I think this was the first time I ever tried to override a var – so the way to initialize them is as Sébastien suggested: by creating a new instance (with default initial values for those vars) and then imperatively re-assigning those vars in the constructor:

new MutationObserverInit {
    this.childList = somChildList
}

The benefit of it is we don't have to specify(implement) all the vars, only those that we care about.

[armanbilge]

Aha, excellent clarification! Thank you.

Seems like we need to be more consistent with this? I.e. ResponseInit should be changed to use js.undefined.

[yurique]

Consistency would be nice!

But I can see why we have it done this way: for a ResponseInit, it doesn't make sense to omit any of the fields; while for the MutationObserverInit, having most of the values set to undefined is okay and intended.

[armanbilge]

I thought the same, so I checked MDN and in fact their example omits the headers! Which is impossible with ours.

https://developer.mozilla.org/en-US/docs/Web/API/Response/Response#examples

Given that this inconsistency can be confusing/surprising, I think we should change this.

Also, not necessarily all fields have to default to js.undefined, we can do that only for optional fields.

[yurique]

Indeed. Interesting 🤔.

Well, I'm really not sure what the best way to approach this is. A change like this would be a breaking change, at the very least.

[armanbilge]

Source breaking but not binary-incompatible, so if we decide to go forward with it, it can land in 2.1.0.