feat: add XZ compression and decompression support #115
Conversation
- Add XZ compression module with FileStream and UncompressStream classes - Implement compressFile() and uncompress() functions - Add TypeScript definitions for XZ module - Add comprehensive unit tests for all XZ functionality - Support compression levels 1-9 and multi-threading options - Fixes #96
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an xz namespace and implementation: typings, index export, lib/xz implementation (FileStream, UncompressStream, helpers), lzma-native dependency, and tests for xz compression/decompression across file/buffer/stream sources. Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant xz as xz (lib/xz)
participant utils
participant FS as FileStream
participant lzma as lzma-native
Caller->>xz: compressFile(source, dest, opts)
xz->>utils: makeFileProcessFn(FileStream)
utils-->>xz: wrapper fn
Caller->>FS: new FileStream(opts)
FS->>lzma: instantiate Compressor
FS->>FS: detect source (file|buffer|stream) and pipe into compressor
lzma-->>Caller: compressed data to dest
sequenceDiagram
actor Caller
participant xz as xz (lib/xz)
participant utils
participant US as UncompressStream
participant lzma as lzma-native
Caller->>xz: uncompress(source, dest, opts)
xz->>utils: makeFileProcessFn(UncompressStream)
utils-->>xz: wrapper fn
Caller->>US: new UncompressStream(opts)
US->>lzma: instantiate Decompressor
US->>US: detect source (file|buffer|stream) and pipe into decompressor
lzma-->>Caller: decompressed data to dest
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces comprehensive support for XZ compression and decompression within the compressing library. It adds new modules, classes, and functions to handle XZ archives, allowing users to compress and uncompress files, buffers, and streams using the XZ format. This enhancement expands the library's capabilities by integrating a highly efficient compression algorithm.
Highlights
- New XZ Module: Added a new xz module to the library, providing functionalities for XZ compression and decompression.
- Stream Classes: Introduced FileStream and UncompressStream classes within the xz module for stream-based compression and decompression operations.
- Utility Functions: Implemented compressFile(), uncompress(), and decompress() utility functions for convenient file and data handling with XZ.
- TypeScript Definitions: Included TypeScript definitions for all new XZ functionalities, ensuring type safety and better developer experience.
- New Dependency: Integrated the lzma-native library as a core dependency to power the XZ compression and decompression.
- Comprehensive Testing: Developed comprehensive unit tests for all XZ features, covering various input types and error handling scenarios to ensure robustness.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces XZ compression and decompression support, which is a great addition to the library. The implementation is well-structured and includes new stream classes, utility functions, TypeScript definitions, and comprehensive tests. My review focuses on improving type safety in the TypeScript definitions, reducing code duplication, and addressing a few issues in the unit tests to ensure their correctness and maintainability. Overall, this is a solid contribution.
test/xz/file_stream.test.js
Outdated
| const xzStream = new compressing.xz.FileStream({ | ||
| source: sourceFile, | ||
| level: 6 | ||
| }); |
There was a problem hiding this comment.
The XzFileStream implementation uses the preset option for setting the compression level, but this test is passing level. This means the option is being ignored, and the test is not correctly verifying the functionality for custom compression levels. Please use the preset option instead. It's also a good practice to use a non-default value (the default is 6) to ensure the option is effective.
const xzStream = new compressing.xz.FileStream({
source: sourceFile,
preset: 9
});| const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log'); | ||
| const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz'); | ||
| const fileStream = fs.createWriteStream(destFile); | ||
| await compressing.xz.compressFile(sourceFile, fileStream, { level: 9 }); |
There was a problem hiding this comment.
The compressFile function passes options to XzFileStream, which expects preset for the compression level, not level. This test is not correctly checking the custom compression level functionality. Please change level to preset to fix the test.
| await compressing.xz.compressFile(sourceFile, fileStream, { level: 9 }); | |
| await compressing.xz.compressFile(sourceFile, fileStream, { preset: 9 }); |
| function compressFile(source: sourceType, dest: destType, opts?: any): Promise<void> | ||
|
|
||
| function uncompress(source: sourceType, dest: destType, opts?: any): Promise<void> | ||
|
|
||
| function decompress(source: sourceType, dest: destType, opts?: any): Promise<void> |
There was a problem hiding this comment.
For better type safety, the opts parameter for these functions should be more specific than any. compressFile accepts preset and threads options, while uncompress and decompress don't have specific options. Please consider defining the types for these options inline to improve developer experience and prevent potential bugs.
| function compressFile(source: sourceType, dest: destType, opts?: any): Promise<void> | |
| function uncompress(source: sourceType, dest: destType, opts?: any): Promise<void> | |
| function decompress(source: sourceType, dest: destType, opts?: any): Promise<void> | |
| function compressFile(source: sourceType, dest: destType, opts?: { preset?: number, threads?: number }): Promise<void> | |
| function uncompress(source: sourceType, dest: destType, opts?: {}): Promise<void> | |
| function decompress(source: sourceType, dest: destType, opts?: {}): Promise<void> |
| const sourceType = utils.sourceType(opts.source); | ||
|
|
||
| if (sourceType === 'file') { | ||
| const stream = fs.createReadStream(opts.source, opts.fs); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'buffer') { | ||
| const stream = streamifier.createReadStream(opts.source, opts.streamifier); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'stream') { | ||
| opts.source.on('error', err => this.emit('error', err)); | ||
| opts.source.pipe(this); | ||
| } |
There was a problem hiding this comment.
The logic for handling different source types (file, buffer, stream) is nearly identical to the logic in lib/xz/uncompress_stream.js. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a shared utility function. This would centralize the source handling logic, making future modifications easier and less error-prone.
| const sourceType = utils.sourceType(opts.source); | ||
|
|
||
| if (sourceType === 'file') { | ||
| const stream = fs.createReadStream(opts.source, opts.fs); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'buffer') { | ||
| const stream = streamifier.createReadStream(opts.source, opts.streamifier); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'stream') { | ||
| opts.source.on('error', err => this.emit('error', err)); | ||
| opts.source.pipe(this); | ||
| } |
There was a problem hiding this comment.
This logic for handling different source types is duplicated from lib/xz/file_stream.js. As mentioned in the comment for that file, this should be refactored into a shared utility function to reduce code duplication and improve maintainability.
| it('should be a transform stream', done => { | ||
| const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log'); | ||
| const sourceStream = fs.createReadStream(sourceFile); | ||
| const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz'); | ||
| // console.log('destFile', destFile); | ||
| const xzStream = new compressing.xz.FileStream(); | ||
| const destStream = fs.createWriteStream(destFile); | ||
| pump(sourceStream, xzStream, destStream, err => { | ||
| assert(!err); | ||
| assert(fs.existsSync(destFile)); | ||
| done(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test uses the done callback, while other tests in the suite use async/await. For consistency and improved readability, it's better to use async/await throughout the test suite. You can promisify the stream pipeline to achieve this.
it('should be a transform stream', async () => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const sourceStream = fs.createReadStream(sourceFile);
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
// console.log('destFile', destFile);
const xzStream = new compressing.xz.FileStream();
const destStream = fs.createWriteStream(destFile);
await new Promise((resolve, reject) => {
pump(sourceStream, xzStream, destStream, err => {
if (err) return reject(err);
resolve();
});
});
assert(fs.existsSync(destFile));
});| it('xz.compressFile(file, stream)', async () => { | ||
| const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log'); | ||
| const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz'); | ||
| // console.log('destFile', destFile); |
There was a problem hiding this comment.
This commented-out console.log should be removed. There are other instances in this file (e.g., lines 24, 41, 51) that should also be removed to keep the test file clean.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (13)
package.json (1)
46-46: Adding lzma-native: verify prebuild availability across CI platformslzma-native ships native bindings. Please confirm prebuilt binaries exist for Node 18 on Linux/macOS/Windows to avoid build-from-source instability in CI and downstream users.
If prebuilds are incomplete, consider:
- documenting the build toolchain requirements
- or gating as an optional feature with graceful error when unavailable.
index.d.ts (1)
64-72: DX consistency: acceptlevelalias in FileStream optionsOther modules (e.g., gzip) commonly use
level. Tests also pass{ level: 9 }. Typings only mentionpreset. Add alevel?: numberalias for smoother UX.Apply this diff:
export class FileStream extends ReadStream { constructor(opts?: { - preset?: number, + preset?: number, + level?: number, threads?: number, source?: sourceType });test/xz/index.test.js (2)
68-68: Remove trailing whitespace to satisfy lintESLint flags trailing spaces on Line 68.
Apply this diff:
- +
151-151: Add newline at EOF to satisfy lintCI is failing with “Newline required at end of file but not found”.
Apply this diff:
-}); +}); +lib/xz/file_stream.js (1)
42-42: Add newline at EOF to satisfy lintESLint/CI reports “Newline required at end of file but not found”.
Apply this diff:
-module.exports = XzFileStream; +module.exports = XzFileStream; +lib/xz/uncompress_stream.js (1)
11-11: Optionally pass through lzma-native Decompressor optionsIf we anticipate exposing lzma-native options (e.g., memlimit, threads), accept them under opts.lzma and pass to super. Keeps stream plumbing options separate from decompressor options.
- super(); + // Allow passing lzma-native Decompressor options via opts.lzma + super(opts.lzma);test/xz/file_stream.test.js (2)
94-101: Typo in test description: “exit” → “exist”Updates the description for clarity and correctness.
- it('should emit error if sourceFile does not exit', done => { + it('should emit error if sourceFile does not exist', done => {
103-111: Grammar: subject-verb agreement in test descriptionUse “emits” for readability and consistency.
- it('should emit error if sourceStream emit error', done => { + it('should emit error if sourceStream emits error', done => {test/xz/uncompress_stream.test.js (5)
40-40: Grammar: add article “a”Minor readability improvement in test title.
- it('should be transform stream', done => { + it('should be a transform stream', done => {
116-123: Typo in test description: “exit” → “exist”Clarify the description.
- it('should emit error if sourceFile does not exit', done => { + it('should emit error if sourceFile does not exist', done => {
125-133: Grammar: subject-verb agreementUse “emits” for readability and consistency.
- it('should emit error if sourceStream emit error', done => { + it('should emit error if sourceStream emits error', done => {
135-149: Grammar: subject-verb agreementSame adjustment for this description.
- it('should emit error if streamifier.createReadStream emit error', done => { + it('should emit error if streamifier.createReadStream emits error', done => {
31-36: Optional: clean up the generated .xz file after testscreateXzFile writes to os.tmpdir(). Consider removing the file in an after() hook to avoid test artifacts piling up.
Add this after the suite finishes:
// Add at the bottom of the describe(), after tests after(async () => { try { if (sourceFile) await fs.promises.unlink(sourceFile); } catch (_) { // ignore } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
index.d.ts(1 hunks)index.js(1 hunks)lib/xz/file_stream.js(1 hunks)lib/xz/index.js(1 hunks)lib/xz/uncompress_stream.js(1 hunks)package.json(1 hunks)test/xz/file_stream.test.js(1 hunks)test/xz/index.test.js(1 hunks)test/xz/uncompress_stream.test.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
lib/xz/uncompress_stream.js (1)
lib/xz/index.js (2)
utils(3-3)XzUncompressStream(5-5)
test/xz/uncompress_stream.test.js (4)
lib/xz/file_stream.js (2)
fs(3-3)streamifier(6-6)lib/xz/uncompress_stream.js (2)
fs(3-3)streamifier(6-6)test/xz/file_stream.test.js (7)
fs(1-1)os(2-2)uuid(4-4)path(3-3)assert(7-7)pump(5-5)compressing(6-6)test/xz/index.test.js (7)
fs(3-3)os(4-4)uuid(6-6)path(5-5)assert(8-8)compressing(7-7)isWindows(9-9)
lib/xz/index.js (2)
lib/xz/file_stream.js (1)
utils(5-5)lib/xz/uncompress_stream.js (1)
utils(5-5)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (windows-latest, 22)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (macos-latest, 24)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (macos-latest, 18)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (macos-latest, 20)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (windows-latest, 24)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (windows-latest, 20)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (windows-latest, 18)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 24)
test/xz/index.test.js
[failure] 151-151:
Newline required at end of file but not found
[failure] 68-68:
Trailing spaces not allowed
lib/xz/uncompress_stream.js
[failure] 38-38:
Newline required at end of file but not found
test/xz/file_stream.test.js
[failure] 113-113:
Newline required at end of file but not found
[failure] 84-84:
Missing trailing comma
[failure] 82-82:
Trailing spaces not allowed
test/xz/uncompress_stream.test.js
[failure] 18-18:
Trailing spaces not allowed
lib/xz/file_stream.js
[failure] 42-42:
Newline required at end of file but not found
[failure] 13-13:
Missing trailing comma
lib/xz/index.js
[failure] 11-11:
Newline required at end of file but not found
🪛 GitHub Actions: CI
lib/xz/file_stream.js
[error] 13-13: ESLint: Missing trailing comma. (comma-dangle)
🔇 Additional comments (2)
index.js (1)
7-7: LGTM: publicxzexportTop-level export is correctly wired to
./lib/xz, aligning with the new namespace and tests.lib/xz/index.js (1)
7-11: Exports look good and API surface is cohesiveNice, consistent surface: FileStream/UncompressStream plus compressFile/uncompress/decompress wrappers via utils.makeFileProcessFn. This aligns with existing modules and provides both uncompress/decompress aliases.
| const lzmaOpts = { | ||
| preset: opts.preset || 6, | ||
| threads: opts.threads || 0 | ||
| }; | ||
| super(lzmaOpts); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Support level alias and fix trailing comma (CI lint failure)
- Map
opts.leveltolzmapreset for API consistency. - Add trailing comma to satisfy
comma-danglerule.
Apply this diff:
- const lzmaOpts = {
- preset: opts.preset || 6,
- threads: opts.threads || 0
- };
+ const preset = opts.preset != null
+ ? opts.preset
+ : (opts.level != null ? opts.level : 6);
+ const lzmaOpts = {
+ preset,
+ threads: opts.threads || 0,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lzmaOpts = { | |
| preset: opts.preset || 6, | |
| threads: opts.threads || 0 | |
| }; | |
| super(lzmaOpts); | |
| const preset = opts.preset != null | |
| ? opts.preset | |
| : (opts.level != null ? opts.level : 6); | |
| const lzmaOpts = { | |
| preset, | |
| threads: opts.threads || 0, | |
| }; | |
| super(lzmaOpts); |
🧰 Tools
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (windows-latest, 22)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (macos-latest, 24)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (macos-latest, 18)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (macos-latest, 20)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (windows-latest, 24)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (windows-latest, 20)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (windows-latest, 18)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 24)
[failure] 13-13:
Missing trailing comma
🪛 GitHub Actions: CI
[error] 13-13: ESLint: Missing trailing comma. (comma-dangle)
🤖 Prompt for AI Agents
In lib/xz/file_stream.js around lines 11 to 15, the lzma options object should
accept the legacy alias opts.level and must include a trailing comma to satisfy
the comma-dangle lint rule; change the preset assignment to prefer opts.preset,
then opts.level, then 6 as the default (e.g. preset: opts.preset ?? opts.level
?? 6), keep threads defaulting to opts.threads || 0, and add a trailing comma
after the threads property so the object ends with a comma before calling
super(lzmaOpts).
| if (sourceType === 'file') { | ||
| const stream = fs.createReadStream(opts.source, opts.fs); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'buffer') { | ||
| const stream = streamifier.createReadStream(opts.source, opts.streamifier); | ||
| stream.on('error', err => this.emit('error', err)); | ||
| stream.pipe(this); | ||
| return; | ||
| } | ||
|
|
||
| if (sourceType === 'stream') { | ||
| opts.source.on('error', err => this.emit('error', err)); | ||
| opts.source.pipe(this); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer destroying on upstream errors to ensure teardown and proper pipeline propagation
Forwarding errors with this.emit('error', err) works, but using this.destroy(err) tears down the decompressor and guarantees pipeline() propagates the error cleanly and promptly. Apply across all three source types for consistency.
if (sourceType === 'file') {
const stream = fs.createReadStream(opts.source, opts.fs);
- stream.on('error', err => this.emit('error', err));
+ stream.on('error', err => this.destroy(err));
stream.pipe(this);
return;
}
if (sourceType === 'buffer') {
const stream = streamifier.createReadStream(opts.source, opts.streamifier);
- stream.on('error', err => this.emit('error', err));
+ stream.on('error', err => this.destroy(err));
stream.pipe(this);
return;
}
if (sourceType === 'stream') {
- opts.source.on('error', err => this.emit('error', err));
+ opts.source.on('error', err => this.destroy(err));
opts.source.pipe(this);
+ return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (sourceType === 'file') { | |
| const stream = fs.createReadStream(opts.source, opts.fs); | |
| stream.on('error', err => this.emit('error', err)); | |
| stream.pipe(this); | |
| return; | |
| } | |
| if (sourceType === 'buffer') { | |
| const stream = streamifier.createReadStream(opts.source, opts.streamifier); | |
| stream.on('error', err => this.emit('error', err)); | |
| stream.pipe(this); | |
| return; | |
| } | |
| if (sourceType === 'stream') { | |
| opts.source.on('error', err => this.emit('error', err)); | |
| opts.source.pipe(this); | |
| } | |
| if (sourceType === 'file') { | |
| const stream = fs.createReadStream(opts.source, opts.fs); | |
| stream.on('error', err => this.destroy(err)); | |
| stream.pipe(this); | |
| return; | |
| } | |
| if (sourceType === 'buffer') { | |
| const stream = streamifier.createReadStream(opts.source, opts.streamifier); | |
| stream.on('error', err => this.destroy(err)); | |
| stream.pipe(this); | |
| return; | |
| } | |
| if (sourceType === 'stream') { | |
| opts.source.on('error', err => this.destroy(err)); | |
| opts.source.pipe(this); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In lib/xz/uncompress_stream.js around lines 15 to 32, the upstream error
handlers call this.emit('error', err) which only forwards the error; change each
handler to call this.destroy(err) so the decompressor is torn down and the error
is propagated cleanly through pipeline(): replace stream.on('error', err =>
this.emit('error', err)) and opts.source.on('error', err => this.emit('error',
err)) with handlers that invoke this.destroy(err) (ensuring the handler context
is the decompressor) for the file, buffer and stream source branches to keep
behavior consistent.
- Remove trailing spaces - Add missing end-of-line characters - Fix array bracket spacing issues
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #115 +/- ##
==========================================
+ Coverage 97.99% 98.15% +0.16%
==========================================
Files 19 22 +3
Lines 1048 1140 +92
Branches 277 301 +24
==========================================
+ Hits 1027 1119 +92
Misses 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/xz/file_stream.js (2)
11-14: Fix preset handling: allow preset=0 and supportopts.levelalias (also satisfies lint).Using
opts.preset || 6prevents consumers from usingpreset: 0(valid LZMA level) and ignores the legacylevelalias requested in review. Prefer explicit nullish checks and maplevel→preset.Apply this diff:
- const lzmaOpts = { - preset: opts.preset || 6, - threads: opts.threads || 0, - }; + const preset = opts.preset != null + ? opts.preset + : (opts.level != null ? opts.level : 6); + const lzmaOpts = { + preset, + threads: opts.threads || 0, + };
19-36: DRY: Extract shared “source → stream” wiring to a utility and reuse in uncompress_stream.This file repeats the same source handling logic as in lib/xz/uncompress_stream.js. Centralize it (e.g., utils.pipeFromSource(opts, destStream)) to reduce duplication and future drift.
Example utility you can add (outside this diff, e.g., in lib/utils.js):
function pipeFromSource(opts, destStream) { const type = sourceType(opts.source); if (type === 'file') { const stream = fs.createReadStream(opts.source, opts.fs); stream.on('error', err => destStream.emit('error', err)); return stream.pipe(destStream); } if (type === 'buffer') { const stream = streamifier.createReadStream(opts.source, opts.streamifier); stream.on('error', err => destStream.emit('error', err)); return stream.pipe(destStream); } if (type === 'stream') { opts.source.on('error', err => destStream.emit('error', err)); return opts.source.pipe(destStream); } // no-op when no source provided (allows external piping) return destStream; }Then here and in uncompress_stream.js:
utils.pipeFromSource(opts, this);test/xz/uncompress_stream.test.js (1)
18-18: Fix lint: remove trailing whitespace on the blank line.CI flagged trailing spaces on this line; strip them to satisfy the linter.
Apply this diff:
- +
🧹 Nitpick comments (7)
lib/xz/file_stream.js (2)
38-39: Clarify behavior for unsupported source types.Current comment “do nothing” also applies when an invalid/unknown source type is passed. Consider no-op only when opts.source is undefined; otherwise, emit a TypeError to surface misconfiguration early.
Example:
const type = utils.sourceType(opts.source); if (opts.source !== undefined && type !== 'file' && type !== 'buffer' && type !== 'stream') { process.nextTick(() => this.emit('error', new TypeError('Unsupported XZ source type'))); }
12-13: Optional: validate lzma preset range upfront.Validate
presetin [0..9] andthreads>= 0 to fail fast with a clear message instead of relying on native errors.Example:
if (!(preset >= 0 && preset <= 9)) throw new RangeError(`Invalid XZ preset: ${preset}`); if (!(Number.isInteger(lzmaOpts.threads) && lzmaOpts.threads >= 0)) throw new RangeError(`Invalid threads: ${lzmaOpts.threads}`);test/xz/uncompress_stream.test.js (5)
16-18: Avoid temp file name collisions in createXzFile().Use a unique filename to prevent rare collisions across concurrent runs.
Apply this diff:
- const destFile = path.join(os.tmpdir(), 'test-xx.log.xz'); + const destFile = path.join(os.tmpdir(), `test-xx-${uuid.v4()}.log.xz`);
46-57: Prefer assert.ifError(err) and clean up temp file after assertions.
assert.ifError(err)expresses intent better. Also unlink the temp file to avoid littering /tmp.Apply this diff:
- pump(sourceStream, uncompressStream, destStream, err => { - assert(!err); + pump(sourceStream, uncompressStream, destStream, err => { + assert.ifError(err); assert(fs.existsSync(destFile)); const originalFileBuffer = fs.readFileSync(originalFile); const destFileBuffer = fs.readFileSync(destFile); assert.equal(destFileBuffer.length, originalFileBuffer.length); if (!isWindows) { // EOL not equal to linux assert.equal(destFileBuffer.toString('utf8'), originalFileBuffer.toString('utf8')); } - done(); + fs.unlinkSync(destFile); + done(); });Consider applying the same two tweaks to the other pipeline tests for consistency (Lines 65-76, 84-95, 103-114).
116-123: Fix description typo: “exit” → “exist”.Minor test name correction.
Apply this diff:
-it('should emit error if sourceFile does not exit', done => { +it('should emit error if sourceFile does not exist', done => {
125-133: Grammar: “emit” → “emits” (and optional rename to avoid shadowing).Improve readability and avoid overshadowing the outer
sourceFileby renaming the local constant.Apply this diff:
-it('should emit error if sourceStream emit error', done => { - const sourceFile = 'file-not-exist'; - const sourceStream = fs.createReadStream(sourceFile); +it('should emit error if sourceStream emits error', done => { + const nonExistentFile = 'file-not-exist'; + const sourceStream = fs.createReadStream(nonExistentFile);
68-73: Assert strictness: prefer strictEqual for Buffer length and content.Using strict assertions reduces coercion surprises.
Example changes (apply similarly across these blocks):
-assert.equal(destFileBuffer.length, originalFileBuffer.length); +assert.strictEqual(destFileBuffer.length, originalFileBuffer.length); ... -assert.equal(destFileBuffer.toString('utf8'), originalFileBuffer.toString('utf8')); +assert.strictEqual(destFileBuffer.toString('utf8'), originalFileBuffer.toString('utf8'));Also applies to: 87-92, 106-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/xz/file_stream.js(1 hunks)lib/xz/index.js(1 hunks)lib/xz/uncompress_stream.js(1 hunks)test/xz/file_stream.test.js(1 hunks)test/xz/index.test.js(1 hunks)test/xz/uncompress_stream.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- test/xz/file_stream.test.js
- lib/xz/uncompress_stream.js
- lib/xz/index.js
- test/xz/index.test.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
lib/xz/file_stream.js (2)
lib/xz/uncompress_stream.js (4)
fs(3-3)lzma(4-4)utils(5-5)streamifier(6-6)lib/xz/index.js (2)
utils(3-3)XzFileStream(4-4)
test/xz/uncompress_stream.test.js (2)
lib/xz/file_stream.js (2)
fs(3-3)streamifier(6-6)lib/xz/uncompress_stream.js (2)
fs(3-3)streamifier(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Node.js / Test (macos-latest, 24)
- GitHub Check: Node.js / Test (windows-latest, 22)
This PR adds XZ compression and decompression support to the compressing library, addressing issue #96. Added XZ compression module with FileStream and UncompressStream classes, implemented compressFile() and uncompress() functions, added TypeScript definitions, and comprehensive unit tests. Fixes #96
Code with TRAE SOLO.
Summary by CodeRabbit
New Features
Tests
Chores