-
-
Notifications
You must be signed in to change notification settings - Fork 41
unconstrain Buffer #11
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
Conversation
|
ping @feross - it'd be good to resolve this as currently, it breaks a few peoples builds |
|
hey guys, when are we planning to merges these two PRs? |
|
ping @feross |
feross
left a comment
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.
This PR doesn't make sense. If the goal is to prevent node users from using the npm buffer package, then no change is needed. require('buffer') will always return the built-in buffer package in Node.js. Built-in modules get priority over packages in node_modules.
Tip: the way to require the npm package is require('buffer/')
|
|
||
| if (Buffer.from && Buffer.alloc && Buffer.allocUnsafe && Buffer.allocUnsafeSlow) { | ||
| module.exports = buffer | ||
| module.exports = Buffer |
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.
This is a huge breaking change. require('buffer') !== Buffer. Try doing require('buffer') and look at what you get.
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.
Indeed! Sorry
| throw new TypeError('Argument must be a number') | ||
| } | ||
| return buffer.SlowBuffer(size) | ||
| return Buffer.SlowBuffer(size) |
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.
Buffer.SlowBuffer is undefined...
I was not aware of this. Awesome! |
This, combined with #10 should resolve the issues at large.
buffer, the dependency, is only used inbrowser.js, otherwise the nativeBufferis used (intrinsically) and therefore we shim it if necessary.