Skip to content

Ensure that the native dgram node API does not introduce a breaking change#5729

Merged
petebacondarwin merged 3 commits intomainfrom
pbd/node/dgram
Dec 20, 2025
Merged

Ensure that the native dgram node API does not introduce a breaking change#5729
petebacondarwin merged 3 commits intomainfrom
pbd/node/dgram

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin commented Dec 19, 2025

The current implementation throws "not implemented" errors when constructing a Socket and when calling the Socket's methods, which is different to Node.js and the unenv polyfill. Now they are simple noops, inline with unenv.

This change will allow us to safely remove the node:dgram polyfill from the Cloudflare unenv preset.

For reference:

Node.js implementation
unenv implementation

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 19, 2025

CodSpeed Performance Report

Merging #5729 will not alter performance

Comparing pbd/node/dgram (9d617d8) with main (6409569)

Summary

✅ 57 untouched
⏩ 34 skipped1

Footnotes

  1. 34 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.

Copy link
Copy Markdown
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.

I much prefer failing noisily in general, but ok.

@petebacondarwin
Copy link
Copy Markdown
Contributor Author

I much prefer failing noisily in general, but ok.

Yeah, I hear you. But it would be frustrating if we let this loose and it breaks a bunch of applications for no real reason.

Thanks for the review!

@petebacondarwin petebacondarwin merged commit 0b9e22b into main Dec 20, 2025
32 of 34 checks passed
@petebacondarwin petebacondarwin deleted the pbd/node/dgram branch December 20, 2025 17:41
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.

4 participants