Allow users to pass in their own compression-stream function

[expressjs]

Allow users to pass in their own compression-stream function

[dougwilson]

Cool. This library cannot take a dependency on a native addon, due to the situation of native addons and Windows, making it really hard for people to use.

But, I do want to increase the flexibility of this module such that users can define their own encodings, like brotli.

[dougwilson]

Huh, it looks like that module you referenced, brotli, is actually a pure-JS module :)

[nickdesaulniers]

due to the situation of native addons and Windows

can you elaborate on this?

[dougwilson]

When you install Node.js, running npm install native_module will fail because there is no tool chains to compile anything on Windows. As I exclusively develop on Windows for these modules, if I'm not even able to install a module out-of-the-box, then I can't even develop against it, let alone all the other users.

This has been a major issue for years & years and no real progress towards addressing. For example, a standard Node.js install on my machine, when I try to install your nanomsg module, here is the error:

$ npm install nanomsg
|
> [email protected] install c:\Users\doug.wilson\test2\node_modules\nanomsg
> node-gyp rebuild


c:\Users\doug.wilson\test2\node_modules\nanomsg>node "c:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild
gyp ERR! configure error
gyp ERR! stack Error: Can't find Python executable "python", you can set the PYTHON env variable.
gyp ERR! stack     at failNoPython (c:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:103:14)
gyp ERR! stack     at c:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:64:11
gyp ERR! stack     at Object.oncomplete (evalmachine.<anonymous>:107:15)
gyp ERR! System Windows_NT 6.1.7601
gyp ERR! command "node" "c:\\Program Files\\nodejs\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd c:\Users\doug.wilson\test2\node_modules\nanomsg
gyp ERR! node -v v0.10.33
gyp ERR! node-gyp -v v1.0.1
gyp ERR! not ok

npm ERR! [email protected] install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is most likely a problem with the nanomsg package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node-gyp rebuild
npm ERR! You can get their info via:
npm ERR!     npm owner ls nanomsg
npm ERR! There is likely additional logging output above.
npm ERR! System Windows_NT 6.1.7601
npm ERR! command "c:\\Program Files\\nodejs\\node.exe" "c:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install" "nanomsg"
npm ERR! cwd c:\Users\doug.wilson\test2
npm ERR! node -v v0.10.33
npm ERR! npm -v 1.4.28
npm ERR! code ELIFECYCLE
npm ERR! not ok code 0

Just installing Python onto Windows (not a simple task) is only the start. Users then have to figure out how to get the correct version of Visual Studio installed, get the proper environment variables setup so gyp works, etc. It's pretty much an impossible task for all people starting on Node.js on Windows.

[nickdesaulniers]

Isn't this also the case with OSX as command line tools aren't installed by default?

[dougwilson]

I'm not familiar with Mac OS X, but I can say that when I had a module that has a native dependency, I only got endless Windows users making issue reports, but no Mac OS X or Linux users, so there is some large disconnect between those two populations of developers.

You can even find dozens of issues in the Node.js repository (like nodejs/node-v0.x-archive#8005) expressing the issue of how hard it is for typical Windows developers to use native modules. I don't see any of that in the Node.js repo from Mac OS X users.

[dougwilson]

P.S. as far as I can tell, that brotli module doesn't implement a streaming interface, which is kinda a downer :(

[dougwilson]

It looks like there is a native addon https://www.npmjs.com/package/iltorb that provides brotli and even does so as a stream. This means we have here the following wants:

1. Implement a way for users to define their own encodings in the module, which would let a user use a module like https://www.npmjs.com/package/iltorb
2. If someone is willing to either get brotli into Node.js core itself or make a pure-JS brotli implementation that provides a stream interface, we can add it as a dependency for brotli out-of-the-box.

[nickdesaulniers]

Nice!

1. yep! This code currently has a conditional that checks if gzip else use deflate. We should make it check first to see if it was passed a custom compression function that implements streams and use that.
2. I don't think we need to wait for brotli to land in Node core. I don't think it will, unless brotli was added to zlib, which it may never. With number 1 from above implemented, I don't think 2 will be an issue at all.

[nickdesaulniers]

I'll work on this today, so far I have the tests:

diff --git a/test/compression.js b/test/compression.js
index 26e0ca2..87abbd2 100644
--- a/test/compression.js
+++ b/test/compression.js
@@ -3,6 +3,7 @@ var bytes = require('bytes');
 var crypto = require('crypto');
 var http = require('http');
 var request = require('supertest');
+var stream = require('stream');

 var compression = require('..');

@@ -492,6 +493,40 @@ describe('compression()', function(){
     })
   })

+  describe('when "Accept-Encoding: custom"', function () {
+    it('should not use content encoding without a custom compressor function', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end('hello, world')
+      })
+
+      request(server)
+      .get('/')
+      .set('Accept-Encoding', 'custom')
+      .expect(shouldNotHaveHeader('Content-Encoding'))
+      .expect(200, 'hello, world', done)
+    })
+
+    it('should use content encoding with a custom compressor function', function (done) {
+      var compressor = new stream.Transform
+      var opts = {
+        threshold: 0,
+        compressor: {
+          'bingo': compressor
+        }
+      }
+      var server = createServer(opts, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end('hello, world')
+      })
+
+      request(server)
+      .get('/')
+      .set('Accept-Encoding', 'bingo, gzip')
+      .expect('Content-Encoding', 'bingo', done)
+    })
+  })
+
   describe('when "Cache-Control: no-transform" response header', function () {
     it('should not compress response', function (done) {
       var server = createServer({ threshold: 0 }, function (req, res) {

[nickdesaulniers]

So here's an issue I'm running into. Firefox 44 (currently Firefox Nightly) sends the header: Accept-Encoding: gzip, deflate, br. The use of the accept.encoding function in this library will always return the first match, based on the order specified by the client. Since we can't manually disable gzip or deflate, gzip will be matched first, and custom encodings will never be used.

I propose that if the consumer of this lib specifies a custom encoding, we choose that, otherwise defer to accept.encoding. Thoughts?

[nickdesaulniers]

I added this to my PR

[LeandroFavero]

Maybe this can help:
A JavaScript port of the Brotli compression algorithm
https://github.com/devongovett/brotli.js

[KenjiBaheux]

Howdy!

I'm Kenji Baheux, product manager on the web platform team @ Chrome and helping the Brotli team.
I would like to understand how we can help you get Brotli support in expressjs.

We discussed about trying to get Brotli in Node core but felt that it would be hard given the sense that core is too big. However, it doesn't seem to be a blocker based on this comment.

The iltorb module seems to be the best route.

Are you blocked?
Anything that the Brotli team could help you with?

Best,

Note: Brotli is enabled by default in Chrome 51 (current stable).

[dougwilson]

Hi @KenjiBaheux, the only blocker here is that from Express.js rules for the widest support, we need one of the following to be implemented:

1. Complete pluggability in this module for people to integration their own custom streams. This would let people add Brotli support, but may not meet your goals, since this would still not get it to be an out-of-the-box option.
2. Get Brotli in the Node.js code so we can use it. You seem to indicate this is off the table for now.
3. Provide a pure-JS implemention of the algorithm with full streaming & flushing support so we can add it as a dependency of this module. It cannot require any native code or a compiler to use the dependency.

I hope this helps!

[nickdesaulniers]

Regarding route #3, iltorb just had some major refactorings that I think allow for this. I'm not pursing this anymore, but maybe @MayhemYDG or @addaleax could help.

[nickdesaulniers]

oh, missed the note about pure JS. good luck with that!

[KenjiBaheux]

Doug, thanks for the quick reply.

It's a tough one :)

• Option 1 is not great
• Option 2 is hard now. We think that with the right timing we have a chance, we'll wait after more demand.
• Option 3 is hard now and then.

We were wondering if the following option would be more practical:

Initially focus on the static cases where maximal compression can be achieved* by: 1. having a tool that would crawl a static asset graph to produce compressed .brotli files, 2. and then building express middleware that does the much simpler job of just stat()ing for the .brotli file and sending it down to the user instead of the original .js or .css file.

Thoughts?

*: for dynamic assets, lower Brotli levels are recommended for speed reasons but it would still outperform gzip compression.

[nstepien]

Would it help if iltorb offered pre-built binary fallbacks? I don't know how to go about that however.

[addaleax]

Oh, hi everyone! I think for iltorb to be used here requires adding a .flush() method like zlib streams have it (nstepien/iltorb#8), although that should have become implementable; in fact, the “official” C API from google/brotli has moved a lot closer to what other compression libraries provide.

I wouldn’t discard option 2 completely, either, but for now that’s probably off limits just because the brotli C API isn’t nearly as stable as the ones of other Node core dependencies right now. Also, other core collaborators tend to be more concerned about feature bloat than I am.

@MayhemYDG I do that for my lzma-native, so I have some experience with it.

[nstepien]

https://twitter.com/rvagg/status/748114279685988352
This could help in the future.

[nstepien]

iltorb now offers pre-built binaries, and features a .flush() method for compression streams.