search

paf31 / purescript-jquery

Type declarations for jQuery
MIT License
30 stars 28 forks source link

Add some standard JQuery event fields #20

Closed pelotom closed 7 years ago

pelotom commented 9 years ago

Per documentation here: http://api.jquery.com/category/events/event-object/

paf31 commented 9 years ago

This looks useful, but seems like it will restrict us to only using these fields. Maybe an events module with some FFI functions would be more easily extended?

Also, are these fields always defined for every event? The page doesn't seem clear about that.

pelotom commented 9 years ago

I think eventually it would be cool to have different onFoo registration functions with associated specialized event types, e.g.

onClick :: forall e a. String -> (MouseEvent -> JQuery -> Eff (dom :: DOM | e) a) -> JQuery -> Eff (dom :: DOM | e) JQuery

with fields as documented here. At that point I'd advocate no longer exposing the more general on function.

But as it stands, it makes sense for the event type associated with on to have whatever fields can be guaranteed for all event types, which I believe is the case for the fields I've added here. It's certainly no more unsafe than just using an opaque foreign imported type, which forces the user to write some FFI to extract whatever fields they believe it to have. They still might have to do that for some fields, but for those whose presence we can guarantee, we should expose them, right? It doesn't restrict someone from using fields not defined here, it just means they have to write FFI, which they would've had to do anyway otherwise.

pelotom commented 9 years ago

I confess I'm not sure what "standard practice" is on when to use foreign functions to access foreign data vs. simply exposing the underlying JavaScript object fields via record types. It seems like there's a minor efficiency argument for the latter, as well as the nice fact that you don't have to pollute the general namespace with these accessor functions. Also I wonder, if this isn't a good case in which to use record types, what is?

garyb commented 9 years ago

Those properties are present on all jQuery event objects as far as I know.

stefankoegel commented 9 years ago

In my opinion, because JQuery events always have these properties, it would be correct to expose them via record types. Later on it would still be possible to add more specialized events with more or other properties, like @pelotom suggested.

This feature would also be useful in one of my projects, by removing one FFI function (or one unsafeFromForeign).

yemi commented 9 years ago

+1 This would be great to have

paf31 commented 7 years ago

I'll add these as getters in the next release. Thanks for the PR anyway and sorry for the delay.