Skip to content

lib: enable WebSocket by default#51594

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
Uzlopak:expose-websocket-by-default
Feb 4, 2024
Merged

lib: enable WebSocket by default#51594
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
Uzlopak:expose-websocket-by-default

Conversation

@Uzlopak

@Uzlopak Uzlopak commented Jan 29, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

For node 22 we enable WebSocket by default.

Is the target correct?

@Uzlopak Uzlopak marked this pull request as ready for review January 29, 2024 13:12
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jan 29, 2024
Uzlopak
Uzlopak commented Jan 29, 2024
View reviewed changes
Comment thread doc/api/globals.md Outdated
@Uzlopak Uzlopak force-pushed the expose-websocket-by-default branch from 5051680 to 6858b5f Compare January 29, 2024 13:15
@Uzlopak Uzlopak changed the title feat: enable WebSocket by default lib: enable WebSocket by default Jan 29, 2024
@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jan 29, 2024
@Uzlopak Uzlopak force-pushed the expose-websocket-by-default branch from 6858b5f to b9bdfc3 Compare January 29, 2024 13:23
mcollina
mcollina approved these changes Jan 29, 2024
View reviewed changes

@mcollina 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.

lgtm

@targos

targos commented Jan 31, 2024

Copy link
Copy Markdown
Member

It seems to break a lot of tests.

@Uzlopak

Uzlopak commented Jan 31, 2024

Copy link
Copy Markdown
Contributor Author

Let me investigate this

@JMTK JMTK mentioned this pull request Jan 31, 2024
Open
@Uzlopak Uzlopak force-pushed the expose-websocket-by-default branch from b9bdfc3 to 2722af1 Compare January 31, 2024 15:06
@Uzlopak

Uzlopak commented Jan 31, 2024

Copy link
Copy Markdown
Contributor Author

@targos
I checked it. It starts erroring when i set in node_options.h experimental_websocket to true

But I have no clue, why this results in async hooks issues.

@Uzlopak

Uzlopak commented Jan 31, 2024

Copy link
Copy Markdown
Contributor Author

I think I should also do thinks like in #51598

Uzlopak and others added 2 commits February 2, 2024 12:25
@Uzlopak @targos
lib: enable WebSocket by default
68b38a6
@targos
fixup! lib: enable WebSocket by default
3f16766 
do not load WebSocket in all tests

use global WebSocket for benchmark
@targos targos force-pushed the expose-websocket-by-default branch from 2722af1 to 3f16766 Compare February 2, 2024 11:48
@targos

targos commented Feb 2, 2024

Copy link
Copy Markdown
Member

@targos targos added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 2, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 2, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57018/

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 4, 2024
@nodejs-github-bot nodejs-github-bot merged commit c975384 into nodejs:main Feb 4, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 4, 2024

Copy link
Copy Markdown
Collaborator

Landed in c975384

@Uzlopak Uzlopak deleted the expose-websocket-by-default branch February 4, 2024 14:07
@RafaelGSS RafaelGSS mentioned this pull request Apr 12, 2024
Merged
@benjamingr benjamingr mentioned this pull request Apr 21, 2024
Closed
@regseb regseb mentioned this pull request Apr 25, 2024
Closed
@colinrotherham colinrotherham mentioned this pull request May 1, 2024
Closed
8 tasks
@bizob2828 bizob2828 mentioned this pull request Jul 9, 2024
Closed
@aspandey aspandey mentioned this pull request Nov 7, 2024
Merged
@ebadiere ebadiere mentioned this pull request Nov 11, 2024
Closed
8 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Apr 2, 2026
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@ShogunPanda ShogunPanda ShogunPanda approved these changes
@ronag ronag ronag approved these changes
@UlisesGascon UlisesGascon UlisesGascon approved these changes
@trivikr trivikr trivikr approved these changes
@KhafraDev KhafraDev KhafraDev approved these changes
@targos targos Awaiting requested review from targos

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@Uzlopak @targos @nodejs-github-bot @mcollina @ShogunPanda @ronag @UlisesGascon @trivikr @KhafraDev