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.

Sign up for GitHub

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

Jump to bottom

add node:net module #3315

Merged
merged 7 commits into from
Jan 14, 2025
Merged

add node:net module #3315

merged 7 commits into from
Jan 14, 2025

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Jan 9, 2025 •
edited
Loading

Work in progress. Based on @jasnell's pull-request #2217 with couple of changes/additions

TODO

  • Write a bazel rule for testing TCP servers through Node.js
  • Add tests

@anonrig anonrig requested a review from jasnell January 9, 2025 22:12
@anonrig anonrig requested review from a team as code owners January 9, 2025 22:12
@anonrig anonrig requested a review from dom96 January 9, 2025 22:12
@anonrig anonrig force-pushed the yagiz/add-node-net branch from 4f60ae6 to 519fd7f Compare January 9, 2025 22:14
@jasnell
Copy link
Member

jasnell commented Jan 9, 2025

Write a bazel rule for testing TCP servers through Node.js

So that it is absolutely clear... the original take on this PR relied on implementing TCP-ingress within workerd to allow workerd itself to act as a test server. We're backing off on that due to disagreements over the design of it and instead will be looking at having the bazel test start/stop a node.js-based test server to use for testing.

@jasnell
Copy link
Member

jasnell commented Jan 9, 2025

Add tests

The tests my original PR had are fairly comprehensive and should be relatively easy to port over with the new node.js-based test server approach.

@bradzacher bradzacher mentioned this pull request Jan 10, 2025
Closed
4 tasks
@anonrig anonrig force-pushed the yagiz/add-node-net branch 3 times, most recently from 3345607 to 464a0a0 Compare January 10, 2025 20:49
@anonrig anonrig force-pushed the yagiz/add-node-net branch 7 times, most recently from cb624a0 to 9380b4c Compare January 13, 2025 15:55
@anonrig anonrig requested a review from jasnell January 13, 2025 15:59
@jasnell

This comment was marked as resolved.

Sign in to view
@anonrig anonrig force-pushed the yagiz/add-node-net branch 3 times, most recently from 6b20d21 to c87e32c Compare January 14, 2025 17:13
@anonrig anonrig force-pushed the yagiz/add-node-net branch from 46ede10 to cd566f5 Compare January 14, 2025 19:35
@anonrig anonrig force-pushed the yagiz/add-node-net branch from cd566f5 to 010175a Compare January 14, 2025 20:59
@anonrig anonrig merged commit 12dff19 into main Jan 14, 2025
15 checks passed
@anonrig anonrig deleted the yagiz/add-node-net branch January 14, 2025 21:33
@thomasgauvin thomasgauvin mentioned this pull request Jan 16, 2025
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@dom96 dom96 Awaiting requested review from dom96 dom96 is a code owner automatically assigned from cloudflare/workers-runtime-1

Assignees
No one assigned
Labels
api nodejs compat socket
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anonrig @jasnell