Skip to content

Conversation

JamieMagee
Copy link
Contributor

This adds support for zstd compressed archives.

This change is dependent on isaacs/minizlib#35

Closes #438

@isaacs isaacs mentioned this pull request Sep 21, 2025
@isaacs
Copy link
Owner

isaacs commented Sep 21, 2025

Main has the minizlib that has zstd support, wanna rebase and see if this works?

Posted one little nit, but otherwise this looks great! I'll review more closely and get it landed once it's rebased on main and loading minizlib the normal way.

@JamieMagee JamieMagee marked this pull request as ready for review September 21, 2025 21:56
@JamieMagee
Copy link
Contributor Author

Main has the minizlib that has zstd support, wanna rebase and see if this works?

Everything works locally for me on Node 24

Details
$ npm run test

> [email protected] pretest
> npm run prepare


> [email protected] prepare
> tshy


> [email protected] test
> tap

 PASS  test/large-numbers.js 36 OK 3.300s
 PASS  test/cwd-error.js 1 OK 3.382s
 PASS  test/mode-fix.js 11 OK 3.251s
 PASS  test/header.js 39 OK 3.505s
 PASS  test/load-all.js 1 OK 3.709s
 PASS  test/map.js 5 OK 3.715s
 PASS  test/normalize-windows-path.js 4 OK 3.682s
 PASS  test/options.js 24 OK 3.682s
 PASS  test/index.js 5 OK 3.940s
 PASS  test/pack.js 681 OK 3.810s
 PASS  test/path-reservations.js 39 OK 6.860s
 PASS  test/make-command.ts 38 OK 10s
 PASS  test/pax.js 9 OK 7.011s
 PASS  test/list.ts 26 OK 10s
 PASS  test/read-entry.js 18 OK 6.907s
 PASS  test/strip-absolute-path.js 18 OK 7.088s
 PASS  test/symlink-error.js 1 OK 7.084s
 PASS  test/types.js 6 OK 7.046s
 PASS  test/strip-trailing-slashes.js 3 OK 7.201s
 PASS  test/get-write-flag.js 6 OK 11s
 PASS  test/warn-method.js 8 OK 1.596s
 PASS  test/winchars.js 3 OK 1.494s
 PASS  test/update.js 83 OK 2.081s
 PASS  test/write-entry.js 296 OK 1.932s
 PASS  test/replace.ts 104 OK 9.251s
 PASS  test/unpack.js 592 OK 9.742s
 PASS  test/writable-assignment-check.ts 1 OK 3.345s
 PASS  test/extract.ts 70 OK 14s
 PASS  test/create.ts 60 OK 16s
 PASS  test/normalize-unicode.js 65 OK 17s
 PASS  test/parse.js 18780 OK 24s


  🌈 TEST COMPLETE 🌈


Asserts:  21033 pass  0 fail  21033 of 21033 complete
Suites:      31 pass  0 fail        31 of 31 complete

# { total: 21033, pass: 21033 }
# time=32239.257ms

Posted one little nit, but otherwise this looks great! I'll review more closely and get it landed once it's rebased on main and loading minizlib the normal way.

I addressed your comment about pre-calculating ZIP_HEADER_LEN.

Would you like me to add a guard for versions of Node that don't have support for zstd?

@isaacs
Copy link
Owner

isaacs commented Sep 23, 2025

Ah, right, obviously this wont' work on earlier node versions.

Don't sweat the green CI then, I'll add a workaround in the tests so it skips it for older versions.

EDIT(@isaacs): Remove node 20 from ci workflow, since Zstd is not
available until node 22. The failure mode is handled in minizlib anyway.

PR-URL: isaacs#439
Credit: @JamieMagee
Close: isaacs#439
Reviewed-by: @isaacs
@isaacs
Copy link
Owner

isaacs commented Sep 23, 2025

Landed and published on 7.5.0. Thanks for all the work on making this happen!

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.

Add support for zstd

2 participants