assigning Buffer in repl throws error

[meandavejustice]

in a repl if I do

var Buffer = require('buffer');

I get this error

_stream_writable.js:171
  if (Buffer.isBuffer(chunk))
             ^
TypeError: Object #<Object> has no method 'isBuffer'
    at WriteStream.Writable.write (_stream_writable.js:171:14)
    at WriteStream.Socket.write (net.js:615:40)
    at finish (repl.js:291:27)
    at REPLServer.self.eval (repl.js:122:7)
    at repl.js:249:20
    at REPLServer.self.eval (repl.js:122:7)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)

I understand that core modules are included in repl globally, But this behavior seems wrong.

v.0.10.32

[meandavejustice]

cc/ @chrisdickinson

[odeke-em]

I've dug through the source. This is because 'isBuffer' was declared as a static method for Buffer, and never exposed, this is on https://github.com/joyent/node/blob/v0.10.32-release/lib/buffer.js#L277 and yet in the newer releases, util.isBuffer is being used inside _stream_writable.js for that check. Any thoughts on exposing this method or better conforming to the new usage of util.isBuffer?

[indutny]

This is happening because we are using Buffer through-out all of lib/ files without actually doing require('buffer') in them. I guess something like these at the top of all these files should do the trick:

var Buffer = require('buffer').Buffer.

cc @trevnorris

[trevnorris]

Overriding a documented global is going to lead to issues. Not sure I'd consider this a bug. That being said, I'd okay a patch either replacing Buffer.isBuffer() with util.isBuffer() or what @indutny mentioned.

[odeke-em]

Roger that. However, doing explicit requires in each of those file won't remedy the name conflict with the global object 'Buffer'.
Also util.isBuffer does not yet exist in util.js before these releases. A simple thought would be to advise avoiding giving names to variables that clash with those in the global namespace.
Irrespective of the requires, as many bug reports could be created by someone getting names of all the objects in the globally present in the repl, and then redefining them.
If a bit of complexity can be tolerated, we can ensure that the globally used objects are non writable with
Object.defineProperty(this, name, {writable: false})
clauses to avoid any future issues of name conflicts within the repl. Let me know what you think.

[indutny]

@odeke-em I think that we should follow the way that I have proposed :) lib/* should not be using global objects, ideally.

[odeke-em]

Thank you folks for the good tips. @indutny, kuddos -- I've been able to test this out in code with no more fails, PR with your solution coming up in a few. In deed global objects should not be used in lib/*.

[trevnorris]

Fixed by 523929c.