Avoid duplicate rows / eventListener

[nwolverson]

Thanks for fixing #12 , didn't get around to test before making PR. But I think despite mentioning it you introduced a duplicate row issue with DOM, EventListener needs to include or not include DOM everywhere or it gets doubled up.

[garyb]

Actually, I think I fixed this the wrong way, dom :: DOM shouldn't appear in eventListener's effects at all and we should just use eff in both locations - that should allow us to use the add/remove in its current form which definitely seems more like the right type to me. Would you mind making that change instead please? (Assuming it works!)

[nwolverson]

I don't think removing it entirely works, if you don't include the dom :: DOM in the callback's row, then the callback can't use a DOM effect without causing a duplicate row on the addEventListener. I think either has to be "baked in" to EventListener or mentioned on each EventListener. By baked in I mean:

foreign import eventListener :: forall eff a. (Event -> Eff (dom :: DOM | eff) a) -> EventListener (eff)

foreign import addEventListener :: forall eff. EventType -> EventListener (eff) -> Boolean -> EventTarget -> Eff (dom :: DOM | eff) Unit

(i.e. original version only allowed DOM effect on the handler, the "don't mention" would allow anything except DOM. Unless you meant the above)

[garyb]

I'm suggesting:

foreign import eventListener :: forall eff a. (Event -> Eff eff a) -> EventListener eff

foreign import addEventListener :: forall eff. EventType -> EventListener (dom :: DOM | eff) -> Boolean -> EventTarget -> Eff (dom :: DOM | eff) Unit

So eventListener isn't specifically tied to DOM at all, it's up to the use case for DOM to be introduced. I think addEventListener needs to use the (dom :: DOM | eff) row in both cases, as I think we'll end up with duplicate row errors if it is only present on the output where the listener does need to do some DOM effect.

[nwolverson]

Ahh that makes more sense. Yes, that sure seems to work out.

[garyb]

Thanks!