-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
Start moving to Uint8Array in new APIs? #41588
Comments
That will cause soo much confusion. I believe there are several methods/props that Buffer override and which act differently than Uint8Array. |
In particular |
I'm fine with using Uint8Array for Web apis which define the type. But for node api's I think we should stick with Buffer. |
It really depends on the subsystems we are targeting. It's impossible to make a generic call. |
I have a big problem with Buffers.
// Somewhere in your code
const privateBuf = Buffer.from(privateKey, 'hex');
// Rogue package can access
Buffer.from('1').buffer
// Which will of course show the contents of `privateBuf`
// No need in complex memory dumps! This happens because there is 8KB shared buffer reused for all |
FWIW: I agree having an API that behaves like a subclass but "lies" about keeping the same API structure is super-confusing. |
What about the following suggestion:
Less likely to reach consensus but I'd still like that:
|
We will never be able to get rid of Buffer. The generic rules of thumb I have in mind are:
Examples where Buffer may be needed:
|
Yes please :)
This is a great example to show that it’s not actually trivial, because
That’s also the current state of things – even |
I ran into that as well (even though I knew about the bug before) so I'd like to echo it's a problem.
That's neat I didn't know that. |
@jasnell That’s a bug then :) |
That needs to be implemented as a hex/base64 decoder in stdlib, no need in using Buffers for this. The new stdlib decoders will work with Uint8arrays etc.
Which is the same as .subarray()
Pooling can be easily done with Uint8Arrays. And it could be implemented in a way which does not expose private contents to a global variable. I'm doing this in https://github.com/paulmillr/noble-hashes (see src/sha256.ts). |
@paulmillr to address some of the comment status:
We are (likely) going to docs-depercate (and
The ask here is that any API that aims to be compatible with web APIs (for example
I can see why this is done but this is very unfortunate and I did not realize this before. I'm wondering if this can be fixed while keeping the pooling behaviour? (By overriding |
(and similarly - this is very much an area Node could use more people involved and more contributions in!) |
I'm agaisnt the idea. It will not only cause confusion and compatibility issues, but also decrease in (micro) performance |
I'm a bit against using
I'm comparing this a bit to this article: https://jakearchibald.com/2021/function-callback-risks/ At some point somebody might just say: screw it: we are going to switch to using Uint8Array instead (cuz we already treat it as Uint8Array-like) so it can change the hole underlying infrastructure when it uses slice I don't think that extending any built in types are such a great idea, what if buffer added a custom I think Buffer is bloated with stuff that we don't actually need (specially for browser) I even wonder why Blob exist on the buffer object at all... Maybe it would be a good idea to fleshed out all of the utility method like to/from hex and base64 got added into a own sperate util module you could import from honestly, i just wish to see Buffer being deprecated and gone all together. but that is likely never going to happen |
I'm having a somewhat related problem with |
I wrote a blog post on my intent to move away from |
It looks like it's being worked on: https://github.com/tc39/proposal-arraybuffer-base64 |
I think the first step would be to add a note here about preferring |
This comment was marked as off-topic.
This comment was marked as off-topic.
I wish that buffer never got added to the global scope in the first place. It had led to many bundler polyfilling it when it is seen as optional. I can only hope that a new spec'ed web worker dose not include globalThis.buffer globalThis.process and globalThis.global by default. It should be explicit imported when needed |
I don't believe we have consensus on that at all. |
It would be quite a low effort to create a PR with that note, and probably the best way to seek consensus. |
I don't think there is a good alternative for https://github.com/sindresorhus/uint8array-extras does not provide helpers with the same performance. |
TBH I'm not sure I understand why this issue is still open. @benjamin when you say "the ask here is that Node.js should prefer Uint8Arrays over Buffers in new APIs." what do you mean by "prefer"? Is it about the type of the value returned by new APIs? @ronag it's easy to create a |
I would be in favor of this package, if it meant that i didn't need to ship node:buffer to browser and other runtimes. even if it meant a perf loss.
Maybe stands as a reminder that newly developed api's should prefer to return Uint8Array instead?
Can only speak for myself, but i guess he means that ppl should avoid spending time on reading up on the Buffer class and all stuff in it and resort to using Uint8Array/DataView/TextEncoder/Decoder instead as it's better cross compatible with other env. not about changing the return type of what new api's should return. it's quite funny how atob/btoa and this contradicts each other. i think this statment should be removed or changed: |
@jimmywarting I don't think it's appropriate to open the |
|
No, because that creates bifurcation. Better to be consistently wrong than inconsistently right. |
Once my proposal for native Uint8Array <-> base64 is shipping, which I am hoping will be within a year or two, I think it would be reasonable to suggest to readers of the Buffer documentation that the native Uint8Array class should be able to fulfill their needs.
Those are also part of the JS proposal for base64 on Uint8Arrays, incidentally. I just haven't been emphasizing hex because it's a lot easier (and simpler) than base64. |
There was a suggestion by @jasnell to use
Uint8Array
s in new APIs overBuffer
s as well as a weigh-in by @sindresorhus saying it is easier to author cross-platform APIs when usingUint8Array
s.Here is a context #41553 (comment)
That is, the ask here is that Node.js should prefer
Uint8Array
s overBuffer
s in new APIs.What does everyone think? Should we stick to
Buffer
(which is a subclass ofUint8Array
as a reminder) or preferUint8Array
s over buffers when possible in new APIs?cc @nodejs/buffer @nodejs/streams
The text was updated successfully, but these errors were encountered: