Fix #219: Fix Event constructors. #367
Conversation
| } | ||
| | ||
| trait DeviceOrientationEventInit extends js.Object { | ||
| trait DeviceOrientationEventInit extends dom.raw.EventInit { |
There was a problem hiding this comment.
I would recommend importing dom.raw.EventInit at the top of the file, and then use EventInit everywhere. There might be other files where this comment applies.
src/main/scala/org/scalajs/dom/experimental/serviceworkers/ServiceWorkers.scala Show resolved Hide resolved
| var isReload: js.UndefOr[Boolean] = js.undefined | ||
| var request: js.UndefOr[Request] = js.undefined | ||
| var clientId: js.UndefOr[String] = js.undefined | ||
| |
There was a problem hiding this comment.
And there should be no blank line here.
src/main/scala/org/scalajs/dom/experimental/serviceworkers/ServiceWorkers.scala Show resolved Hide resolved
| * Returns the event's data. It can be any data type. | ||
| */ | ||
| val data: Any = js.native | ||
| val data: js.Any = js.native |
There was a problem hiding this comment.
Why did this become a js.Any? Is the data interpreted by JS code in any way? If it is just carried around, it should stay Any.
| * MDN | ||
| */ | ||
| def data: Any = js.native | ||
| def data: String | Blob | ArrayBuffer = js.native |
There was a problem hiding this comment.
MDN says that data can be any data type, so this should stay Any.
| def initMessageEvent(typeArg: String, canBubbleArg: Boolean, | ||
| cancelableArg: Boolean, dataArg: js.Any, originArg: String, | ||
| lastEventIdArg: String, sourceArg: Window): Unit = js.native | ||
| cancelableArg: Boolean, dataArg: String | Blob | ArrayBuffer, |
There was a problem hiding this comment.
This should probably be Any as well.
| } | ||
| | ||
| trait CustomEventInit extends EventInit { | ||
| var detailArg: js.UndefOr[js.Any] = js.undefined |
There was a problem hiding this comment.
I am pretty sure this should an Any as well.
| @js.native | ||
| @JSGlobal | ||
| class BeforeUnloadEvent extends Event { | ||
| class BeforeUnloadEvent extends Event("") { |
There was a problem hiding this comment.
It is more customary to use js.native as the arguments to another argument, in order to be confused that "" is not actually taken into account.
There was a problem hiding this comment.
Ah, I did not know the custom / tips.
That sounds reasonable.
Fixed in b7194a5
| @JSGlobal | ||
| class MutationEvent(typeArg: String, init: js.UndefOr[MutationEventInit]) | ||
| extends Event(typeArg, init) { | ||
| class MutationEvent extends Event("", js.undefined) { |
There was a problem hiding this comment.
If it has no constructor, its constructor should be marked private. Otherwise there is nothing preventing to do new MutationEvent from the Scala side (with a run-time error).
There was a problem hiding this comment.
Fixed in 879d4ae
Used private[this] instead of private(), thanks to deprecation warning declaring private constructors in native JS classes is deprecated, because they do not behave the same way as in Scala.js-defined JS classes. Use `private[this]` instead. This will become an error in 1.0.0.
| } | ||
| | ||
| @deprecated("Obsolete.", "WHATWG DOM") | ||
| trait MutationEventInit extends EventInit { |
There was a problem hiding this comment.
Do we still need this definition? Couldn't we just not define it at all?
2 participants
Closes #219
***EventInitnew ***Event()raises RuntimeError due to lack of required argument@deprecatedbased on MDN