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.

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

Expose zlib constants directly under zlib module #2823

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

npaun
Copy link
Member

@npaun npaun commented Sep 27, 2024

This is supported in NodeJS but is deprecated.
Fixes #2805

@npaun npaun requested a review from anonrig September 27, 2024 16:49
@npaun npaun requested review from a team as code owners September 27, 2024 16:49
@npaun npaun requested a review from hoodmane September 27, 2024 16:49
@@ -100,12 +213,122 @@ export {
brotliDecompressSync,
brotliCompress,
brotliCompressSync,

// NodeJS also exports all constants directly under zlib, but this is deprecated
Copy link
Member

Choose a reason for hiding this comment

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

@jasnell afaik, there isn't any other alternative way to expose this, but any idea?

Copy link
Member

Choose a reason for hiding this comment

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

Well... you could move all of the constants here into a separate internal/zlib-constants import as named exports only... then do

import * as zlibConstants from 'node-internal:zlib-constants';

export const constants = { ...zlibConstants };
export * from 'node-internal:zlib-constants';

But might not be worth the extra hassle. I'd just go with this for now.

Copy link
Member

Choose a reason for hiding this comment

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

JavaScript is a weird language.

@npaun npaun merged commit b1656ad into main Sep 27, 2024
14 checks passed
@npaun npaun deleted the npaun/zlib-constants-pollution branch September 27, 2024 17:37
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.

🐛 Bug Report — Runtime APIs — node:zlib not exporting all necessary APIs via the default export
3 participants