doc: clarify use of Uint8Array for n-api - #48742
Conversation
Sorry, something went wrong.
`napi_get_buffer_info` always supported receiving `Uint8Array` as a `value` argument because `node::Buffer` is a subclass of `Uint8Array` and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the `value` argument.
|
@nodejs/node-api lets discuss in the next weekly meeting |
Sorry, something went wrong.
|
In fact, napi_get_buffer_info supports any array buffer view as its argument. Compared to napi_get_typedarray_info, napi_get_buffer_info returns the byte length while napi_get_typedarray_info returns the element length (which should be identical when the bytes-per-element is 1). Are there any use cases that prefer napi_get_buffer_info to napi_get_typedarray_info on typed arrays that are not node::Buffer? |
Sorry, something went wrong.
|
@indutny we discussed a bit in the meeting today. Just waiting on the answer from you on: Are there any use cases that prefer napi_get_buffer_info to napi_get_typedarray_info on typed arrays that are not node::Buffer? |
Sorry, something went wrong.
|
Great news! Thanks!
Our use case comes through Neon wrapper for Rust addons which supports either JSArrayBuffer or JSBuffer: https://docs.rs/neon/latest/neon/types/buffer/trait.TypedArray.html . |
Sorry, something went wrong.
|
I've created a PR to remove usages on napi_get_buffer_info in node-addon-api c++ headers: nodejs/node-addon-api#1354. It can be replaced with napi_get_typedarray_info as inheriting Napi::Uint8Array. I think we can document that napi_get_typedarray_info can be used instead of napi_get_buffer_info. |
Sorry, something went wrong.
|
Amazing. Thank you! |
Sorry, something went wrong.
| * `[in] value`: The JavaScript value to check. | ||
| * `[out] result`: Whether the given `napi_value` represents a `node::Buffer` | ||
| object. | ||
| * `[out] result`: Whether the given `napi_value` represents a `node::Buffer` or |
There was a problem hiding this comment.
napi_is_typedarray should be preferred if the caller needs to check if the value is a Uint8Array.
Sorry, something went wrong.
There was a problem hiding this comment.
Committed! Thanks!
Sorry, something went wrong.
|
From my understading in the discussion in the Node-api team meeting today the preferred path forward is that this PR be updated to suggest that napi_get_typedarray_info be used instead for uint8arrays instead of indicating that napi_get_buffer_info supports them. This is because we'd prefer that people working with uint8arrays use napi_get_typearray_info versus encouraging more use of napi_get_buffer_info. @indutny are you ok with updating the PR along those lines? |
Sorry, something went wrong.
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
|
@mhdawson totally! If this is the direction you are more comfortable with - I'm happy to go with it. It definitely hurts most to have two different APIs for objects that are the same for most practical uses. Thank you! |
Sorry, something went wrong.
|
@indutny could you please fix the lint check? We can then land this. |
Sorry, something went wrong.
|
Done, sorry about that! |
Sorry, something went wrong.
`napi_get_buffer_info` always supported receiving `Uint8Array` as a `value` argument because `node::Buffer` is a subclass of `Uint8Array` and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the `value` argument. PR-URL: #48742 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
`napi_get_buffer_info` always supported receiving `Uint8Array` as a `value` argument because `node::Buffer` is a subclass of `Uint8Array` and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the `value` argument. PR-URL: #48742 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
`napi_get_buffer_info` always supported receiving `Uint8Array` as a `value` argument because `node::Buffer` is a subclass of `Uint8Array` and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the `value` argument. PR-URL: nodejs/node#48742 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
`napi_get_buffer_info` always supported receiving `Uint8Array` as a `value` argument because `node::Buffer` is a subclass of `Uint8Array` and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the `value` argument. PR-URL: nodejs/node#48742 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
napi_get_buffer_info always supported receiving Uint8Array as a value argument because node::Buffer is a subclass of Uint8Array and the underlying V8 APIs don't distinguish between two. With this change we mark both types as supported by the API so that the user code doesn't have to unknowingly use oficially unsupported type of the value argument.
Hello!
I think it'd be great to officially mark Uint8Arrays as supported since it is very likely that there is existing code in the wild that relies on this. The N-API never distinguished between node::Buffer and Uint8Array so it'd be a breaking change if we started to bail on Uint8Array anyway. Let's mark it as supported now to signify existing code as compliant to official APIs!