Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to addEventListener for object containing handleEvent #2092

Closed
1 task done
mattrunyon opened this issue Nov 3, 2022 · 0 comments
Closed
1 task done

Comments

@mattrunyon
Copy link

mattrunyon commented Nov 3, 2022

Is there an existing issue for this?

  • I've searched for any related issues and avoided creating a duplicate issue.

Description

The Node EventTarget class supports both a function as the listener, and an object with a handleEvent function. I.e. addEventListener(name: string, listener: Function | { handleEvent: Function })

Node docs

I've read through #1818 and see it is unlikely this package will use the native Node EventTarget. I think it would be helpful to add this to the EventTarget shim to more closely match the Node implementation.

My use case is generated code which I don't control that is generating addEventListener(name, { handleEvent: () => {}). Valid in the browser, but breaks with Node using ws

I am currently working around by using the following. In an implementation in this package, it might be simpler to use handler.handleEvent.call(handler) to keep the this context

class CustomWs extends ws {
  static boundHandlerMap = new Map();

  addEventListener(name, handler, ...rest) {
    if (typeof handler.handleEvent === 'function') {
      const boundHandler = handler.handleEvent.bind(handler);
      boundHandlerMap.set(handler, boundHandler);
      super.addEventListener(name, boundHandler, ...rest);
    } else {
      super.addEventListener(name, handler, ...rest);
    }
  }

  removeEventListener(name, handler, ...rest) {
    if (typeof handler.handleEvent === 'function') {
      const boundHandler = boundHandlerMap.get(handler, boundHandler);
      super.removeEventListener(name, boundHandler, ...rest);
    } else {
      super.removeEventListener(name, handler, ...rest);
    }
  }
}

ws version

8.10.0

Node.js Version

16.17.0

System

OS: macOS 12.3.1

Expected result

socket.addEventListener('message', { handleEvent: e => console.log(e) }) should work identically to socket.addEventListener('message', e => console.log(e))

Actual result

The handleEvent object does not get added and the listener is never called

Attachments

No response

@lpinca lpinca closed this as completed in c59eda5 Nov 4, 2022
lpinca added a commit that referenced this issue Nov 6, 2022
Make `WebSocket.prototype.addEventListener()` support an event listener
specified as an object with a `handleEvent()` method.

Fixes #2092
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant