From 2fae63a270220c430ffd086ae61e344627352663 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 07:32:30 +0000 Subject: [PATCH 1/2] Drop the host's shadowing implementations of runtime Node-API functions Hermes' first-party Node-API (adopted in #372, integrated via hermes_napi_host in #398) already implements the buffer functions, napi_get_version and napi_get_node_version. RuntimeNodeApi.{cpp,hpp} still defined all of these, and since the generated injector (scripts/generate-injector.mts) resolves each NodeApiHost field by unqualified name inside `namespace callstack::react_native_node_api`, the host's shims won and Hermes' implementations were never reached. Remove napi_create_buffer, napi_create_buffer_copy, napi_create_external_buffer, napi_get_buffer_info, napi_is_buffer, napi_get_version and napi_get_node_version from RuntimeNodeApi.{cpp,hpp}, letting unqualified lookup fall through to Hermes' own symbols. This also fixes two bugs the shims carried: - napi_get_buffer_info wrote its typed-array-kind output into a mutable global (`ArrayType`) that every subsequent napi_create_buffer / napi_create_external_buffer call read back, so calling it on e.g. a Float64Array corrupted every later buffer creation (and raced across runtimes). - napi_create_buffer_copy accepted `result_data` but never wrote it. napi_is_buffer / napi_get_buffer_info also become stricter, matching Node: true/napi_ok only for Uint8Array, napi_invalid_arg otherwise, instead of accepting any ArrayBuffer/TypedArray. Keep napi_fatal_error's host-side implementation: Hermes routes it to stderr, which is not logcat on Android, while the host's version reaches logcat via the "NodeApiHost" logger tag. Documented why this one intentionally keeps shadowing Hermes so a future sweep doesn't remove it as dead weight. RuntimeNodeApi.{cpp,hpp} keep their own translation unit rather than folding into Logger-adjacent code: the file now holds exactly the one shim the host deliberately keeps, and renaming would touch the injector, CMakeLists and podspec globbing for no functional benefit. Adds a changeset (patch) for the observable behavior change: addons now see Hermes' real napi_get_node_version instead of napi_generic_failure, and the stricter buffer type-checking. Closes #67. Verified: pnpm install && pnpm run build, pnpm --filter react-native-node-api run test (pre-existing failures only, confirmed present on unmodified origin/next too - they stem from running as root, not this change), eslint and prettier on touched files. Native C++ compilation was not verified - no Android/iOS toolchain is available in this environment. Closes #428 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm --- .changeset/drop-shadowing-napi-shims.md | 26 +++++ packages/host/cpp/RuntimeNodeApi.cpp | 138 +----------------------- packages/host/cpp/RuntimeNodeApi.hpp | 40 +++---- 3 files changed, 49 insertions(+), 155 deletions(-) create mode 100644 .changeset/drop-shadowing-napi-shims.md diff --git a/.changeset/drop-shadowing-napi-shims.md b/.changeset/drop-shadowing-napi-shims.md new file mode 100644 index 00000000..9c78c2d3 --- /dev/null +++ b/.changeset/drop-shadowing-napi-shims.md @@ -0,0 +1,26 @@ +--- +"react-native-node-api": patch +--- + +Drop the host's shadowing implementations of Node-API functions that Hermes' +first-party Node-API already provides, so addons observe Hermes' behavior +instead of the host's older shims: + +- `napi_get_node_version` now reports Hermes' own version (release name + `"hermes"`) instead of unconditionally failing with `napi_generic_failure`. +- `napi_is_buffer` now returns `true` only for `Uint8Array`, matching Node, + instead of any `ArrayBuffer`/`TypedArray`. +- `napi_get_buffer_info` now returns `napi_invalid_arg` for non-`Uint8Array` + values, matching Node, instead of `napi_ok` with zeroed output. +- `napi_create_buffer_copy` now writes a non-`NULL` `result_data` argument, as + documented, instead of silently ignoring it. +- `napi_create_buffer`, `napi_create_external_buffer` and `napi_get_version` + are unchanged in observable behavior, now served by Hermes directly. + +This also fixes a bug where calling `napi_get_buffer_info` on a non-`Uint8Array` +typed array (e.g. a `Float64Array`) left a process-global flag corrupted, so +that every subsequent `napi_create_buffer`/`napi_create_external_buffer` call +produced the wrong typed array view. + +`napi_fatal_error` keeps its host-side implementation, so fatal Node-API +errors keep reaching logcat on Android instead of only stderr. diff --git a/packages/host/cpp/RuntimeNodeApi.cpp b/packages/host/cpp/RuntimeNodeApi.cpp index c4e1c44c..880d904c 100644 --- a/packages/host/cpp/RuntimeNodeApi.cpp +++ b/packages/host/cpp/RuntimeNodeApi.cpp @@ -1,125 +1,12 @@ #include "RuntimeNodeApi.hpp" #include "Logger.hpp" -#include "Versions.hpp" -#include -auto ArrayType = napi_uint8_array; +#include namespace callstack::react_native_node_api { -napi_status napi_create_buffer(napi_env env, size_t length, void **data, - napi_value *result) { - napi_value buffer; - if (const auto status = napi_create_arraybuffer(env, length, data, &buffer); - status != napi_ok) { - return status; - } - - // Warning: The returned data structure does not fully align with the - // characteristics of a Buffer. - // @see - // https://github.com/callstackincubator/react-native-node-api/issues/171 - return napi_create_typedarray(env, ArrayType, length, buffer, 0, result); -} - -napi_status napi_create_buffer_copy(napi_env env, size_t length, - const void *data, void **result_data, - napi_value *result) { - if (!length || !data || !result) { - return napi_invalid_arg; - } - - void *buffer = nullptr; - if (const auto status = callstack::react_native_node_api::napi_create_buffer( - env, length, &buffer, result); - status != napi_ok) { - return status; - } - - std::memcpy(buffer, data, length); - return napi_ok; -} - -napi_status napi_is_buffer(napi_env env, napi_value value, bool *result) { - if (!result) { - return napi_invalid_arg; - } - - if (!value) { - *result = false; - return napi_ok; - } - - napi_valuetype type{}; - if (const auto status = napi_typeof(env, value, &type); status != napi_ok) { - return status; - } - - if (type != napi_object && type != napi_external) { - *result = false; - return napi_ok; - } - - auto isArrayBuffer{false}; - if (const auto status = napi_is_arraybuffer(env, value, &isArrayBuffer); - status != napi_ok) { - return status; - } - auto isTypedArray{false}; - if (const auto status = napi_is_typedarray(env, value, &isTypedArray); - status != napi_ok) { - return status; - } - - *result = isArrayBuffer || isTypedArray; - return napi_ok; -} - -napi_status napi_get_buffer_info(napi_env env, napi_value value, void **data, - size_t *length) { - if (!data || !length) { - return napi_invalid_arg; - } - *data = nullptr; - *length = 0; - if (!value) { - return napi_ok; - } - - auto isArrayBuffer{false}; - if (const auto status = napi_is_arraybuffer(env, value, &isArrayBuffer); - status == napi_ok && isArrayBuffer) { - return napi_get_arraybuffer_info(env, value, data, length); - } - - auto isTypedArray{false}; - if (const auto status = napi_is_typedarray(env, value, &isTypedArray); - status == napi_ok && isTypedArray) { - return napi_get_typedarray_info(env, value, &ArrayType, length, data, - nullptr, nullptr); - } - - return napi_ok; -} - -napi_status -napi_create_external_buffer(napi_env env, size_t length, void *data, - node_api_basic_finalize basic_finalize_cb, - void *finalize_hint, napi_value *result) { - napi_value buffer; - if (const auto status = napi_create_external_arraybuffer( - env, data, length, basic_finalize_cb, finalize_hint, &buffer); - status != napi_ok) { - return status; - } - - // Warning: The returned data structure does not fully align with the - // characteristics of a Buffer. - // @see - // https://github.com/callstackincubator/react-native-node-api/issues/171 - return napi_create_typedarray(env, ArrayType, length, buffer, 0, result); -} - +// See the comment on the declaration in RuntimeNodeApi.hpp for why this +// deliberately shadows Hermes' own napi_fatal_error. void napi_fatal_error(const char *location, size_t location_len, const char *message, size_t message_len) { if (location && location_len) { @@ -132,23 +19,4 @@ void napi_fatal_error(const char *location, size_t location_len, abort(); } -napi_status napi_get_node_version(node_api_basic_env env, - const napi_node_version **result) { - if (!result) { - return napi_invalid_arg; - } - - *result = nullptr; - return napi_generic_failure; -} - -napi_status napi_get_version(node_api_basic_env env, uint32_t *result) { - if (!result) { - return napi_invalid_arg; - } - - *result = NAPI_VERSION; - return napi_ok; -} - } // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/RuntimeNodeApi.hpp b/packages/host/cpp/RuntimeNodeApi.hpp index 1a5e62ea..0440027c 100644 --- a/packages/host/cpp/RuntimeNodeApi.hpp +++ b/packages/host/cpp/RuntimeNodeApi.hpp @@ -3,30 +3,30 @@ #include "node_api.h" namespace callstack::react_native_node_api { -napi_status napi_create_buffer(napi_env env, size_t length, void **data, - napi_value *result); - -napi_status napi_create_buffer_copy(napi_env env, size_t length, - const void *data, void **result_data, - napi_value *result); - -napi_status napi_is_buffer(napi_env env, napi_value value, bool *result); - -napi_status napi_get_buffer_info(napi_env env, napi_value value, void **data, - size_t *length); - -napi_status -napi_create_external_buffer(napi_env env, size_t length, void *data, - node_api_basic_finalize basic_finalize_cb, - void *finalize_hint, napi_value *result); +// Hermes' first-party Node-API implementation (API/napi/hermes_napi.cpp) +// already defines napi_fatal_error, routing it through hermes_fatal() -> +// llvh::report_fatal_error(), which writes to stderr. On Android stderr is +// not logcat, so that message would be lost exactly when it matters most: +// right before the process aborts. +// +// This declaration is deliberately kept so it shadows Hermes' symbol: the +// generated injector (scripts/generate-injector.mts) resolves each +// NodeApiHost field by unqualified name inside +// `namespace callstack::react_native_node_api`, and this header is included +// there, so unqualified lookup finds this declaration before it would reach +// Hermes' exported symbol. That lets the host's implementation win, which +// logs via the host logger to logcat with the "NodeApiHost" tag (see +// Logger.cpp) before aborting. +// +// Every other Node-API function the host used to shim here (buffers, +// napi_get_version, napi_get_node_version) was removed in favor of letting +// the same lookup fall through to Hermes' own implementation - see +// https://github.com/callstackincubator/react-native-node-api/issues/428. +// Do not remove this one the same way without replacing the logcat routing. void __attribute__((noreturn)) napi_fatal_error(const char *location, size_t location_len, const char *message, size_t message_len); -napi_status napi_get_node_version(node_api_basic_env env, - const napi_node_version **result); - -napi_status napi_get_version(node_api_basic_env env, uint32_t *result); } // namespace callstack::react_native_node_api From 3a7323bcce87b136bb75af9c9148ea93e8c47bed Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 08:17:35 +0000 Subject: [PATCH 2/2] =?UTF-8?q?ci:=20fix=20host-cpp-tests=20label=20check?= =?UTF-8?q?=20to=20match=20the=20actual=20"Host=20=F0=9F=8F=A1"=20label?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job checked for a label literally named "host", but the repository's real label (used on issues, e.g. #428/#420/#412) is "Host 🏡" — a label named plain "host" existed too, seemingly a leftover/duplicate, and has since been deleted. The condition never actually matched the label anyone would apply in practice, so this job only ever ran on pushes to main/next, never on a labeled PR. Found while attaching labels to this PR: the CI still showed green with host-cpp-tests silently not running, exactly the kind of gap .claude/CLAUDE.md (#436) exists to prevent. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm --- .github/workflows/check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 632bb32b..53cdb585 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -154,7 +154,7 @@ jobs: ctest --test-dir build --output-on-failure working-directory: packages/weak-node-api host-cpp-tests: - if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next' || contains(github.event.pull_request.labels.*.name, 'host') + if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next' || contains(github.event.pull_request.labels.*.name, 'Host 🏡') strategy: fail-fast: false matrix: