InboxSDK / InboxSDK

The InboxSDK lets you build apps for Gmail.
Other
733 stars 50 forks source link

Drop `event-listener-with-options` library #1198

Closed wegry closed 3 months ago

wegry commented 3 months ago

The native EventTarget.prototype.addEventListener allows options in all browsers we support. We use this library in one spot.

AlexVCS commented 3 months ago

I'd like to work on this issue. Can you please assign the issue to me?

AlexVCS commented 3 months ago

I'm working on this but coming up against an issue. Here is the modified fromEventsWithOptions code:

import * as Kefir from 'kefir';

export default function fromEventsWithOptions(
  target: EventTarget,
  eventName: string,
  options: boolean,
): Kefir.Observable<any, never> {
  return Kefir.stream((emitter) => {
    addEventListener(target, eventName, emitter.emit, options);
    return () => {
      removeEventListener(target, eventName, emitter.emit, options);
    };
  });
}

I get errors on both options declarations that are params passed into the add or removeEventListener functions. They say Expected 2-3 arguments, but got 4.. I get why that's happening, upon looking at the ScrollableContainByScreen file where fromEventsWithOptions is called. I've tried adding an addition type for emitter in fromEventsWithOptions, among other things, but haven't gotten it working. Any advice? Thanks

wegry commented 3 months ago

@AlexVCS in the snippet above, addEventListener is implicitly being called on window. It should probably be target.addEventListener(eventName, emitter.emit, options).

AlexVCS commented 3 months ago

@AlexVCS in the snippet above, addEventListener is implicitly being called on window. It should probably be target.addEventListener(eventName, emitter.emit, options).

Thanks! I saw that as a potential solution but wasn't sure about it 😅