Skip to content

node-api: add support for Float16Array #58879

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

node-api: add support for Float16Array #58879

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3d5bf56
node-api: add support for Float16Array
IlyasShabi Jun 28, 2025
4bff794
add changes section
IlyasShabi Jun 29, 2025
5bdffbb
fix order
IlyasShabi Jun 29, 2025
9559ac9
fix enum entries order
IlyasShabi Jun 29, 2025
5ff4dd4
check module version
IlyasShabi Jun 30, 2025
dc28ba5
fix typedarray tests
IlyasShabi Jul 3, 2025
cf84ea1
fix signature of typedarray finalize callback
IlyasShabi Jul 3, 2025
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
8 changes: 8 additions & 0 deletions doc/api/n-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2254,6 +2254,13 @@ object such that no properties can be set on it, and no prototype.

#### `napi_typedarray_type`

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/58879
description: Added `napi_float16_array` for Float16Array support.
-->

```c
typedef enum {
napi_int8_array,
Expand All @@ -2267,6 +2274,7 @@ typedef enum {
napi_float64_array,
napi_bigint64_array,
napi_biguint64_array,
napi_float16_array,
} napi_typedarray_type;
```

Expand Down
3 changes: 3 additions & 0 deletions src/js_native_api_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ typedef enum {
napi_float64_array,
napi_bigint64_array,
napi_biguint64_array,
#ifdef NAPI_EXPERIMENTAL
Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed this in the 11 July Node-API meeting.

In the past, we have added two enum values to napi_status that were not behind experimental guards: napi_no_external_buffers_allowed (ref) and napi_cannot_run_js (ref).

I'm not sure this should be guarded by an NAPI_EXPERIMENTAL flag.

Adding this under an experimental flag would mean that it is possible for napi_get_typedarray_info (which has no guard) can return an enum that I (if I do not define NAPI_EXPERIMENTAL) have no way of checking against.

Does it make sense to have this under a guard?

Copy link
Member

@vmoroz vmoroz Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@KevinEady , I believe for the correct versioning support we must use the experimental flag.
While we did not provide the experimental guard for the status values, we always used to do it for other enum types.
E.g. see the property attributes:

node/src/js_native_api_types.h

Lines 73 to 80 in a472745

#if NAPI_VERSION >= 8
// Default for class methods.
napi_default_method = napi_writable | napi_configurable,
// Default for object properties, like in JS obj[prop].
napi_default_jsproperty = napi_writable | napi_enumerable | napi_configurable,
#endif // NAPI_VERSION >= 8
} napi_property_attributes;

It is important that Node-API keeps a good versioning story and the ABI stability.

For the same reason the napi_get_typedarray_info behavior must be properly guarded.
E.g. today before this PR, the napi_get_typedarray_info has certain behavior and returns values that does not include the Float16 array type. We must ensure that this behavior is kept unchanged unless developers vote to use the newer API version where the behavior is changed.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should distinguish enum values used in in-parameters and out-parameters.

Unlike napi_property_attributes, napi_typedarray_type is used as an out-parameter in napi_get_typedarray_info. An addon should expect that JavaScript can introduce new typed array types, and it is inevitable to receive a new typed array as a parameter.

I think guarding napi_get_typedarray_info does not make sense. What error should an addon expect when napi_is_typedarray returns true, but napi_get_typedarray_info rejects it? In general, an addon should expect unknown enum (integer) values being returned from Node-API functions, and handles them (like, throwing an error).

I filed #59085 to discuss the ABI guarantee on enum values.

Copy link
Contributor

Choose a reason for hiding this comment

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

After codifying our rules for enums and ABI stability in #59085 , the version guard should be removed.

napi_float16_array,
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

#endif // NAPI_EXPERIMENTAL
} napi_typedarray_type;

typedef enum {
Expand Down
6 changes: 6 additions & 0 deletions src/js_native_api_v8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3165,6 +3165,10 @@ napi_status NAPI_CDECL napi_create_typedarray(napi_env env,
CREATE_TYPED_ARRAY(
env, BigUint64Array, 8, buffer, byte_offset, length, typedArray);
break;
case napi_float16_array:
CREATE_TYPED_ARRAY(
env, Float16Array, 2, buffer, byte_offset, length, typedArray);
break;
default:
return napi_set_last_error(env, napi_invalid_arg);
}
Expand Down Expand Up @@ -3203,6 +3207,8 @@ napi_status NAPI_CDECL napi_get_typedarray_info(napi_env env,
*type = napi_int32_array;
} else if (value->IsUint32Array()) {
*type = napi_uint32_array;
} else if (value->IsFloat16Array()) {
*type = napi_float16_array;
} else if (value->IsFloat32Array()) {
*type = napi_float32_array;
} else if (value->IsFloat64Array()) {
Expand Down
6 changes: 3 additions & 3 deletions test/js-native-api/test_typedarray/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ assert.strictEqual(externalResult[2], 2);
// Validate creation of all kinds of TypedArrays
const buffer = new ArrayBuffer(128);
const arrayTypes = [ Int8Array, Uint8Array, Uint8ClampedArray, Int16Array,
Uint16Array, Int32Array, Uint32Array, Float32Array,
Float64Array, BigInt64Array, BigUint64Array ];
Uint16Array, Int32Array, Uint32Array, Float16Array,
Copy link
Member

Choose a reason for hiding this comment

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

Note that the unit test is failing because the test binding.gyp must be changed to use the NAPI_EXPERIMENTAL macro. Otherwise, it is compiled with the latest stable Node-API version.
See:

node/test/js-native-api/test_finalizer/binding.gyp

Line 5 in c08a1d1

"defines": [ "NAPI_EXPERIMENTAL" ],
as example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the experimental guards will be removed, we would not need this define.

Float32Array, Float64Array, BigInt64Array, BigUint64Array ];

arrayTypes.forEach((currentType) => {
const template = Reflect.construct(currentType, buffer);
Expand All @@ -64,7 +64,7 @@ arrayTypes.forEach((currentType) => {
});

const nonByteArrayTypes = [ Int16Array, Uint16Array, Int32Array, Uint32Array,
Float32Array, Float64Array,
Float16Array, Float32Array, Float64Array,
BigInt64Array, BigUint64Array ];
nonByteArrayTypes.forEach((currentType) => {
const template = Reflect.construct(currentType, buffer);
Expand Down
Loading