Skip to content

doc: Uint8Array support in Buffer functions #19948

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.

Sign up for GitHub

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

Closed
reviewher wants to merge 1 commit into from
+2 −2
Closed

doc: Uint8Array support in Buffer functions #19948

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/api/buffer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ changes:

> Stability: 0 - Deprecated: Use [`Buffer.from(buffer)`] instead.

* `buffer` {Buffer} An existing `Buffer` to copy data from.
* `buffer` {Buffer|Uint8Array} An existing `Buffer` to copy data from.
Copy link
Member

@jasnell jasnell Apr 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should likely be Buffer|TypedArray with the description An existing BufferorTypedArray` instance to copy from.

It's worth noting that any TypedArray can be used, not just Uint8Array. The issue, however, is that as values are copied, each will be coerced to an 8-byte value. For instance, Buffer.from(new Uint32Array([0xffff, 2, 3]) will produce a Buffer with values [0xff, 2, 3]. Likewise, Buffer.from(new Float32Array([1.23, 2, 3])) will yield a Buffer with values [1, 2, 3].

Copy link
Member

@addaleax addaleax Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell That’s more or less already documented as https://nodejs.org/api/buffer.html#buffer_class_method_buffer_from_array – we accept anything with a length property, basically.

Copy link
Member

@jasnell jasnell Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, I meant just worth noting in this thread, not in the docs. My mistake, I wasn't clear about that at all. The change here that I'd like to see is just listing TypedArray instead of Uint8Array in the bullet point.

Copy link
Contributor

@SheetJSDev SheetJSDev Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell the body language may need some clarification if the signature mentions general Typed Arrays:

Copies the passed buffer data onto a new Buffer instance.

For a typed array that isn't a Uint8Array, does the "data" refer to the typed values or the .buffer data? e.g.

Buffer.from(new Float32Array([Math.PI]))        // <Buffer 03>
Buffer.from(new Float32Array([Math.PI]).buffer) // <Buffer db 0f 49 40>

This issue is obviated in the Uint8Array case but is relevant for other typed arrays.

Copy link
Member

@addaleax addaleax Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, exactly: Other kinds of TypedArrays are treated just like regular Arrays. Hence my comment above – I think it would make more sense to document TypedArray as part of the Buffer.from(array) heading.

Copy link
Member

@jasnell jasnell Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Buffer.from(new Float32Array([Math.PI]).buffer) variant is already covered in the documentation by the Class Method: Buffer.from(arrayBuffer[, byteOffset[, length]]) section. In that specific case, when an ArrayBuffer instance is passed, the Buffer returned is a view over the existing ArrayBuffer and no data is copied or coerced.

The Buffer.from(new Float32Array([Math.PI])) case is covered by the Class Method: Buffer.from(buffer) variant but the doc for that specific variant only needs to be indicate that any TypedArray can be passed but that each typed value will be copied and coerced into 8-bit values in the returned Buffer.

The Class Method: Buffer.from(buffer) and Class Method: Buffer.from(array) variants can be merged into a single section as the behavior for each is generally identical.

Copy link
Author

@reviewher reviewher Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local state was messed up, resubmitted PR in #19949 . I agree with @addaleax 's suggestion to clarify the typed array situation elsewhere, but that is a separate discussion

Copy link
Member

@jasnell jasnell Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that Buffer.from(new Uint8Array([1,2,3])) is treated exactly the same as Buffer.from([1,2,3]) ... that is, the new Buffer contains a copy of the original.

while Buffer.from((new Uint8Array([1,2,3])).buffer) returns a new Buffer that contains a view over the same backing ArrayBuffer. No data is copied.


Copies the passed `buffer` data onto a new `Buffer` instance.

Expand Down Expand Up @@ -842,7 +842,7 @@ A `TypeError` will be thrown if `arrayBuffer` is not an [`ArrayBuffer`] or a
added: v5.10.0
-->

* `buffer` {Buffer} An existing `Buffer` to copy data from.
* `buffer` {Buffer|Uint8Array} An existing `Buffer` to copy data from.

Copies the passed `buffer` data onto a new `Buffer` instance.

Expand Down