-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[feature]: zlib deflate concurrency limit #1204
Conversation
4ec7b28
to
3833221
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments, only style nits. Other than that LGTM.
lib/PerMessageDeflate.js
Outdated
@@ -4,12 +4,21 @@ const safeBuffer = require('safe-buffer'); | |||
const zlib = require('zlib'); | |||
|
|||
const bufferUtil = require('./BufferUtil'); | |||
const Limiter = require('async-limiter'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please move this to the above block?
lib/PerMessageDeflate.js
Outdated
* | ||
* @param {Buffer} data Compressed data | ||
* @param {Boolean} fin Specifies whether or not this is the last fragment | ||
* @param {Function} callback Callback | ||
* @public | ||
*/ | ||
decompress (data, fin, callback) { | ||
zlibLimiter.push((done) => { | ||
this._decompress(data, fin, function (err, result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind to use arrow functions for consistency?
lib/PerMessageDeflate.js
Outdated
*/ | ||
compress (data, fin, callback) { | ||
zlibLimiter.push((done) => { | ||
this._compress(data, fin, function (err, result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
package.json
Outdated
@@ -26,6 +26,7 @@ | |||
"lint": "eslint ." | |||
}, | |||
"dependencies": { | |||
"async-limiter": "^1.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please use the same versioning scheme?
3833221
to
946edc2
Compare
Great - fixed nits. |
Thank you. |
Ref: #1202
Built on top of async-limiter which is only 1kB of dependency-free ES5.