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

[Idea]: allow setting the compression level #84

Closed
matteosacchetto opened this issue Oct 29, 2024 · 4 comments · Fixed by #85
Closed

[Idea]: allow setting the compression level #84

matteosacchetto opened this issue Oct 29, 2024 · 4 comments · Fixed by #85

Comments

@matteosacchetto
Copy link

I was investigating the code of this library, and I've seen that it takes advantage of node:zlib's deflateRaw class/functions.

Given that zlib's compression methods typically accept a parameter which is the compression level (between 1 and 9, defaults to 6), I was wondering if it could be interesting/worth exposing that parameter. It would allow a user of the library, to tune the compression level based on time/disk space.

To implement it, I think we could do one of the following:

  • entry.compress: instead of restricting it to be a boolean, we could allow it to be a boolean | number, where if it is a false or 0, no compression, if it is true, default compression level, otherwise the level is set based on entry.compress
  • entry.level: have an additional parameter which specifies the level

I do not know how many users could be interested in this kind of feature, so I am leaving it here just as an idea, hoping to get some feedback from library users

@thejoshwolfe
Copy link
Owner

great idea! easy to support.

@thejoshwolfe
Copy link
Owner

published in version 3.2.0

@matteosacchetto
Copy link
Author

Great to hear that!
I was checking the PR and I have a quick question

yazl/index.js

Lines 97 to 99 in dc85d0b

zlib.deflateRaw(buffer, {level:1}, function(err, compressedBuffer) {
setCompressedBuffer(compressedBuffer);
});

In the addBuffer method (the lines highlighted above), I see that the level is set to 1. Shouldn't it be entry.compressionLevel? Or is there a reason for the fixed value?

Thank you in advance!

@thejoshwolfe
Copy link
Owner

Good catch! that's a typo. oops. I even have a test that makes sure the compression level is passed to zlib, but i only tested it for addFile() 🤦

fix published in 3.2.1.

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 a pull request may close this issue.

2 participants