Skip to content

zlib: add support for brotli compression dictionary#61763

Merged
nodejs-github-bot merged 3 commits intonodejs:mainfrom
rokob:aw-52250-brotli-dict
Feb 13, 2026
Merged

zlib: add support for brotli compression dictionary#61763
nodejs-github-bot merged 3 commits intonodejs:mainfrom
rokob:aw-52250-brotli-dict

Conversation

@rokob
Copy link
Contributor

@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
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 80.64516% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.74%. Comparing base (3819c7f) to head (f733e95).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 72.09% 6 Missing and 6 partials ⚠️
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     
Files with missing lines Coverage Δ
lib/zlib.js 98.30% <100.00%> (+0.02%) ⬆️
src/node_zlib.cc 77.79% <72.09%> (-0.27%) ⬇️

... and 42 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.

@rokob
Copy link
Contributor Author

rokob commented Feb 11, 2026

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.

@addaleax addaleax added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 12, 2026
@rokob
Copy link
Contributor Author

rokob commented Feb 12, 2026

I ran make format-cpp locally which passed which I realized looking at the actions was not setting the CLANG_FORMAT_START variable correctly. I set that locally which captured the issues that showed up in CI and fixed those. My bad.

@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 12, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 12, 2026
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 13, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 13, 2026
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 13, 2026
@nodejs-github-bot
Copy link
Collaborator

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 dictionary

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
PR-URL: #61763
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>

[detached HEAD 0b291ff59d] 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
Rebasing (3/6)
Rebasing (4/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Fix lint, convert to r-value ref instead of string_view, add tests

PR-URL: #61763
Fixes: #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>

[detached HEAD 5edccb22a4] 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(-)
Rebasing (5/6)
Rebasing (6/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Actually fix the lint

PR-URL: #61763
Fixes: #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>

[detached HEAD 12eff1e7c3] 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(-)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/21993102795

@addaleax addaleax added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 13, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 13, 2026
@nodejs-github-bot nodejs-github-bot merged commit 8f613ee into nodejs:main Feb 13, 2026
92 of 95 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 8f613ee

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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

6 participants