zlib: add support for brotli compression dictionary#61763
zlib: add support for brotli compression dictionary#61763nodejs-github-bot merged 3 commits intonodejs:mainfrom
Conversation
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61763 +/- ##
=======================================
Coverage 89.74% 89.74%
=======================================
Files 675 675
Lines 204642 204729 +87
Branches 39322 39333 +11
=======================================
+ Hits 183657 183744 +87
+ Misses 13257 13253 -4
- Partials 7728 7732 +4
🚀 New features to boost your workflow:
|
|
I updated the initializer to use an r-value ref and added some argument validation on the js side along with some tests. Linters all pass locally for me. |
|
I ran |
Commit Queue failed- Loading data for nodejs/node/pull/61763 ✔ Done loading data for nodejs/node/pull/61763 ----------------------------------- PR info ------------------------------------ Title zlib: add support for brotli compression dictionary (#61763) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch rokob:aw-52250-brotli-dict -> nodejs:main Labels c++, zlib, author ready, needs-ci Commits 3 - zlib: add support for brotli compression dictionary - Fix lint, convert to r-value ref instead of string_view, add tests - Actually fix the lint Committers 1 - Andy Weiss <andy.weiss@reddit.com> PR-URL: https://github.com/nodejs/node/pull/61763 Fixes: https://github.com/nodejs/node/issues/52250 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/61763 Fixes: https://github.com/nodejs/node/issues/52250 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> -------------------------------------------------------------------------------- ℹ This PR was created on Tue, 10 Feb 2026 15:54:25 GMT ✔ Approvals: 4 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/61763#pullrequestreview-3791926125 ✔ - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/61763#pullrequestreview-3784656650 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/61763#pullrequestreview-3788840436 ✔ - Gürgün Dayıoğlu (@gurgunday): https://github.com/nodejs/node/pull/61763#pullrequestreview-3793564199 ⚠ GitHub cannot link the author of 'zlib: add support for brotli compression dictionary' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ⚠ GitHub cannot link the author of 'Fix lint, convert to r-value ref instead of string_view, add tests' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ⚠ GitHub cannot link the author of 'Actually fix the lint' to their GitHub account. ⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-02-12T16:11:42Z: https://ci.nodejs.org/job/node-test-pull-request/71325/ - Querying data for job/node-test-pull-request/71325/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 61763 From https://github.com/nodejs/node * branch refs/pull/61763/merge -> FETCH_HEAD ✔ Fetched commits as 3d30c302c37f..f733e95c098a -------------------------------------------------------------------------------- Auto-merging lib/zlib.js [main 61638f9171] zlib: add support for brotli compression dictionary Author: Andy Weiss <andy.weiss@reddit.com> Date: Tue Feb 10 10:45:22 2026 -0500 3 files changed, 168 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-zlib-brotli-dictionary.js Auto-merging lib/zlib.js [main ca6e6c1767] Fix lint, convert to r-value ref instead of string_view, add tests Author: Andy Weiss <andy.weiss@reddit.com> Date: Wed Feb 11 11:46:32 2026 -0500 3 files changed, 84 insertions(+), 24 deletions(-) [main 3777af02ab] Actually fix the lint Author: Andy Weiss <andy.weiss@reddit.com> Date: Thu Feb 12 08:59:07 2026 -0500 1 file changed, 12 insertions(+), 11 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. (node:743) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/6) Executing: git node land --amend --yes ⚠ Found Fixes: https://github.com/nodejs/node/issues/52250, skipping.. --------------------------------- New Message ---------------------------------- zlib: add support for brotli compression dictionaryhttps://github.com/nodejs/node/actions/runs/21993102795 |
|
Landed in 8f613ee |
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