Use Buffer.(from|alloc) instead of deprecated Buffer API #30
Conversation
This also includes a dependnecy on a polyfill targeting older Node.js versions where Buffer.alloc() and Buffer.from() API is not implemented (Node.js < 4.5.0 and some 5.x versions). Ref: nodejs/node#19079 Ref: https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor Ref: https://nodejs.org/api/buffer.html#buffer_class_buffer Ref: https://github.com/ChALkeR/safer-buffer/blob/master/Porting-Buffer.md
| @@ -13,7 +13,9 @@ | |||
| "url": "git://github.com/mcavage/node-asn1.git" | |||
| }, | |||
| "main": "lib/index.js", | |||
| "dependencies": {}, | |||
| "dependencies": { | |||
| "safer-buffer": "~2.1.0" | |||
There was a problem hiding this comment.
This is not ^2.1.0 only because of npm bundled with Node.js 0.8 is too old and fails on ranges with ^.
The code itself works on 0.8. I suggest changing to ^2.1.0 and dropping Node.js 0.8 from test matrix, but that could be done in a non-patch release, so I'm not including that to this PR.
In fact, I suggest dropping safer-buffer in the 0.3.0 completely and bumping the minimum required Node.js version to 4.5.0 or to 6.0.0.
There was a problem hiding this comment.
Yeah, I think we can drop 0.8.x from this now (pretty sure the last of our production systems on 0.8 at Joyent have moved over to 0.10 or 4.x now, but I'm still checking). We don't have the luxury of dropping 0.10 anytime soon from this library, though.
There was a problem hiding this comment.
I believe the only Joyent component on 0.8 still is https://github.com/joyent/sdc-amon/blob/master/Makefile#L34
There was a problem hiding this comment.
Ah, thanks @cburroughs. It looks like all of amon's deps that would pull in asn1 are pinned to a version that also pins asn1, so yeah, I think we can drop 0.8 compat in this library safely without breaking those images. Whenever we get around to updating it we'll probably have to update its node version with the other deps though.
There was a problem hiding this comment.
@arekinath good to merge and release then? :)
There was a problem hiding this comment.
Hey @arekinath, do you have an idea when we can expect the release to reach npm? It's one of the only two remaining dependencies we have to update for Yarn to stop printing the warning (yarnpkg/yarn#6208) :)
There was a problem hiding this comment.
I've just published asn1@0.2.4.
|
Could someone merge it if everything is OK please? @pfmooney maybe? Node 10 is becoming LTS quite soon, I would love to see those warnings go away. |
This also includes a dependnecy on a polyfill targeting older Node.js versions where Buffer.alloc() and Buffer.from() API is not implemented (Node.js < 4.5.0 and some 5.x versions).
Ref: nodejs/node#19079
Ref: https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor
Ref: https://nodejs.org/api/buffer.html#buffer_class_buffer
Ref: https://github.com/ChALkeR/safer-buffer/blob/master/Porting-Buffer.md
Fixes: #29
I also recommend, after this lands (so that 0.2.x users will get the fix), to drop
safer-bufferpolyfill along with support for Node.js < 4.5.0 or even < 6.0.0 and to release that as 0.3.0.