Skip to content

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Jul 3, 2025

This pull-request enables the following modules and functions behind a compat flag

  • node:_http_agent
    • Agent
    • globalAgent
  • node:_http_client
    • ClientRequest
  • node:_http_common
    • _checkIsHttpToken
    • _checkInvalidHeaderChar
    • chunkExpression
    • continueExpression
    • methods
    • kIncomingMessage
  • node:_http_incoming
    • IncomingMessage
  • node:_http_outgoing
    • validateHeaderName
    • validateHeaderValue
    • kUniqueHeaders
    • kHighWaterMark
    • parseUniqueHeadersOption
    • OutgoingMessage
  • node:http
    • validateHeaderName
    • validateHeaderValue
    • METHODS
    • STATUS_CODES
    • ClientRequest
    • request
    • get
    • WebSocket
    • OutgoingMessage
    • Agent
    • globalAgent
    • IncomingMessage
  • node:https
    • get
    • request
    • Agent
    • globalAgent

cc @vicb

@anonrig anonrig requested review from jasnell, guybedford and npaun July 3, 2025 18:43
@anonrig anonrig requested review from a team as code owners July 3, 2025 18:43
@vicb
Copy link
Contributor

vicb commented Jul 3, 2025

Thanks for the listing the changes, it will help syncing the unenv preset

A few questions first:

  • what are the timeline to merge this PR?
  • The change was merged 2 days ago, how has it been validated since then ? (i.e. why is now a good time to remove the exp flag, did it reach prod already?)
  • Has a doc PR/issue been created to update https://developers.cloudflare.com/workers/runtime-apis/nodejs/?
  • When I asked earlier about Agent support there were some limitations, have those been lifted or still apply? If they still apply it would be nice to add them to the PR description - as well as any other diff with the Node API.

But then, I'd like to thank you for the implementation. It has been requested many times by users and it should really improve the compatibility with Node 🙏

I'll work on updating the unenv preset as soon as this is released.

@jasnell
Copy link
Collaborator

jasnell commented Jul 3, 2025

what are the timeline to merge this PR?

I think we can merge this any time really. The big question is when should we unflag in production.

The #4205 was merged 2 days ago, how has it been validated since then ? (i.e. why is now a good time to remove the exp flag, did it reach prod already?)

Before we flip the switch to make this generally available in production, we should have some internal teams do some experimentation to help validate this. I don't think we've done so yet.

Has a doc PR/issue been created to update developers.cloudflare.com/workers/runtime-apis/nodejs?

I've not yet seen a documentation PR opened. We'll definitely need one... https://github.com/cloudflare/cloudflare-docs/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Aopen+%22node%3Ahttp%22+

When I asked earlier about Agent support there were some limitations, have those been lifted or still apply? If they still apply it would be nice to add them to the PR description - as well as any other diff with the Node API.

Pretty sure the restrictions are still there. We're not really implementing Agent. What is there is a largely non-functional stub. We should absolutely make sure these make it into the docs.

Personally, I'd like to at least see the documentation PR opened before we merge this.

This comment was marked as resolved.

@anonrig
Copy link
Member Author

anonrig commented Jul 7, 2025

Opened a PR for docs @jasnell @vicb #4456

cloudflare/cloudflare-docs#23481

Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM but I'd prefer to hold off on merging this until after it hits production and internal teams have a chance to experiment with it a bit.

@vicb

This comment was marked as resolved.

@anonrig anonrig force-pushed the yagiz/remove-experimental branch from 70bea86 to 08742cb Compare July 15, 2025 17:10
@anonrig anonrig changed the title remove experimental flag from http and https modules put http and https modules behind compat flag Jul 15, 2025
@anonrig anonrig force-pushed the yagiz/remove-experimental branch from 08742cb to 8afb38f Compare July 15, 2025 17:12
@anonrig anonrig merged commit c931b08 into main Jul 15, 2025
20 of 21 checks passed
@anonrig anonrig deleted the yagiz/remove-experimental branch July 15, 2025 20:20
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

Successfully merging this pull request may close these issues.

5 participants