node-api: add support for Float16Array #58879

IlyasShabi · 2025-06-28T22:52:08Z

This PR aims to add support for Float16Array in n-api, addressing this issue
cc: @nodejs/node-api

nodejs-github-bot · 2025-06-28T22:52:12Z

Review requested:

@nodejs/node-api

doc/api/n-api.md

codecov · 2025-06-28T23:48:57Z

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.09%. Comparing base (f0a9478) to head (5ff4dd4).
Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/js_native_api_v8.cc	50.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58879      +/-   ##
==========================================
- Coverage   90.09%   90.09%   -0.01%     
==========================================
  Files         640      640              
  Lines      188491   188433      -58     
  Branches    36969    36962       -7     
==========================================
- Hits       169823   169767      -56     
+ Misses      11399    11379      -20     
- Partials     7269     7287      +18

Files with missing lines	Coverage Δ
src/js_native_api_v8.cc	`74.81% <50.00%> (-1.75%)`	⬇️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

doc/api/n-api.md

VoltrexKeyva · 2025-06-29T15:16:46Z

test/js-native-api/test_typedarray/test.js

 const arrayTypes = [ Int8Array, Uint8Array, Uint8ClampedArray, Int16Array,
                     Uint16Array, Int32Array, Uint32Array, Float32Array,
-                     Float64Array, BigInt64Array, BigUint64Array ];
+                     Float64Array, BigInt64Array, BigUint64Array, Float16Array ];


Nit:
I think it would be better to add it before Float32Array for consistency.

@VoltrexKeyva, we must never add new enum entries in Node-API in the middle. It breaks the ABI compatibility rules.

I see, didn't know that, these suggestions should be reverted then.

VoltrexKeyva · 2025-06-29T15:16:58Z

test/js-native-api/test_typedarray/test.js

 const nonByteArrayTypes = [ Int16Array, Uint16Array, Int32Array, Uint32Array,
                            Float32Array, Float64Array,
-                            BigInt64Array, BigUint64Array ];
+                            BigInt64Array, BigUint64Array, Float16Array ];


VoltrexKeyva · 2025-06-29T15:18:11Z

doc/api/n-api.md

@@ -2267,6 +2274,7 @@ typedef enum {
  napi_float64_array,
  napi_bigint64_array,
  napi_biguint64_array,
+  napi_float16_array


VoltrexKeyva · 2025-06-29T15:18:42Z

src/js_native_api_types.h

@@ -105,6 +105,7 @@ typedef enum {
  napi_float64_array,
  napi_bigint64_array,
  napi_biguint64_array,
+  napi_float16_array,


VoltrexKeyva · 2025-06-29T15:18:59Z

src/js_native_api_v8.cc

@@ -3211,6 +3215,8 @@ napi_status NAPI_CDECL napi_get_typedarray_info(napi_env env,
      *type = napi_bigint64_array;
    } else if (value->IsBigUint64Array()) {
      *type = napi_biguint64_array;
+    } else if (value->IsFloat16Array()) {
+      *type = napi_float16_array;


vmoroz · 2025-06-29T17:41:29Z

src/js_native_api_types.h

@@ -101,6 +101,7 @@ typedef enum {
  napi_uint16_array,
  napi_int32_array,
  napi_uint32_array,
+  napi_float16_array,


It is important to note that Node-API is an ABI stable API. It means that any code that was compiled before with the previous version of Node-API must work in the future version of Node-API. See more details here: https://github.com/nodejs/node/blob/main/doc/contributing/adding-new-napi-api.md

By inserting a new enum value in the middle of the existing enum values we effectively change integer values for all enum values that follow the inserted entry. Any previously compiled code that used to create float32 array will suddenly create the float16 array, float64 became float32, etc.

The only valid place to insert new enum values in Node-API is at the end of an enum.

Also, any Node-API changes must be versioned and initially be part of the experimental version.
I.e. the new enum entry must be under #ifdef NAPI_EXPERIMENTAL. E.g. see this commit where the new Node-API property attributes were added.

@IlyasShabi , thank you for fixing the enum order!

Like @legendecas mentioned here #58889 (comment), we also need a new experiemental feature macro to be defined. As an example, please see this commit: 59e7444 and how it defined the NODE_API_EXPERIMENTAL_HAS_PROPERTY_KEYS macro. The macro will be deleted when the code becomes part of an official Node-API version after some "baking" time.

vmoroz

We need to ensure that the new Node-API changes follow the Node-API change requirements outlined in this document: https://github.com/nodejs/node/blob/main/doc/contributing/adding-new-napi-api.md

Note that the requirement that says "Should be implemented in at least one other VM implementation of Node.js." - it is currently being discussed to be revised towards something like "being implementable by other JS runtimes and JS engines" - see #58852. I.e. the new API like in this PR must be good as long as it is not V8-specific.

ABI breaking changes made in subsequent commits. Removing the approval until all issues can be resolved

jasnell · 2025-06-29T20:53:27Z

@IlyasShabi ... to address the ABI breaking issues, you might take some inspiration from this duplicate PR #58889

src/js_native_api_v8.cc

vmoroz · 2025-06-30T18:33:33Z

src/js_native_api.h

@@ -548,6 +548,7 @@ node_api_post_finalizer(node_api_basic_env env,
                        void* finalize_data,
                        void* finalize_hint);

+#define NODE_API_EXPERIMENTAL_HAS_FLOAT16_ARRAY


It is rather unexpected to see this macro defined here. It must be defined in the same place where you define the new enum value. E.g:

#ifdef NAPI_EXPERIMENTAL #define NODE_API_EXPERIMENTAL_HAS_FLOAT16_ARRAY napi_float16_array, #endif // NAPI_EXPERIMENTAL

addaleax

Feels quite silly to put this behind an experimental flag – this seems more of a "fix" to account for the fact that the language now exposes a new variant, rather than a new feature.

addaleax · 2025-06-30T18:39:07Z

src/js_native_api_v8.cc

@@ -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) {


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.

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.

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.

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". :)

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.

vmoroz · 2025-06-30T19:01:42Z

test/js-native-api/test_typedarray/test.js

@@ -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,


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.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Jun 28, 2025

github-project-automation bot added this to Node-API Team Project Jun 28, 2025

github-project-automation bot moved this to Need Triage in Node-API Team Project Jun 28, 2025

node-api: add support for Float16Array

3d5bf56

IlyasShabi changed the title ~~napi: add support for Float16Array~~ node-api: add support for Float16Array Jun 28, 2025

IlyasShabi force-pushed the ishabi/napi-float16array branch from 48a7303 to 3d5bf56 Compare June 28, 2025 22:54

addaleax reviewed Jun 28, 2025

View reviewed changes

doc/api/n-api.md Outdated Show resolved Hide resolved

add changes section

4bff794

IlyasShabi requested a review from addaleax June 29, 2025 11:26

jasnell reviewed Jun 29, 2025

View reviewed changes

doc/api/n-api.md Outdated Show resolved Hide resolved

jasnell previously approved these changes Jun 29, 2025

View reviewed changes

VoltrexKeyva reviewed Jun 29, 2025

View reviewed changes

fix order

5bdffbb

vmoroz reviewed Jun 29, 2025

View reviewed changes

vmoroz requested changes Jun 29, 2025

View reviewed changes

legendecas mentioned this pull request Jun 29, 2025

node-api: add napi_float16_array #58889

Open

fix enum entries order

9559ac9

jasnell reviewed Jun 29, 2025

View reviewed changes

src/js_native_api_v8.cc Show resolved Hide resolved

jasnell reviewed Jun 29, 2025

View reviewed changes

src/js_native_api_v8.cc Show resolved Hide resolved

check module version

5ff4dd4

vmoroz reviewed Jun 30, 2025

View reviewed changes

addaleax reviewed Jun 30, 2025

View reviewed changes

vmoroz reviewed Jun 30, 2025

View reviewed changes

Uh oh!

node-api: add support for Float16Array #58879

Are you sure you want to change the base?

node-api: add support for Float16Array #58879

Uh oh!

Conversation

IlyasShabi commented Jun 28, 2025

Uh oh!

nodejs-github-bot commented Jun 28, 2025

Uh oh!

Uh oh!

codecov bot commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

VoltrexKeyva Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vmoroz left a comment

Choose a reason for hiding this comment

Uh oh!

jasnell commented Jun 29, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jun 28, 2025 •

edited

Loading

VoltrexKeyva Jun 29, 2025 •

edited

Loading