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
+26 −4
Open

node-api: add support for Float16Array #58879

Show file tree
Hide file tree
Changes from all 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
4 changes: 4 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,10 @@ 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.

#define NODE_API_EXPERIMENTAL_HAS_FLOAT16_ARRAY
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
9 changes: 9 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,13 @@ 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:
if (env->module_api_version != NAPI_VERSION_EXPERIMENTAL) {
Copy link
Member

Choose a reason for hiding this comment

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

This bit in particular is odd – why the check here? Even if this feature is experimental, the enum value being used at all should be enough to imply that the caller has opted into this feature.

Copy link
Member

Choose a reason for hiding this comment

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

Right, it may look silly, but if we want to ensure predictable behavior between Node-API versions, then we must do it.
If we only focus on Node.js, and do not care much about versions, then you are right - it may be a bit of overhead.
But when you start to take into account support for other runtimes and JS engine, having a consistent and well-defined behavior becomes very important. To me this strict well-defined Node-API versioning is one of its best-selling points.

Say, someone creates a plugin that relies on the new Float16Array.
If we add it to Node.js directly without Node-API version, then this addon may work or not work in Deno or Bun depending on stars.
But if we explicitly say that Float16Array is the part of the experimental, and then version 11, then the Deno and Bun developers will be able to provide exactly the same contract and thus give the addon developers the best predictable experience.

Having the predictable behavior in this code file is also important for unit tests. Without versioning here, we cannot create reliable tests that can check the differences between versions. E.g. previously the unit test was successful even if the code relied on the experimental feature. Now it fails as expected because the addon must not use it yet.

Copy link
Member

Choose a reason for hiding this comment

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

depending on stars.

It depends on a very specific question though, not stars; namely, whether the runtime has support for creating Float16Array instances through Node-API. The behavior is predictable; either napi_create_typedarray() succeeds or it fails, and the caller should be able to handle both.

In any case, do what you must.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! We just follow the established process here.
We often see some completely unexpected side effects even from something that looks quite innocent.
Thus, it is better to be "defensive" about all new features and follow the "book". :)

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel strongly about adding a runtime version guard. IMO, adding an NAPI_EXPERIMENTAL and NODE_API_EXPERIMENTAL_HAS_FLOAT16_ARRAY around the definition should be sufficient, and addons can use NODE_API_EXPERIMENTAL_HAS_FLOAT16_ARRAY to guard if napi_float16_array is defined.

We added runtime version checks for features like finalizers, which can cause side effects. But for napi_create_typedarray, I think it should not cause side-effects.

return napi_set_last_error(env, napi_invalid_arg);
}
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 +3210,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()) {
Copy link
Member

Choose a reason for hiding this comment

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

Likewise here. Wrap the else if here in an #ifdef NAPI_EXPERIMENTAL

Copy link
Member

Choose a reason for hiding this comment

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

I support @jasnell point here. Since the napi_float16_array is under the experimental flag, we must not return that value for clients that do not use the experimental version.

Copy link
Member

Choose a reason for hiding this comment

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

Though I see the issue that previously we did not return any deterministic value at all.
I guess there must be an "else" statement that returns some value like std::numeric_limits<int32_t>::max().

*type = napi_float16_array;
} else if (value->IsFloat32Array()) {
*type = napi_float32_array;
} else if (value->IsFloat64Array()) {
Expand Down
1 change: 1 addition & 0 deletions test/js-native-api/test_typedarray/binding.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"targets": [
{
"target_name": "test_typedarray",
"defines": [ "NAPI_EXPERIMENTAL" ],
"sources": [
"test_typedarray.c"
]
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.

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
2 changes: 1 addition & 1 deletion test/js-native-api/test_typedarray/test_typedarray.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static napi_value Multiply(napi_env env, napi_callback_info info) {
return output_array;
}

static void FinalizeCallback(napi_env env,
static void FinalizeCallback(node_api_basic_env env,
void* finalize_data,
void* finalize_hint)
{
Expand Down
Loading