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

fix(node/zlib): consistently return buffer #21747

Merged
merged 1 commit into from
Dec 31, 2023

Conversation

JoviDeCroock
Copy link
Contributor

This fixes point 3 of #20516

This PR creates consistency between the sync and async versions of the brotli compress where we will always return a buffer like Node.

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@littledivy littledivy merged commit 4339a6c into denoland:main Dec 31, 2023
14 checks passed
littledivy pushed a commit that referenced this pull request Jan 3, 2024
Fixes #20516 
Follow up to #21747 and #21746

This tackles the last point of #20516 where certain inputs weren't
accepted in the other zlib methods

This adds the `toU8` conversion of `_brotli` to `_zlib.mjs`, when we
create the ZLibBuffer, we'll sanitize the input. I noticed that the
async had no handler for `string` input so I added that as well.
bartlomieju pushed a commit that referenced this pull request Jan 4, 2024
This fixes point 3 of #20516

This PR creates consistency between the sync and async versions of the
brotli compress where we will always return a buffer like Node.
bartlomieju pushed a commit that referenced this pull request Jan 4, 2024
Fixes #20516 
Follow up to #21747 and #21746

This tackles the last point of #20516 where certain inputs weren't
accepted in the other zlib methods

This adds the `toU8` conversion of `_brotli` to `_zlib.mjs`, when we
create the ZLibBuffer, we'll sanitize the input. I noticed that the
async had no handler for `string` input so I added that as well.
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