Skip to content
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

maybe require('buffer') could show a message #6891

Closed
mh-cbon opened this issue May 20, 2016 · 4 comments
Closed

maybe require('buffer') could show a message #6891

mh-cbon opened this issue May 20, 2016 · 4 comments
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@mh-cbon
Copy link

mh-cbon commented May 20, 2016

  • Version: 6.1.0
  • Platform: linux
  • Subsystem: fedora

I misread the doc, i did var Buffer = require('buffer'); and could not figure out why this was true Buffer.alloc===undefined.

Maybe node could display a warning message if someone tries to require buffer like i did, or, maybe it could throw an error as if it was not an existing package ?

@bnoordhuis
Copy link
Member

Maybe node could display a warning message if someone tries to require buffer like i did

Node has no (reliable) way to determine how the return value of require('buffer') is used.

maybe it could throw an error as if it was not an existing package ?

That would be a breaking change and rather disruptive to the ecosystem, so the chances of that happening are low.

What might be an option is to change lib/buffer.js so that it does:

Buffer.Buffer = Buffer;
Buffer.SlowBuffer = SlowBuffer;
// etc.
module.exports = Buffer;

But personally, I don't think that's much of an improvement.

@bnoordhuis bnoordhuis added buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js. labels May 20, 2016
@ChALkeR
Copy link
Member

ChALkeR commented May 20, 2016

@mh-cbon Why should it throw? buffer is an existing module, it's documented, and there is no way it's going to throw an error now. Moreover, require('buffer') is the documented way to get buffer.INSPECT_MAX_BYTES: https://nodejs.org/api/buffer.html#buffer_buffer_inspect_max_bytes and buffer.kMaxLength: https://nodejs.org/api/buffer.html#buffer_class_method_buffer_alloc_size_fill_encoding.

@mh-cbon
Copy link
Author

mh-cbon commented May 20, 2016

:s I see, makes sense, thanks for information.

So i was thinking, maybe that sentence, at the beginning of the doc page about Buffer, is misleading

The Buffer class is a global within Node.js, making it unlikely that one would need to ever use require('buffer').

As require('buffer') is in fact a totally different thing of Buffer.

As a doc reader I d say it should stop right after the coma.

@mh-cbon mh-cbon closed this as completed May 20, 2016
@ChALkeR
Copy link
Member

ChALkeR commented May 20, 2016

@mh-cbon Perhaps that should look like this instead:

The Buffer class is a global within Node.js, making it unlikely that one would need to ever use require('buffer').Buffer.

But I'm not sure. /cc @nodejs/documentation

stevemao added a commit to stevemao/node that referenced this issue May 21, 2016
addaleax pushed a commit that referenced this issue May 24, 2016
Fixes: #6891
PR-URL: #6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue May 30, 2016
Fixes: nodejs#6891
PR-URL: nodejs#6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this issue Jun 2, 2016
Fixes: #6891
PR-URL: #6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Jun 3, 2016
Fixes: #6891
PR-URL: #6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
Fixes: #6891
PR-URL: #6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
Fixes: #6891
PR-URL: #6914
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

3 participants