Load addons through Hermes' hermes_napi_load_module - #445
Merged
Conversation
The vendored Hermes ships a first-party addon loader that does what CxxNodeApiHostModule did by hand — dlopen, resolve the init function, create the exports object and call it — plus the deprecated napi_module_register fallback the host never implemented. Hand the platform specific path to it instead, and drop AddonLoaders.hpp along with the host's own loading and initialization code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX
The check workflow only re-evaluates its label conditions on opened, synchronize and reopened events, so the labels added after opening this PR need a push to take effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX
This was referenced Aug 13, 2026
kraenhansen
added a commit
that referenced
this pull request
Aug 13, 2026
…446) * Add a fixture registering via the deprecated napi_module_register The host gained support for addons that register themselves by calling napi_module_register while their library loads (#445), but nothing in the repo exercises that path — every other addon here exports napi_register_module_v1, which the loader finds first. This addon exports no such symbol, so it only loads if the fallback works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX * Trigger the label-gated CI jobs The check workflow only re-evaluates its label conditions on opened, synchronize and reopened events, so the labels added after opening this PR need a push to take effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX * Drop the gyp file from the module-register fixture Nothing builds this from binding.gyp — cmake-rn drives the CMake project directly. The sibling fixtures keep theirs to stay close to upstream sources they were derived from, which does not apply to an addon written here. CMakeLists.txt is now hand-maintained rather than regenerated by gyp-to-cmake, which skips the directory now that there is no binding.gyp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX * Require the addon directly instead of through bindings The bindings package earns its place when an addon has to be found across the several output directories node-gyp might have used. This addon is built by cmake-rn to one known location, so a plain require says the same thing with one less dependency — and it exercises the Babel plugin's ordinary require path rather than its bindings special case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ugFE6vmMUVMTuoupvhMhX --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The vendored Hermes ships a first-party addon loader we were not using. At the pinned commit,
API/napi/hermes_napi.hexposes:which does exactly what
CxxNodeApiHostModuledid by hand —dlopen(RTLD_NOW | RTLD_LOCAL), resolvenapi_register_module_v1, create anexportsobject, call the init function — plus the two things we never implemented: falling back to the module recorded by anapi_module_registercall, and reporting failures as real errors.This hands the platform specific path to it instead. The host keeps deciding what to load (library name →
@rpath/<name>.framework/<name>orlib<name>.so) and keeps creating onenapi_envper addon; Hermes does the loading and initializing.AddonLoaders.hppgoes with it — thePosixLoaderpolicy was its only user, and theWin32Loader/WinRTLoadervariants next to it were never referenced.Behavior changes
napi_module_registernow load. Previously the host only looked for anapi_register_module_v1export and, finding none, logged a debug line and resolved the require toundefined. See the note on verification below — this addresses Support module registration via calls tonapi_module_register#3, but I'd rather not close it until something exercises the path.requireNodeAddonthrows instead of resolving toundefined, with a message naming the addon, the path tried, and the underlying reason (thedlopenerror, or "nonapi_register_module_v1export and no registerednapi_module"). Related: Would be good to have a debug log for failed module resolution #369.node_api_get_module_file_namereports the path the addon was loaded from. It readsenv->moduleFileName_, which onlyhermes_napi_load_modulesets, so it returned an empty string for every addon before this.requireNodeAddonwith anything other than a single string now throws instead of resolving toundefined(the// TODO: Throw a meaningful errorthat sat in that branch).Handle scope
Loading and initializing now happens inside a
napi_open_handle_scope/napi_close_handle_scopepair.hermes_napi_load_modulerequires one, but this was already a latent bug: everynapi_valuein Hermes is a slot handed out bynapi_env__::addToCurrentScope, andnapi_env__::markHandleScopesreturns early when the scope stack is empty, so theexportsobject the host created on a fresh env and passed to the addon's init function was not a GC root. A collection triggered by the addon while it populatedexportscould free it. The env constructor pre-allocates the first handle block, so the empty-stack path did not read out of bounds — it just produced unrooted values (and tripped the"addToCurrentScope called with no open handle scope"assert in debug builds).Verification
Syntax-checked for
__APPLE__and__ANDROID__, debug and release, under-Wall -Wextra, against stub JSI/TurboModule headers — the host C++ test target only compilesHermesNapiHost.cppandLogger.cpp, so this file is not covered byhost-cpp-tests. The real check is the label-gated iOS/Android test app runs, hence the labels on this PR.Not verified: the deprecated
napi_module_registerpath, because no fixture in the repo uses it. #104 added one (node-addon-examples/.../2_function_arguments_twisted) that could be salvaged for this.Follow-ups, not in this PR
napi_env__::module_api_versionis fixed atNAPI_VERSION,hermes_napi_create_envtakes no version, andhermes_napi_load_modulereads neithernode_api_module_get_api_version_v1nornm_version. Detecting the version here is easy; there is nowhere to put the answer. Wants an issue onfacebook/hermes.requireNodeAddon(path)to support more use-cases #91, RefactorrequireNodeAddonto take the package name and extension-less path of an addon #222, and the 3-argumentrequireNodeAddonin Refactor C++ loader: extract cache and addon registry, resolve TODOs #104) is deliberately untouched here. Worth noting for whoever picks it up: the mangled name is a build artifact, not just a lookup key —getLibraryName()also driveslink-modules, which renames the.so/.frameworkto match — so any runtime-side mapping has to agree with the naming strategy the build used.Generated by Claude Code