-
Notifications
You must be signed in to change notification settings - Fork 569
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
Use npm scripts instead of gulp #530
Use npm scripts instead of gulp #530
Conversation
@@ -136,7 +136,7 @@ Polling.prototype.onDataRequest = function (req, res) { | |||
this.dataReq = req; | |||
this.dataRes = res; | |||
|
|||
var chunks = isBinary ? new Buffer(0) : ''; | |||
var chunks = isBinary ? new Buffer(0) : ''; // eslint-disable-line node/no-deprecated-api |
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.
Why?
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.
> eslint lib/ test/ *.js
~/engine.io/lib/transports/polling.js
165:27 error 'new Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead node/no-deprecated-api
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.
@darrachequesne I think the question is, why add the eslint-disable-line
instead of going with the advice in the deprecation message and using Buffer.alloc(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.
Next question then 😄 : do you suggest adding yet another dependency (safe-buffer
), or losing support for Node.js <4.5.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.
@darrachequesne Any of those would be fine. Is there a reason for keeping support to Node.js versions <4.5.0
? Those are obsolete and are not getting security updates. Even 4.x branch would be out of security support in a few months.
Also, you can use Buffer.concat([])
for this specific line (to construct an empty Buffer
) to keep support for old Node.js versions and avoid hitting deprecated API. But I would recommend just to drop support for everything older than 4.5.0
.
The kind of change this PR does introduce