FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: expose websocket in nodehttp by anfibiacreativa · Pull Request #53721 · nodejs/node · GitHub

/ node Public

http: expose websocket in nodehttp - #53721

Merged
nodejs-github-bot merged 27 commits into
nodejs:mainfrom
anfibiacreativa:feat/expose-websockets-in-nodehttp
Jul 8, 2024
ConversationCommits (27)ChecksFiles changed
Merged

http: expose websocket in nodehttp#53721
nodejs-github-bot merged 27 commits into
nodejs:mainfrom
anfibiacreativa:feat/expose-websockets-in-nodehttp

Conversation

anfibiacreativa commented Jul 4, 2024

Copy link
Copy Markdown
Member

This PR exposes Websocket in node:http as requested in #53684
Once we discuss, if it satisfies the requirement, we can backport to Node.js 20 and 22

CC: @mcollina @nodejs/http @manekinekko

nodejs-github-bot commented Jul 4, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 4, 2024
mcollina reviewed Jul 4, 2024
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good job! Docs are missing too.

Can you add a test that the given objects are the same of the global?

Comment thread lib/http.js Outdated
richardlau added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 4, 2024

avivkeller commented Jul 4, 2024

Copy link
Copy Markdown
Member

Once we discuss, if it satisfies the requirement, we can backport to Node.js 20 and 22

I've added lts-watch-v20.x to the PR, as it may need to be backported to v20, but feel free to remove the label if you disagree.

avivkeller reviewed Jul 4, 2024
View reviewed changes
Comment thread test/parallel/test-http-import-websocket.js Outdated
Comment thread lib/http.js
Comment thread lib/http.js
avivkeller self-requested a review July 4, 2024 21:21
avivkeller reviewed Jul 4, 2024
View reviewed changes

avivkeller left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, just needs some documentation, but from (mostly, except minor changes) here-on-out, I'll leave the reviewing to collaborators.

avivkeller added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jul 5, 2024

PuruVJ commented Jul 5, 2024

Copy link
Copy Markdown

Thank you @anfibiacreativa for implementing this so darn fast! 🚀🙏

manekinekko commented Jul 5, 2024

Copy link
Copy Markdown
Contributor

As discussed in #53684, can we track this change to be backported to v22.x as well?

anfibiacreativa commented Jul 5, 2024

Copy link
Copy Markdown
Member Author

As discussed in #53684, can we track this change to be backported to v22.x as well?

Yes, @mcollina should we do the backporting in the same PR?

avivkeller commented Jul 5, 2024

Copy link
Copy Markdown
Member

As discussed in #53684, can we track this change to be backported to v22.x as well?

Once this lands, backport-requested-... labels can be added if needed.

There's no LTS watch label for v22.x yet.

mcollina commented Jul 5, 2024

Copy link
Copy Markdown
Member

As discussed in #53684, can we track this change to be backported to v22.x as well?

This will be released in v22 first, then backported to v20.x after at least two weeks.

lpinca commented Jul 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

It may be the most appropriate module, but why http? WebSocket is a different protocol and the implementation uses undici under the hood which shares very little/no code with the Node.js http module.

We don't expose fetch, Request, Response, etc. in the http module.

avivkeller commented Jul 5, 2024

Copy link
Copy Markdown
Member

What would you recommend exposing it under (if it were to be exposed at all), net?

lpinca commented Jul 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What would you recommend exposing it under (if it were to be exposed at all), net?

Just like other new globals I would not backport it. Anyway, that is my personal opinion, not a blocker.

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2024
nodejs-github-bot merged commit a1869fa into nodejs:main Jul 8, 2024

nodejs-github-bot commented Jul 8, 2024

Copy link
Copy Markdown
Collaborator

Landed in a1869fa

avivkeller added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 8, 2024

manekinekko commented Jul 8, 2024

Copy link
Copy Markdown
Contributor

Congrats @anfibiacreativa on your 1st contribution to node core!

Thank you all for taking the time to review these changes.

aduh95 pushed a commit that referenced this pull request Jul 12, 2024
http: expose websockets
ec0837a 
PR-URL: #53721
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 mentioned this pull request Jul 12, 2024
Merged
aduh95 added a commit that referenced this pull request Jul 12, 2024
2024-07-15, Version 22.5.0 (Current)
df75c77 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) #53721
module:
  * add __esModule to require()'d ESM (Joyee Cheung) #52166
path:
  * (SEMVER-MINOR) add `matchGlob` method (Aviv Keller) #52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) #53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) #53462
worker:
  * (SEMVER-MINOR) add postMessageToThread (Paolo Insogna) #53682

PR-URL: #53826
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
http: expose websockets
1367c55 
PR-URL: #53721
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 added a commit that referenced this pull request Jul 16, 2024
2024-07-16, Version 22.5.0 (Current)
3f9c06e 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) #53721
lib:
  * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) #53752
module:
  * add `__esModule` to `require()`'d ESM (Joyee Cheung) #52166
path:
  * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) #52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) #53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) #53462
test_runner:
  * support glob matching coverage files (Aviv Keller) #53553
worker:
  * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) #53682

PR-URL: #53826
aduh95 added a commit that referenced this pull request Jul 16, 2024
2024-07-16, Version 22.5.0 (Current)
433f759 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) #53721
lib:
  * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) #53752
module:
  * add `__esModule` to `require()`'d ESM (Joyee Cheung) #52166
path:
  * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) #52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) #53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) #53462
test_runner:
  * support glob matching coverage files (Aviv Keller) #53553
worker:
  * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) #53682

PR-URL: #53826
aduh95 added a commit that referenced this pull request Jul 17, 2024
2024-07-17, Version 22.5.0 (Current)
34de839 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) #53721
lib:
  * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) #53752
module:
  * add `__esModule` to `require()`'d ESM (Joyee Cheung) #52166
path:
  * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) #52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) #53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) #53462
test_runner:
  * support glob matching coverage files (Aviv Keller) #53553
worker:
  * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) #53682

PR-URL: #53826
RafaelGSS pushed a commit that referenced this pull request Jul 17, 2024
2024-07-17, Version 22.5.0 (Current)
db1c2be 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) #53721
lib:
  * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) #53752
module:
  * add `__esModule` to `require()`'d ESM (Joyee Cheung) #52166
path:
  * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) #52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) #53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) #53462
test_runner:
  * support glob matching coverage files (Aviv Keller) #53553
worker:
  * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) #53682

PR-URL: #53826
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
2024-07-17, Version 22.5.0 (Current)
274b746 
Notable changes:

http:
  * (SEMVER-MINOR) expose websockets (Natalia Venditto) nodejs#53721
lib:
  * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) nodejs#53752
module:
  * add `__esModule` to `require()`'d ESM (Joyee Cheung) nodejs#52166
path:
  * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) nodejs#52881
process:
  * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) nodejs#53239
stream:
  * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) nodejs#53462
test_runner:
  * support glob matching coverage files (Aviv Keller) nodejs#53553
worker:
  * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) nodejs#53682

PR-URL: nodejs#53826

MTyson commented Jul 20, 2024

Copy link
Copy Markdown

Is there any info on this yet? It looks like, based on the code, that it is just exposing undici lib and that is just a client, not a server?

mcollina commented Jul 21, 2024

Copy link
Copy Markdown
Member

This will be released in the next version of Node v22.

We don't plan to add a server.

mcollina commented Aug 22, 2024

Copy link
Copy Markdown
Member

@marco-ippolito @nodejs/releasers I saw this was not picked up by the latest 20.17. How can we make sure it's picked up in the next one?

marco-ippolito commented Aug 22, 2024

Copy link
Copy Markdown
Member

@marco-ippolito @nodejs/releasers I saw this was not picked up by the latest 20.17. How can we make sure it's picked up in the next one?

Yes, I was unsure because we didnt unflag websocket client on v20

mcollina commented Aug 22, 2024

Copy link
Copy Markdown
Member

The whole point of this is exposing the Websocket client in v20. It's written in the linked issue.

Can you make sure it's in the next one?

KhafraDev commented Aug 22, 2024

Copy link
Copy Markdown
Member

WebSocket can be exposed in node v20 (at least in 20.17.0) with --experimental-websocket as a global

avivkeller mentioned this pull request Sep 13, 2024
Closed

targos commented Sep 21, 2024

Copy link
Copy Markdown
Member

This lands cleanly on v20.x-staging but the test doesn't pass:

  • It needs --experimental-websocket
  • Even with the flag, assert.strictEqual(NodeHttpMessageEvent, MessageEvent); fails because the global MessageEvent in v20.x is not the undici one.

This was referenced Jul 1, 2026
Open
Merged
bolshoytoster mentioned this pull request Jul 16, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

avivkeller avivkeller requested changes
mcollina mcollina approved these changes
ShogunPanda ShogunPanda approved these changes
jasnell jasnell approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL