You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Buffer() without a new keyword is going to be hard-deprecated soon #1616
Use the new Buffer API — Buffer.alloc()/Buffer.from()/Buffer.allocUnsafe() (requires a shim for v0.10/v0.12 and older v4.x versions prior to v4.5.0).
Use new Buffer() for the time being — just add the new keyword everywhere. You should manually check that everything is safe in this case, see the links below for more explanation. That might be hard-deprecated at some later point.
The grep above only includes the lines that call Buffer() without the new keyword, if you choose to move to the new API — you should probably also replace new Buffer(…) calls.
Is there a list of things that need to be done to get this done? I am willing to help, but a casual glance at the source in the repo has nothing other than the tests usage of Buffer. Is the solution traverse the entire dep tree for usages of the old api?
Summarizing, I think there are a few ways to resolve this:
modify the browserify command to suppress the deprecation (easy but a hack)
vendor every dependency and make the relevant fixes in the tree (moderate but goes against the spirit of npm)
chase down every dependency, fix every incorrect use, and update the dependencies (hard and we risk facing the same issue next time node "pulls a fast one")
Upon reflection I'm a fan of choice 2, but it will be up to @substack to decide which is the best course of action
@spelunk To be fair, "pulling a fast one" would be if node core simply dropped constructor usage without any warning. The warning is there to give people time to migrate.
I honestly believe the only real solution is to fix the deprecations upstream/downstream, otherwise you're just masking the problem and you will get bitten once the deprecation becomes a removal at some point. At that time, things will probably be even worse because you'll be scrambling to fix actual broken dependencies.
@ai you can find the first offending use case by running browserify using the --trace-warnings option:
$ touch foo.js
$ node --trace-warnings `which browserify` foo.js >/dev/null
(node:18812) DeprecationWarning: Using Buffer without `new` will soon stop working. Use `new Buffer()`, or preferably `Buffer.from()`, `Buffer.allocUnsafe()` or `Buffer.alloc()` instead.
at Buffer (buffer.js:79:13)
at DestroyableTransform.write [as _transform] (/usr/local/lib/node_modules/browserify/node_modules/browser-pack/index.js:53:32)
at DestroyableTransform.Transform._read (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:159:10)
at DestroyableTransform.Transform._write (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:147:83)
at doWrite (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:313:64)
at writeOrBuffer (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:302:5)
at DestroyableTransform.Writable.write (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:241:11)
at DestroyableTransform.ondata (/usr/local/lib/node_modules/browserify/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:531:20)
at emitOne (events.js:96:13)
at DestroyableTransform.emit (events.js:188:7)
The offending line from /usr/local/lib/node_modules/browserify/node_modules/browser-pack/index.js is
if(first)stream.push(Buffer(prelude+'({'));
I think it was fixed in the source repo: browserify/browser-pack@c91b816 , so in this case it just comes down to @substack publishing a new version of the module. Hopefully that's the only offending case
@substack @ai changing the shebang line of bin/cmd.js to call node with the --node-deprecation argument suppresses the message without having to go through and change all of the dependencies:
Activity
terinjokes commentedon Aug 19, 2016
😑 . Time to make sure the entire browserify dependency tree is updated, I suppose.
ChALkeR commentedon Aug 25, 2016
Note: the deprecation was landed and will start printing warnings in Node.js 7.0.
theskillwithin commentedon Nov 4, 2016
i just installed browserify, nodejs 7.0
its a warning, but is it even running?
spelunk commentedon Nov 13, 2016
@substack would you accept a PR fixing this issue? I ended up rolling back to node 6 but at some point we'll have to upgrade :/
wesleytodd commentedon Nov 13, 2016
Is there a list of things that need to be done to get this done? I am willing to help, but a casual glance at the source in the repo has nothing other than the tests usage of
Buffer. Is the solution traverse the entire dep tree for usages of the old api?spelunk commentedon Nov 13, 2016
@wesleytodd yeah it looks like it will involve spelunking the dependency tree. One alternative according to @mscdex nodejs/node#9589 (comment) is to change the shebang line in https://github.com/substack/node-browserify/blob/master/bin/cmd.js to read
#!/usr/bin/env node --no-deprecation. It's worth noting that fixing the dependencies would at least help other projects that are suffering from the same issue.Summarizing, I think there are a few ways to resolve this:
Upon reflection I'm a fan of choice 2, but it will be up to @substack to decide which is the best course of action
mscdex commentedon Nov 13, 2016
@spelunk To be fair, "pulling a fast one" would be if node core simply dropped constructor usage without any warning. The warning is there to give people time to migrate.
I honestly believe the only real solution is to fix the deprecations upstream/downstream, otherwise you're just masking the problem and you will get bitten once the deprecation becomes a removal at some point. At that time, things will probably be even worse because you'll be scrambling to fix actual broken dependencies.
wesleytodd commentedon Nov 13, 2016
I agree with @mscdex on the way forward.
Side note, it was actually @substack that called it "pulling a fast one" :)
ai commentedon Nov 21, 2016
Did we found a bad dependency or we need help here?
spelunk commentedon Nov 21, 2016
@ai you can find the first offending use case by running browserify using the --trace-warnings option:
The offending line from /usr/local/lib/node_modules/browserify/node_modules/browser-pack/index.js is
I think it was fixed in the source repo: browserify/browser-pack@c91b816 , so in this case it just comes down to @substack publishing a new version of the module. Hopefully that's the only offending case
ai commentedon Nov 21, 2016
@spelunk thanks for
--trace-warningsargument, never know about it.I pinged
browser-packdevelopers browserify/browser-pack#76ai commentedon Nov 22, 2016
Yeap, I agree that warning is not a error.
Sorry, I thought that you could release it quick in patch release :).
spelunk commentedon Nov 22, 2016
@substack @ai changing the shebang line of bin/cmd.js to call node with the
--node-deprecationargument suppresses the message without having to go through and change all of the dependencies:This has the added benefit of suppressing future messages.