Skip to content

Conversation

@rokob
Copy link

@rokob rokob commented Feb 10, 2026

This change adds JS API support for custom compression dictionaries with Brotli in the zlib library. The underlying Brotli dependency already supports this and zstd exposes something similar. This follows the zstd approach for using a custom dictionary but for Brotli.

Fixes: #52250

Notes

I wanted to get the change up here first to get feedback and ensure the API makes sense. I can then update any relevant docs. The tests

This change adds JS API support for custom compression dictionaries
with Brotli in the zlib library. The underlying Brotli dependency
already supports this and zstd exposes something similar.
This follows the zstd approach for using a custom dictionary but
for Brotli.

Fixes: nodejs#52250
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Feb 10, 2026
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

nice!

@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 11, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 11, 2026
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 11, 2026
void Close();
void DoThreadPoolWork();
CompressionError Init();
CompressionError Init(std::string_view dictionary = {});
Copy link
Member

Choose a reason for hiding this comment

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

This didn't feel important enough to request an update, but since the linter is failing anyway – if this were

Suggested change
CompressionError Init(std::string_view dictionary = {});
CompressionError Init(std::vector<uint8_t>&& dictionary = {});

the ownership semantics would be a bit clearer (i.e. it becomes obvious that this class will take ownership of the argument, instead of the std::string_view case where that's less clear)

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 78.18182% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.72%. Comparing base (3819c7f) to head (2af550d).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 75.51% 6 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61763      +/-   ##
==========================================
- Coverage   89.74%   89.72%   -0.02%     
==========================================
  Files         675      675              
  Lines      204642   204690      +48     
  Branches    39322    39330       +8     
==========================================
+ Hits       183657   183668      +11     
- Misses      13257    13289      +32     
- Partials     7728     7733       +5     
Files with missing lines Coverage Δ
lib/zlib.js 98.27% <100.00%> (+<0.01%) ⬆️
src/node_zlib.cc 78.00% <75.51%> (-0.06%) ⬇️

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Please add latest brotli 1.1.0 bindings with compression dictionary support

4 participants