EventTarrget.addEventListener event subclass

[schettino72]

Currently it is not possible to use an event listener that takes a subclass of Event as a parameter.
This change enables any subclass of Event to be taken as a parameter of the listener.

[sjrd]

The first commit should not be in this PR.

[sjrd]

My concern with this change is that it is not typesafe. The type of event depends on the type parameter, and can't be arbitrary. If we had literal singleton types, we could type appropriately. But with current Scala the only safe typing is to accept only functions taking Event.

[schettino72]

The first commit is a separate PR #81. It was included here since I wanted to include an example testing the functionality.

My concern with this change is that it is not typesafe. The type of event depends on the type parameter, and can't be arbitrary.

I am new to Scala and still a bit confused regarding its type system. Can you show an example demonstrating it is not type safe?

I figure out that if I tried to parametrize only the function parameter it wouldn't compile because of type safe issues. So my solution was to parametrize the whole function (and not just the function parameter). My understanding is that this way scala would create different type-safe versions of the function based on the parameter type.

[vendethiel]

@schettino72 The type that listener (2nd parameter) should take actually depends on addEventListener's type (1st parameter). However, it is not (yet?) possible to give overloads of addEventListener depending on the content of said first variable.
The same problem exists for Canvas.getContext (should have a different result type: for "2d", it should be a CanvasRenderingContext2D, but for "webgl" and "webgl2", it should be a WebGLRenderingContext)

[sjrd]

Concretely, with your change one can write

x.addEventListener("mousedown", { e: dom.KeyboardEvent =>
  // boom
}

This blows up because the name of the event and the event handler type do not match up.

[schettino72]

Reference to the SIP http://docs.scala-lang.org/sips/pending/42.type.html

Thanks for explaining... I guess I understand now. But IMO the situation is a bit different from Canvas.getContext because it takes only a few known string-values as types.

While addEventListener can take user defined event types. So even if you have Singleton types how would the user create an addEventListener for his own event type?
And if the user is responsible for setting the correct Listener type for the given event type it comes back to same situation as my patch... where you would have a failure if the user specifies the wrong type.

So my conclusion is that it is indeed not type safe but there is no other way (Singleton types could solve the problem only for pre-defined events generated by the browser - not custom user defined events). Or do you have any other suggestions on how to write the code from my example?

[sjrd]

Yep, doesn't look like there is any way to define this method properly. The current version is the only typesafe variant. But at the same time, it's true that it is annoying to use in practice. Maybe this is a case where convenience should prevail over safety. Anyway, I believe most people write their handler with a hard asInstanceOf instead to get the right type of event.

So I would agree with this change. It shouldn't be merged before #81 is merged and this is rebased, or the first commit is removed in favor of just having the change in the definition.

[schettino72]

I removed the first commit and rebased. Thanks.

[sjrd]

LGTM

[lihaoyi]

yay =)