Add a fixture registering via the deprecated napi_module_register - #446
Merged
Conversation
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
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
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
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
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.
Follow-up to #445, which made the host load addons that register themselves by calling
napi_module_registerwhile their library loads. Nothing in the repo exercised that path — every other addon here exportsnapi_register_module_v1, which the loader finds first, so the fallback was never reached.tests/module-registerexports nonapi_register_module_v1symbol, so it only loads if the fallback works. Failure mode is a hard one:requireNodeAddonthrows "nonapi_register_module_v1export and no registerednapi_module", and the test fails.The constructor is hand-rolled:
because
node_api.hno longer ships a macro that emits one —NAPI_MODULE_X, the historical spelling, is now an alias of the symbol-basedNAPI_MODULE. #104 solved this by vendoring a 269-linenode_api_deprecated.h; four lines seemed better.Closes #3.
Leaner than its siblings
The fixture has no
binding.gyp, no"gypfile": true, and nobindingsdependency.Nothing builds it from gyp —
cmake-rndrives the CMake project directly — and the reason the other fixtures carry a gyp file (staying close to the upstream sources they were derived from) does not apply to an addon written here.CMakeLists.txtis therefore hand-maintained;gyp-to-cmakeskips the directory, andgenerate-root-projectstill picks it up, since it looks for CMake projects rather than gyp files.bindingsearns its place when an addon has to be found across the several output directoriesnode-gypmight have used. This one is emitted to a single known location, soaddon.jsdoes a plainrequire("./build/RelWithDebInfo/addon.node")— matching whatferric-examplealready does — which has the side benefit of exercising the Babel plugin's ordinary require path rather than itsbindingsspecial case.The
"test": "node addon.js"script went too: without anode-gypbuild there is no host-loadable.nodefor it to require, so it could only ever have failed. Happy to put it back if you'd rather keep the shape uniform.Verification
nm -Dthat it exports neithernapi_register_module_v1nornode_api_module_get_api_version_v1— i.e. it really does force the fallback.process.dlopenand ran the assertion, so the fixture is well-formed independently of our host.addon.jsagainst a directory laid out like a completed build, and confirmed it rewrites torequire("react-native-node-api").requireNodeAddon("module-register-test--addon").generate-root-projectliststests/module-registeramong its sub-projects after the gyp file was removed.tsc --build,eslint,prettier --checkclean;pnpm installleaves the lockfile untouched.The fixture itself runs on device, so this needs the iOS and Android jobs — hence the labels.
Notes
@react-native-node-api/node-addon-examplesis private and no published package changes behavior.node_api_get_module_file_nameassertion I had initially added here. Hermes setsenv->moduleFileName_unconditionally inhermes_napi_load_module, so it holds on our host, but Node.js only populates it when loading throughrequire— viaprocess.dlopenit returns""for symbol-based and deprecated registration alike. An assertion that only holds in one of the two runtimes a fixture can run in seemed worse than no assertion. That claim from Load addons through Hermes'hermes_napi_load_module#445 stays unverified.lastRegisteredModuleafter a load. Node clears its equivalent (nodejs/node@a60056d) precisely so a failed or symbol-less load cannot pick up a stale registration from an earlier addon. Two deprecated addons in one app, or one that fails to load after another registered, can therefore resolve to the wrong init function. Worth folding into the upstream issue drafted in Support Node-API modules declaring their "Node-API version" #4.