search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.93k stars 29.18k forks source link

Allow `Buffer.copy` to take an `ArrayBuffer` #53700

Open ronag opened 3 months ago

ronag commented 3 months ago

What is the problem this feature will solve?

Avoid creating a temporary typed buffer:

Buffer.from(arrayBuffer).copy(dst, 0)

What is the feature you are proposing to solve the problem?

Buffer.copy(arrayBuffer, dst, 0)

What alternatives have you considered?

No response

ronag commented 3 months ago

@nodejs/performance @joyeecheung

aduh95 commented 3 months ago

Buffer.copyWithin might be a clearer name than Buffer.copy.

eliphazb commented 2 months ago

@ronag I'm going to work on this feature :)

ayan-de commented 2 months ago

@ronag i am going to fix this issue

eliphazb commented 2 months ago

@ayan-de I'm already working on it

ayan-de commented 2 months ago

@ayan-de I'm already working on it

No problem i will also try it

ayan-de commented 2 months ago

What is the problem this feature will solve?

Avoid creating a temporary typed buffer:

Buffer.from(arrayBuffer).copy(dst, 0)

What is the feature you are proposing to solve the problem?

Buffer.copy(arrayBuffer, dst, 0)

What alternatives have you considered?

No response

@ronag will this Buffer.copy a new method or it is just buf.copy(target[, targetStart[, sourceStart[, sourceEnd]]]) function that will also start taking ArrayBuffer in the target parameter?

ronag commented 2 months ago

@ronag will this Buffer.copy a new method or it is just buf.copy(target[, targetStart[, sourceStart[, sourceEnd]]]) function that will also start taking ArrayBuffer in the target parameter?

The idea is to be able to copy both to and from an ArrayBuffer so I would prefer a static method.

Qard commented 2 months ago

Might be a good idea to centralize all copying in that static method so the instance copy method delegates to the static method and it just handles copying of all buffer varieties. 🤔

eliphazb commented 2 months ago

@Qard @ronag something like : Buffer.copyWithin(arrayBuffer[, dest[, sourceStart]]]) and throw a error if something else than a arrayBuffer is given ?

Qard commented 2 months ago

Why only ArrayBuffer? Seems to me like it's better to support all varieties of buffer. 🤔

eliphazb commented 2 months ago

@Qard so arrayBuffer buffer array ? array is also be part varieties of buffer ? something like : 

 Buffer.copyWithin(array[, dest[, sourceStart]]])
 Buffer.copyWithin(buffer[, dest[, sourceStart]]])
 Buffer.copyWithin(arrayBuffer[, dest[, sourceStart]]])
Qard commented 2 months ago

I was thinking more the buffer-intended things. So Buffer, ArrayBuffer, and DataView. Maybe typed arrays could be included too.

eliphazb commented 2 weeks ago

@Qard @ronag Can I create a PR draft to show you my code during implementation, to give me some advice if I do something wrong?