Skip to content

Commit b101c95

Browse files
kraenhansenclaude
andcommitted
Phase 2: create a real Node-API env via hermes_napi_create_env
Replace the `env = nullptr` stub in CxxNodeApiHostModule with a real Node-API environment: cast the JSI runtime to `IHermes`, read the underlying `vm::Runtime*` via `getVMRuntimeUnsafe()`, and create the env with `hermes_napi_create_env(vm, nullptr)`. The env is owned by the runtime and cached on the module (shared across all addons). This flips the Phase 1 baseline abort (`assert(status == napi_ok)` right after `napi_create_object(env=nullptr, …)`) green: with `MOCHA_REMOTE_CONTEXT=allTests` the iOS-sim suite now reports 14 passing (node-addon-examples getting-started incl. the Rust ferric addon, buffers, async, and a js-native-api node-test). Linking note: the RN `hermesvm` framework already force-loads `hermesNapi`, so all `napi_*` runtime symbols are exported and `hermes_napi_create_env`'s code is present — but the `hermes_napi_*` public entry points lack the `NAPI_EXTERN` (visibility("default")) attribute the `napi_*` funcs carry, so Hermes' `-fvisibility=hidden` stripped them from the export table. `vendor-hermes` now annotates those declarations in `API/napi/hermes_napi.h` (idempotent) so a clean framework build exports them. This is an upstream oversight to report to Hermes; no force_load or second VM copy in our pod is needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent faaf32e commit b101c95

3 files changed

Lines changed: 75 additions & 2 deletions

File tree

packages/host/cpp/CxxNodeApiHostModule.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,19 @@
22
#include "Logger.hpp"
33
#include "RuntimeNodeApiAsync.hpp"
44

5+
#include <jsi/hermes-interfaces.h>
6+
57
using namespace facebook;
68

9+
// Declared by the vendored Hermes in API/napi/hermes_napi.h. We forward declare
10+
// it here (rather than including that header) to avoid pulling in Hermes' own
11+
// node_api.h alongside the weak-node-api copy already included transitively.
12+
// Only the mangled name matters for linking, so passing host as nullptr is
13+
// enough — async work / thread-safe functions will return failure until a
14+
// host integration is wired up (Phase 3).
15+
struct hermes_napi_host;
16+
napi_env hermes_napi_create_env(void *hermes_runtime, hermes_napi_host *host);
17+
718
namespace callstack::react_native_node_api {
819

920
CxxNodeApiHostModule::CxxNodeApiHostModule(
@@ -108,8 +119,25 @@ bool CxxNodeApiHostModule::initializeNodeModule(jsi::Runtime &rt,
108119
// TODO: Read the version from the addon
109120
// @see
110121
// https://github.com/callstackincubator/react-native-node-api/issues/4
111-
// TODO: Phase 2-3 will replace this with hermes_napi_create_env
112-
napi_env env = nullptr;
122+
123+
// Lazily create the Node-API environment backing this runtime. Hermes binds
124+
// an env to its low-level VM runtime, which we reach through the (unstable)
125+
// IHermes JSI interface. The env is owned by the runtime and shared across
126+
// all addons, so we create it once and cache it.
127+
if (env_ == nullptr) {
128+
// Fully qualified: `using namespace facebook` makes a bare `hermes`
129+
// ambiguous with the top-level `::hermes` (VM) namespace pulled in via
130+
// <jsi/hermes-interfaces.h>.
131+
auto *hermes = facebook::jsi::castInterface<facebook::hermes::IHermes>(&rt);
132+
if (hermes == nullptr) {
133+
log_debug("NapiHost: JSI runtime is not castable to IHermes; cannot "
134+
"create a Node-API environment");
135+
abort();
136+
}
137+
env_ = hermes_napi_create_env(hermes->getVMRuntimeUnsafe(), nullptr);
138+
assert(env_ != nullptr);
139+
}
140+
napi_env env = env_;
113141

114142
// Create the "exports" object
115143
napi_value exports;

packages/host/cpp/CxxNodeApiHostModule.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ class JSI_EXPORT CxxNodeApiHostModule : public facebook::react::TurboModule {
3030
std::unordered_map<std::string, NodeAddon> nodeAddons_;
3131
std::shared_ptr<facebook::react::CallInvoker> callInvoker_;
3232

33+
// The Node-API environment backing every addon loaded into this runtime.
34+
// Created lazily on first use from the underlying Hermes VM runtime and
35+
// shared across all addons (Node-API expects a single env per realm).
36+
napi_env env_ = nullptr;
37+
3338
using LoaderPolicy = PosixLoader; // FIXME: HACK: This is temporary workaround
3439
// for my lazyness (work on iOS and Android)
3540

packages/host/src/node/cli/hermes.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,42 @@ const platformOption = new Option(
2828
"The React Native package to vendor Hermes into",
2929
).default("react-native");
3030

31+
// The public `hermes_napi_*` entry points declared in `API/napi/hermes_napi.h`
32+
// lack the export annotation that the `napi_*` functions carry (`NAPI_EXTERN` ==
33+
// `visibility("default")`). Under Hermes' `-fvisibility=hidden` they are hidden
34+
// and stripped from the shared `hermesvm` framework, so embedders cannot link
35+
// `hermes_napi_create_env` even though its code is force-loaded into the
36+
// framework. Annotate them so they are exported.
37+
//
38+
// This is an upstream oversight (a public header entry point that isn't
39+
// exported) and should be reported to Hermes; until then we patch the vendored
40+
// checkout. The insertion is idempotent so re-running is safe.
41+
const EXPORTED_HERMES_NAPI_ENTRY_POINTS = [
42+
"hermes_napi_create_env",
43+
"hermes_napi_destroy_env",
44+
"hermes_napi_get_last_registered_module",
45+
"hermes_napi_load_module",
46+
];
47+
48+
async function exportHermesNapiEntryPoints(hermesPath: string) {
49+
const headerPath = path.join(hermesPath, "API", "napi", "hermes_napi.h");
50+
const original = await fs.promises.readFile(headerPath, "utf8");
51+
const patched = EXPORTED_HERMES_NAPI_ENTRY_POINTS.reduce(
52+
(contents, name) =>
53+
// A declaration is the only place the name is followed by "(" — doc
54+
// comments reference the names without parentheses. The negative
55+
// lookbehind keeps this idempotent.
56+
contents.replace(
57+
new RegExp(`(?<!NAPI_EXTERN )\\b${name}\\(`, "g"),
58+
`NAPI_EXTERN ${name}(`,
59+
),
60+
original,
61+
);
62+
if (patched !== original) {
63+
await fs.promises.writeFile(headerPath, patched);
64+
}
65+
}
66+
3167
export const command = new Command("vendor-hermes")
3268
.argument("[from]", "Path to a file inside the app package", process.cwd())
3369
.option("--silent", "Don't print anything except the final path", false)
@@ -115,6 +151,10 @@ export const command = new Command("vendor-hermes")
115151
});
116152
}
117153
}
154+
// Ensure the public Node-API entry points are exported from the shared
155+
// framework. Applied unconditionally (idempotent) so an existing checkout
156+
// from before this patch also gets fixed.
157+
await exportHermesNapiEntryPoints(hermesPath);
118158
console.log(hermesPath);
119159
}),
120160
);

0 commit comments

Comments
 (0)