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

Buffer.copyBytesFrom should accept a DataView as well. #47924

Closed
issuefiler opened this issue May 8, 2023 · 5 comments
Closed

Buffer.copyBytesFrom should accept a DataView as well. #47924

issuefiler opened this issue May 8, 2023 · 5 comments
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js. stale

Comments

@issuefiler
Copy link

issuefiler commented May 8, 2023

The issue

The Buffer.copyBytesFrom method was added by the issue #43862. What it does is create a Buffer by copying the data to which the view of an ArrayBuffer or SharedArrayBuffer is referring (“viewing”). The current problem is that it only accepts a TypedArray for its view parameter, when DataViews, too, are views.

> Buffer.copyBytesFrom(new DataView(new SharedArrayBuffer(1024)))
Uncaught:
TypeError [ERR_INVALID_ARG_TYPE]: The "view" argument must be an instance of TypedArray. Received an instance of DataView
    at __node_internal_captureLargerStackTrace (node:internal/errors:490:5)
    at new NodeError (node:internal/errors:399:5)
    at Function.copyBytesFrom (node:buffer:347:11) {
  code: 'ERR_INVALID_ARG_TYPE'
}

My suggestion

I suggest:

  • enabling Buffer.copyBytesFrom to accept a DataView as well for the view parameter.
  • modifying the documentation accordingly.
@issuefiler issuefiler added the feature request Issues that request new features to be added to Node.js. label May 8, 2023
@VoltrexKeyva VoltrexKeyva added the buffer Issues and PRs related to the buffer subsystem. label May 9, 2023
@sankalp1999
Copy link
Contributor

I think I can help with this. I can modify the code to accomodate for dataView.

@sankalp1999
Copy link
Contributor

sankalp1999 commented May 10, 2023

Relevant comments on why DataView was not added earlier.

#46500 (comment)

#46500 (comment)

To be frank at first glance, it looked manageable to me but it seems it's pretty complex.

Comments suggest that a separate method should be created for this. Would need maintainers' advice.

@sankalp1999
Copy link
Contributor

Relevant comments on why DataView was not added earlier.

#46500 (comment)

#46500 (comment)

To be frank at first glance, it looked manageable to me but it seems it's pretty complex.

Comments suggest that a separate method should be created for this. Would need maintainers' advice.

@jasnell what do you think

Copy link
Contributor

github-actions bot commented Nov 9, 2023

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Nov 9, 2023
Copy link
Contributor

github-actions bot commented Dec 9, 2023

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2023
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. stale
Projects
None yet
Development

No branches or pull requests

3 participants