Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for streaming output with option #118

Merged
merged 16 commits into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const path = require('path');
const BufferConstants = require('buffer').constants;
const Vinyl = require('vinyl');
const PluginError = require('plugin-error');
const through = require('through2');
Expand All @@ -13,6 +14,7 @@ module.exports = (filename, options) => {

options = {
compress: true,
buffer: true,
...options
};

Expand Down Expand Up @@ -64,7 +66,22 @@ module.exports = (filename, options) => {
}

(async () => {
const data = await getStream.buffer(zip.outputStream);
let data;
if (options.buffer) {
try {
data = await getStream.buffer(zip.outputStream, {maxBuffer: BufferConstants.MAX_LENGTH});
} catch (error) {
if (error instanceof getStream.MaxBufferError) {
callback(new PluginError('gulp-zip', 'The output ZIP file is too big to store in a buffer (larger than Buffer MAX_LENGTH). To output a stream instead, set the gulp-zip buffer option to `false`.'));
} else {
callback(error);
}

return;
}
} else {
data = zip.outputStream;
}

this.push(new Vinyl({
cwd: firstFile.cwd,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"file"
],
"dependencies": {
"get-stream": "^5.1.0",
"get-stream": "^5.2.0",
"plugin-error": "^1.0.1",
"through2": "^3.0.1",
"vinyl": "^2.1.0",
Expand Down
12 changes: 12 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,15 @@ Default: `undefined`
Overrides the modification timestamp for all files added to the archive.

Tip: Setting it to the same value across executions enables you to create stable archives that change only when the contents of their entries change, regardless of whether those entries were "touched" or regenerated.

##### buffer

Type: `boolean`<br>
Default: `true`

If `true`, the resulting ZIP file contents will be a buffer. Large zip files may not be possible to buffer, depending on the size of [Buffer MAX_LENGTH](https://nodejs.org/api/buffer.html#buffer_buffer_constants_max_length).
If `false`, the ZIP file contents will be a stream.

Similar to [gulp.src's `buffer` option](https://gulpjs.com/docs/en/api/src/#options).
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, but it should be clearly documented in what case the option is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully clarified that now.

Copy link
Owner

Choose a reason for hiding this comment

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

It's not really clearer. As a user, I would have liked to know why the option exists when there's the gulp.src's buffer option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I felt that was explained in the paragraph above. Do you want me to remove the comparison or add more detail here?

Copy link
Owner

Choose a reason for hiding this comment

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

Add more detail.


Tip: set this to `false` to allow creating ZIP files larger than Node's maximum buffer length.
Copy link
Owner

Choose a reason for hiding this comment

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

This is just repeating what's on line 60. I think the option docs should explain why it exists when there's already such option for gulp.src.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would something like this work:

We use this option instead of relying on gulp.src's buffer option because we are mapping many input files to one output file and can't reliably detect what the output mode should be based on the inputs, since Vinyl streams could contain mixed streaming and buffered content.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that's good.

71 changes: 71 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from 'fs';
import path from 'path';
import {constants as BufferConstants} from 'buffer';
import test from 'ava';
import Vinyl from 'vinyl';
import through2 from 'through2';
Expand Down Expand Up @@ -32,6 +33,7 @@ test.cb('should zip files', t => {
stream.on('data', file => {
t.is(path.normalize(file.path), path.join(__dirname, 'fixture/test.zip'));
t.is(file.relative, 'test.zip');
t.true(file.isBuffer());
t.true(file.contents.length > 0);
});

Expand Down Expand Up @@ -225,3 +227,72 @@ test.cb('when `options.modifiedTime` is specified, should create identical zips
}
}));
});

test.cb('should produce a buffer by default', t => {
const stream = zip('test.zip');
const file = vinylFile.readSync(path.join(__dirname, 'fixture/fixture.txt'));

stream.on('data', file => {
t.true(file.isBuffer());
});

stream.on('end', () => {
t.end();
});

stream.write(file);
stream.end();
});

test.cb('should produce a stream if requested', t => {
const stream = zip('test.zip', {buffer: false});
const file = vinylFile.readSync(path.join(__dirname, 'fixture/fixture.txt'));

stream.on('data', file => {
t.true(file.isStream());
});

stream.on('end', () => {
t.end();
});

stream.write(file);
stream.end();
});

test.cb('should explain buffer size errors', t => {
const stream = zip('test.zip', {compress: false});
const unzipper = unzip();
const stats = fs.statSync(path.join(__dirname, 'fixture/fixture.txt'));
stream.pipe(vinylAssign({extract: true})).pipe(unzipper);

stream.on('error', error => {
t.is(error.message, 'The output ZIP file is too big to store in a buffer (larger than Buffer MAX_LENGTH). To output a stream instead, set the gulp-zip buffer option to `false`.');
t.end();
});

function addFile(contents) {
stream.write(new Vinyl({
cwd: __dirname,
base: path.join(__dirname, 'fixture'),
path: path.join(__dirname, 'fixture/file.txt'),
contents,
stat: stats
}));
}

// Yazl internally enforces a lower max buffer size than MAX_LENGTH
const maxYazlBuffer = 1073741823;

// Produce some giant data files to exceed max length but staying under Yazl's maximum
const filesNeeded = Math.floor(BufferConstants.MAX_LENGTH / maxYazlBuffer);
for (let files = 0; files < filesNeeded; files++) {
addFile(Buffer.allocUnsafe(maxYazlBuffer));
}

// Pad all the way up to BufferConstants.MAX_LENGTH
addFile(Buffer.allocUnsafe(BufferConstants.MAX_LENGTH % maxYazlBuffer));

stream.end();
});