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 CloseEvent #46880

Closed
regseb opened this issue Feb 28, 2023 · 3 comments
Closed

Add CloseEvent #46880

regseb opened this issue Feb 28, 2023 · 3 comments
Labels
events Issues and PRs related to the events subsystem / EventEmitter. feature request Issues that request new features to be added to Node.js.

Comments

@regseb
Copy link
Contributor

regseb commented Feb 28, 2023

What is the problem this feature will solve?

The CloseEvent class isn't defined.

What is the feature you are proposing to solve the problem?

Add the CloseEvent class in the global scope. I found it in undici.

What alternatives have you considered?

Export CloseEvent class from events.

@regseb regseb added the feature request Issues that request new features to be added to Node.js. label Feb 28, 2023
@debadree25 debadree25 added the events Issues and PRs related to the events subsystem / EventEmitter. label Feb 28, 2023
@bnoordhuis
Copy link
Member

I'm inclined to say exporting CloseEvent doesn't make sense because it's a websockets things, something node doesn't support natively.

@regseb
Copy link
Contributor Author

regseb commented Feb 28, 2023

I thought WebSockets were implemented in Node. I use mock-socket which automatically adds the WebSocket class to the global scope, but not CloseEvent.

I agree to add CloseEvent when Node will support WebSocket. There is already an issue #19308 for adding WebSockets. And nodejs/undici#1795 already implements it, but it doesn't export CloseEvent https://github.com/nodejs/undici/pull/1795/files#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346:

    module.exports.getGlobalOrigin = getGlobalOrigin
  }

+ if (nodeMajor >= 18) {
+   const { WebSocket } = require('./lib/websocket/websocket')
+
+   module.exports.WebSocket = WebSocket
+ }
+
  module.exports.request = makeDispatcher(api.request)
  module.exports.stream = makeDispatcher(api.stream)
  module.exports.pipeline = makeDispatcher(api.pipeline)

@bnoordhuis
Copy link
Member

I'll close this then. It's something we can revisit if and when node implements websockets.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

3 participants