Add support for also displaying Brotli compressed size

[jonatanlinden]

As the title says. It seems to be gaining ground, maybe someone else also will profit from such an option.

[sindresorhus]

Why not let the user display both if they want to?

[jonatanlinden]

Yes, that would be better of course. To be honest, it is because of my very limited skills with javascript and promises. I'll have a look at it.

[jonatanlinden]

I've done an attempt at letting the user show uncompressed, gzip and brotli size in any combination.

However, there seems to be some issues with the tests that rely on the finish event. After reading around and trying to understand, it seems that (at least one part of the problem) may be that the _flush function is not guaranteed to have finished before the finish event. So the size property of the stream may be undefined in those tests.

The 'finish' and 'end' events are from the parent Writable and Readable classes respectively. The 'finish' event is fired after stream.end() is called and all chunks have been processed by stream._transform(), 'end' is fired after all data has been output which is after the callback in stream._flush() has been called.

[jonatanlinden]

Sorry, that was me following a red herring. Fixed.

[sindresorhus]

Use a for-of loop.

And use descriptive variable/function names. No abbreviations.

[sindresorhus]

Needs a better name.

[sindresorhus]

It's generally better to use a Map for this.

[sindresorhus]

Have you manually verified that all combinations of the options work correctly?

[jonatanlinden]

No, I've just tried a few combinations. I guess the proper thing would be to add a couple of tests.

[jonatanlinden]

Speaking of which, I tried to test some more cases, and found something that I couldn't explain, which is also present in the original as well. In streaming mode, I can't manage to see that the data of the stream is actually piped on. The error could of course lie on my side.

For example, by modifying the test 'should handle stream contents' (and here I'm a bit on shaky ground), the second gulp-size invocation reports a size of 0.

it('should handle stream contents', callback => {
	const contents = through();
	const stream = size();
	const stream2 = size();

	stream2.on('finish', () => {
		assert.strictEqual(stream2.size, 100);
		callback();
	});

	stream.write(new Vinyl({
		path: path.join(__dirname, 'fixture.js'),
		contents
	}));

	contents.end(Buffer.alloc(100));
	stream.pipe(stream2);
	stream.end();
});

[sindresorhus]

I don't know, TBH. I haven't worked on this package for a long time.

[jonatanlinden]

Regarding the issue mentioned above, it seems to be related to gulpjs/gulp#2380. I moved the callback (in the original gulp-size) from finish to directly after piping the file contents in case of it being a stream. After that it works, kind-of, in the sense that it doesn't empty the vinyl stream.

[jonatanlinden]

Sorry, it seems brotli-size requires node >= 10.16.