-
Notifications
You must be signed in to change notification settings - Fork 301
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.
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
🐛 Bug Report — Runtime APIs — node:zlib not exporting all necessary APIs via the default export #2805
Comments
We should definitely add them. Just FYI: We are exporting under constants and codes. Exporting each constant in default is considered deprecated, and not commonly used. Here's the Node.js reference: https://github.com/nodejs/node/blob/main/lib/zlib.js#L926 |
@anonrig thanks for the link! And thanks for looking in this. Our current stance has been to mirror the union of node v18, v20, and v22 APIs, so if Node removes these symbols in the future and they fall out of TLS version train, we should remove them as well. In the meantime we should just mirror whatever node.js exposes and not try to improve or correct for the actual API surface even if it's not ideal. We don't control the node.js API surface, we should just mirror it to the best of our ability and if any cleanup is necessary, we can influence the change upstream and wait for it to trickle down into our compat layer indirectly. |
Removing them isn't that easy due to backwards compat guarantees. It would need to be removed with a compat flag. |
I agree. The point I'm trying to make is that I think it's futile for us to decide what the node.js api surface should be, instead we should just mirror in workerd what the node.js api is today, even if there are things we don't like about it. if we start curating the node.js api surface in workerd, we would just be intentionally introducing incompatibilities, which is contrary to our goal of making |
The latest update of https://workers-nodejs-compat-matrix.pages.dev/ surfaced that we missed exposing some of the newly added
node:zlib
symbols via the default export:There is 44 symbols missing, they are listed in this range: https://github.com/IgorMinar/workers-nodejs-compat-matrix/blob/325bbf17a9cf432f185a37aa915b6f0457e4cc9b/data/workerd.json#L11300-L11514 and marked as
"missing"
.Can we correctly export these? Thank you
// @jasnell @anonrig
The text was updated successfully, but these errors were encountered: