scala-js/scala-js-dom

All facade default arguments should be `js.native`

japgolly opened this issue · 4 comments

japgolly commented

Enforce via scalafix

armanbilge commented

Mostly in agreement with this, the only thing nice about not always doing this is that the default arguments can be self-documenting of the underlying default. Or also wrong or out of sync (which is confusing in code but admittedly not unexpected in scaladoc).

japgolly commented

Yeah I hear what you're saying. This is my current thinking: I didn't even realise defaults other than js.native would even compile, I have a vague sense that it wouldn't back in the day. So in terms of default facade args, I'm equivalent to a new user in not knowing how it works. I see two possibilities:

  1. it's just informational and not actually used (i.e. def a(x=1)=native; a() is equivalent to a() in JS). In which case I find default args misleading. The norm is Scala is that default arguments are used and knowing that they aren't in this case requires some very fine-print reading of the Scala.js docs, if it's even explicitly mentioned at all.

  2. it's used (i.e. def a(x=1)=native; a() is equivalent to a(1) in JS) in which case, how can I produce a Scala equivalent of a() in JS?

Those two options seem like a total dichotomy to me, and retaining defaults other than js.native results in a negative result in both cases. I think ScalaDoc should be the standard place for non-functional information.

armanbilge commented

Hmm, that is confusing! I think what is happening is they all become js.undefined actually. But since only js.UndefOr types can be js.undefined then we use js.native as the default argument since that works everywhere.

👍 let's go ahead with this. The only place I saw non-js.native used (where you first caught it) the default argument was wrong anyway lol. But I'm sure it's littered over the code base.

  • 👍1
japgolly commented

Some places can't use js.native as their default args for now, see scala-js/scala-js#4553

  • 😕1