Conversation
| }, | ||
| "globals": {}, | ||
| "parserOptions": { "ecmaVersion": 8 }, | ||
| "parserOptions": { |
There was a problem hiding this comment.
All formatting changes here are automated by the ESlint plugin in my editor
| 2, | ||
| { "afterLineComment": true, "allowBlockEnd": true } | ||
| { | ||
| "afterLineComment": false, |
There was a problem hiding this comment.
I've changed this because the existing code wasn't following this rule (and also I think it's fairly common to have no empty line between a single line comment and the next line it explains)
| @@ -37,11 +60,11 @@ function verifyBuffer(rspd, reject) { | |||
|
|
|||
| function updateHeaders(res, rspdBefore, rspdAfter, reject) { | |||
| if (!res.headersSent) { | |||
There was a problem hiding this comment.
Automatically formatted, it looks like 4 spaces were used
|
|
||
| var maybeUnzipPromise = zipOrUnzip('gunzip'); | ||
| var maybeZipPromise = zipOrUnzip('gzip'); | ||
| function createEncodingHandler(res) { |
There was a problem hiding this comment.
Obviously, this is the central part
| ret = body; | ||
| } else if (typeof body === 'object') { | ||
| ret = new Buffer(JSON.stringify(body), options.reqBodyEncoding); | ||
| ret = Buffer.from(JSON.stringify(body), options.reqBodyEncoding); |
There was a problem hiding this comment.
Buffer constructor is deprecated and the from method is available in node v6+ so it shouldn't be a breaking change. Fixed it to remove the warnings during the tests
| var compression = require('compression'); | ||
| var shrinkRay = require('shrink-ray-current'); | ||
| var request = require('supertest'); | ||
| // superagent/supertest don't support `br` encoding |
| .get('/') | ||
| .expect(function (res) { | ||
| res.headers.location.match(/localhost:3000/); | ||
| assert(res.headers.location.match(/localhost:3000/)); |
There was a problem hiding this comment.
I assume it was missing here. Didn't check other tests
| beforeEach(function () { | ||
| app = express(); | ||
| target = express(); | ||
| target.use(compression({ threshold: 0 })); |
There was a problem hiding this comment.
The threshold ensures the small payload is still gzip-encoded
|
|
||
| request(app) | ||
| .get('/proxy/test') | ||
| .set('Accept-Encoding', 'gzip') |
| // with the original binary data | ||
| encoding: null, | ||
| headers: { | ||
| 'Accept-Encoding': 'br' |
|
Huh, the |
1 participant
I'll leave a couple of comments to justify some non-obvious changes