Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Oct 13, 2025

Updates Node.js streams implementation to latest while making it more type-safe.

@anonrig anonrig requested a review from jasnell October 13, 2025 19:44
@anonrig anonrig requested review from a team as code owners October 13, 2025 19:44
@github-actions
Copy link

github-actions bot commented Oct 13, 2025

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@anonrig anonrig force-pushed the yagiz/update-streams-impl branch 10 times, most recently from 030c4bc to 354d70f Compare October 14, 2025 19:31
@anonrig anonrig requested review from guybedford and npaun October 14, 2025 19:34
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.

Overall the changes look fine except for the fact that this likely needs to be gated with a compat flag and the original behavior preserved, unfortuntely...

To expand a bit: I am not comfortable approving this as it is. There are definitely a number of commits that are semver-major changes in node.js included and given the way this PR was structured (all in one commit where large chunks are moved around and edited without clear diffs of what specifically was changed).. it's not really possible to determine what possible breaking changes are included.

This change is definitely needed, but the PR needs to be restructured to make it easier to review and likely needs to be gated behind a compat flag that preserves the original behavior.

@anonrig anonrig force-pushed the yagiz/update-streams-impl branch from 354d70f to d35af9a Compare October 17, 2025 19:20
@anonrig anonrig requested a review from jasnell October 17, 2025 19:39
@anonrig anonrig force-pushed the yagiz/update-streams-impl branch 10 times, most recently from 99fe4c8 to a8bb8ad Compare October 22, 2025 19:22
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 22, 2025

CodSpeed Performance Report

Merging #5298 will not alter performance

Comparing yagiz/update-streams-impl (4d384ee) with main (48cfcb1)

Summary

✅ 33 untouched
⏩ 9 skipped1

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@anonrig anonrig requested a review from jasnell October 24, 2025 19:46
@anonrig anonrig force-pushed the yagiz/update-streams-impl branch from a8bb8ad to 4d384ee Compare October 24, 2025 23:51
@anonrig
Copy link
Member Author

anonrig commented Oct 24, 2025

rebasing

@anonrig anonrig enabled auto-merge (rebase) October 24, 2025 23:52
@anonrig anonrig merged commit b6ceaf5 into main Oct 25, 2025
21 checks passed
@anonrig anonrig deleted the yagiz/update-streams-impl branch October 25, 2025 00:24
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.

2 participants