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

napi is missing method to create buffer from ArrayBuffer #54440

Closed
ronag opened this issue Aug 18, 2024 · 5 comments · Fixed by #54505
Closed

napi is missing method to create buffer from ArrayBuffer #54440

ronag opened this issue Aug 18, 2024 · 5 comments · Fixed by #54505
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API.

Comments

@ronag
Copy link
Member

ronag commented Aug 18, 2024

What is the problem this feature will solve?

Not possible to create Buffer from ArrayBuffer with napi

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

Add signature similar to what we have for typed arrays:

napi_status napi_create_typedarray(napi_env env,
                                   napi_typedarray_type type,
                                   size_t length,
                                   napi_value arraybuffer,
                                   size_t byte_offset,
                                   napi_value* result) 
napi_status napi_create_buffer(napi_env env,
                               size_t length,
                               napi_value arraybuffer,
                               size_t byte_offset,
                               napi_value* result) 

What alternatives have you considered?

No response

@ronag ronag added the feature request Issues that request new features to be added to Node.js. label Aug 18, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Aug 18, 2024
@RedYetiDev RedYetiDev added buffer Issues and PRs related to the buffer subsystem. node-api Issues and PRs related to the Node-API. labels Aug 18, 2024
@RedYetiDev RedYetiDev moved this from Awaiting Triage to Triaged in Node.js feature requests Aug 18, 2024
@shubhamsugara22
Copy link

Would like to work on it , and discuss more on it
I think his feature would bring the API in line with the existing napi_create_typedarray function
@ronag @RedYetiDev

@shubhamsugara22
Copy link

We already have a functionin in src/node_api.cc , should we update this function or create new one with different name
@ronag @RedYetiDev
like to discuss on these 2 also , like reference and structure if already done for any other module
// Get the underlying ArrayBuffer data
// Create a Buffer from the ArrayBuffer

napi_status NAPI_CDECL napi_create_buffer(napi_env env,
                                          size_t length,
                                          void** data,
                                          napi_value* result) {
  NAPI_PREAMBLE(env);

@ronag
Copy link
Member Author

ronag commented Aug 21, 2024

napi_create_buffer_from_array_buffer or something?

@shubhamsugara22
Copy link

Though the same name regarding the other 2 points have any idea ?

@gabrielschulhof
Copy link
Contributor

Please use

napi_status node_api_create*

That is, the prefix should not be napi_ for new APIs, but node_api_.

@RedYetiDev RedYetiDev moved this from Triaged to In Progress in Node.js feature requests Sep 13, 2024
nodejs-github-bot pushed a commit that referenced this issue Oct 11, 2024
PR-URL: #54505
Fixes: #54440
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Vladimir Morozov <[email protected]>
@github-project-automation github-project-automation bot moved this from Has PR to Done in Node-API Team Project Oct 11, 2024
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
PR-URL: nodejs#54505
Fixes: nodejs#54440
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Vladimir Morozov <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
PR-URL: nodejs#54505
Fixes: nodejs#54440
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Vladimir Morozov <[email protected]>
ruyadorno pushed a commit that referenced this issue Nov 27, 2024
PR-URL: #54505
Fixes: #54440
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Vladimir Morozov <[email protected]>
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. node-api Issues and PRs related to the Node-API.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants