Skip to content

Refactor C++ loader: extract cache and addon registry, resolve TODOs #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
240ae83
feat(babel): add 2 params to requireNodeAddon(); respect package entr…
May 20, 2025
8768c93
refactor: extract `findNodeAddonForBindings()`
May 20, 2025
bdc06cf
feat: add shortcut to `isNodeApiModule()`
May 20, 2025
fdaf8b5
feat: relax the condition for CJS modules without file exts
May 21, 2025
925d958
style: address linter issues
May 21, 2025
58746ae
feat: extract path normalization from `determineModuleContext()`
Jun 4, 2025
285b098
feat: update tests to cover `determineNormalizedModuleContext()`
Jun 4, 2025
e87a66e
feat: add test for scoped package names
Jun 4, 2025
798fed7
feat: update tests for 3 arg `requireNodeAddon()`
Jun 4, 2025
be9926a
feat: remove xcframework dir from test code
Jun 4, 2025
d5bf070
feat: change the third param to original path
Jun 4, 2025
2085fdf
feat: add test case for require with "main" entry point
Jun 4, 2025
3660b6f
fix: remove unused `requiredFrom` parameter
Jun 4, 2025
a414cd2
fix: remove unsupported babel option "naming"
Jun 4, 2025
dfeafbb
feat: add tests for `findNodeAddonsForBindings()`
Jun 4, 2025
6cfbc52
fix: ensure that module paths use only forward slash (esp. on Windows)
Jun 4, 2025
8bb6e01
feat: add test case for addon that "escaped" its package
Jun 4, 2025
eaa86a0
feat: make `resolvedPath` optional as suggested by Kraen
Jun 11, 2025
824c932
feat: run each test case as separate call to `it()`
Jun 11, 2025
1cdc092
chore: move `findNodeAddonForBindings()` to path-utils
Jun 11, 2025
f2bfc83
chore: rename `originalPath` parameter to `originalId`
Jun 11, 2025
64b967c
chore: replace `[].forEach()` with for-of
Jun 11, 2025
d7ad8a1
feat: move path slash normalization to determineModuleContext()
Jun 11, 2025
b0071ff
chore: use more descriptive variable names
Jun 11, 2025
e0b5139
feat: make requireNodeAddon() take 3 arguments
May 19, 2025
89612c4
fixup: remove single argument requireNodeAddon()
May 20, 2025
439e7a6
feat: add path utility functions
May 19, 2025
dec9035
feat: ensure that paths use safe ASCII alphanumericals
May 19, 2025
40a8791
feat: declare alias for custom resolver function
May 19, 2025
f9a4aae
feat: add support for custom prefix resolvers
May 19, 2025
9caedb8
feat: add support for custom package-specific resolvers
May 19, 2025
d7a2e3c
refactor: extract existing loading code to resolveRelativePath() method
May 19, 2025
3b7f209
feat: add `startsWith()` helper function
May 19, 2025
c90b8fc
feat: compute merged subpath and verify it before loading
May 19, 2025
d6b254f
fix: explicitly cast `pathPrefix` to `std::string` for lookup
May 19, 2025
063e5b5
fix: ensure `joinPath` is called on `requiredFrom`'s parent directory
May 19, 2025
e7fd0be
feat: add stub methods for interacting with require cache
May 20, 2025
68a4a2e
feat: make the `resolveRelativePath()` interact with (no-op) require …
May 20, 2025
4102043
feat: draft first implementation of AddonRegistry
May 21, 2025
b5b7bf3
refactor: swap old TM addon loader with AddonRegistry
May 21, 2025
2befad7
feat: support modules that register via `napi_module_register()`
May 21, 2025
3b30241
fixup: update params in NativeNodeApiHost spec
May 21, 2025
f4130a1
fix: implement proper extension stripping
Jun 9, 2025
6b441b5
feat: extract the "twisted" node api addon example from "wip-refactor…
Jun 18, 2025
a9f3be0
fix: include the generated (and corrected) CMakeList
Jun 18, 2025
d5bf7b4
chore: rename `requiredFrom` param to `originalId`
Jun 11, 2025
6179a4d
fixup: fix types in `endsWith` (use `string_views`)
Jun 11, 2025
437ba9b
fix: change param types of `startsWith` to `string_view`s
Jun 11, 2025
c224486
Merge branch 'mario/twisted-addon-example' into mario/wip-refactor-cpp
Jun 18, 2025
8cd8791
feat: add `2_function_arguments_twisted/napi` to list of examples
Jun 18, 2025
3481e82
fix: defer calling `napi_module_register()` until weak node api injec…
Jun 18, 2025
7d82be1
hack: apply workaround for "twisted" addon
Jun 20, 2025
e8ff65d
refactor: remove unused path utils
Jun 20, 2025
fc0a1a0
fix: assertion firing when loading deprecated way
Jun 20, 2025
04295cf
chore: remove redundant feature flag
Jun 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: rename requiredFrom param to originalId
  • Loading branch information
Mariusz Pasinski committed Jun 18, 2025
commit d5bf7b45e94bff37aee73ce1e65faa16455e16fd
41 changes: 16 additions & 25 deletions packages/host/cpp/CxxNodeApiHostModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
const jsi::Value args[], size_t count) {
auto &thisModule = static_cast<CxxNodeApiHostModule &>(turboModule);
if (3 == count) {
// Must be `requireNodeAddon(requiredPath: string, requiredPackageName: string, requiredFrom: string)`
// Must be `requireNodeAddon(requiredPath: string, requiredPackageName: string, originalId: string)`
return thisModule.requireNodeAddon(rt,
args[0].asString(rt),
args[1].asString(rt),
Expand All @@ -153,25 +153,22 @@ jsi::Value
CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
const jsi::String &requiredPath,
const jsi::String &requiredPackageName,
const jsi::String &requiredFrom) {
const jsi::String &originalId) {
return requireNodeAddon(rt,
requiredPath.utf8(rt),
requiredPackageName.utf8(rt),
requiredFrom.utf8(rt));
originalId.utf8(rt));
}

jsi::Value
CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
const std::string &requiredPath,
const std::string &requiredPackageName,
const std::string &requiredFrom) {
const std::string &originalId) {
// Ensure that user-supplied inputs contain only allowed characters
if (!isModulePathLike(requiredPath)) {
throw jsi::JSError(rt, "Invalid characters in `requiredPath`. Only ASCII alphanumerics are allowed.");
}
if (!isModulePathLike(requiredFrom)) {
throw jsi::JSError(rt, "Invalid characters in `requiredFrom`. Only ASCII alphanumerics are allowed.");
}

// Check if this is a prefixed import (e.g. `node:fs/promises`)
const auto [pathPrefix, strippedPath] = rpartition(requiredPath, ':');
Expand All @@ -180,7 +177,7 @@ CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
std::string pathPrefixCopy(pathPrefix); // HACK: Need explicit cast to `std::string`
if (auto handler = prefixResolvers_.find(pathPrefixCopy); prefixResolvers_.end() != handler) {
// HACK: Smuggle the `pathPrefix` as new `requiredPackageName`
return (handler->second)(rt, strippedPath, pathPrefix, requiredFrom);
return (handler->second)(rt, strippedPath, pathPrefix, originalId);
} else {
throw jsi::JSError(rt, "Unsupported protocol or prefix \"" + pathPrefixCopy + "\". Have you registered it?");
}
Expand All @@ -189,45 +186,39 @@ CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
// Check, if this package has been overridden
if (auto handler = packageOverrides_.find(requiredPackageName); packageOverrides_.end() != handler) {
// This package has a custom resolver, invoke it
return (handler->second)(rt, strippedPath, requiredPackageName, requiredFrom);
return (handler->second)(rt, strippedPath, requiredPackageName, originalId);
}

// Otherwise, "requiredPath" must be a "relative specifier" or a "bare specifier"
return resolveRelativePath(rt, strippedPath, requiredPackageName, requiredFrom);
// Otherwise, "requiredPath" must be a package-relative specifier
return resolveRelativePath(rt, strippedPath, requiredPackageName, originalId);
}

jsi::Value
CxxNodeApiHostModule::resolveRelativePath(facebook::jsi::Runtime &rt,
const std::string_view &requiredPath,
const std::string_view &requiredPackageName,
const std::string_view &requiredFrom) {
// "Rebase" the relative path to get a proper package-relative path
const auto requiredFromDirParts = makeParentPath(requiredFrom);
const auto requiredPathParts = explodePath(requiredPath);
const std::string mergedSubpath = implodePath(joinPath(requiredFromDirParts, requiredPathParts));
if (!isModulePathLike(mergedSubpath)) {
throw jsi::JSError(rt, "Computed subpath is invalid. Check `requiredPath` and `requiredFrom`.");
}
if (!startsWith(mergedSubpath, "./")) {
throw jsi::JSError(rt, "Subpath must be relative and cannot leave its package root.");
const std::string_view &originalId) {
if (!startsWith(requiredPath, "./")) {
throw jsi::JSError(rt, "requiredPath must be relative and cannot leave its package root.");
}

// Check whether (`requiredPackageName`, `mergedSubpath`) is already cached
// Check whether (`requiredPackageName`, `requiredPath`) is already cached
// NOTE: Cache must to be `jsi::Runtime`-local
auto [exports, isCached] = lookupRequireCache(rt,
requiredPackageName,
mergedSubpath);
requiredPath);

if (!isCached) {
// Ask the global addon registry to load given Node-API addon.
// If other runtime loaded it already, the OS will return the same pointer.
// NOTE: This method might try multiple platform-specific paths.
const std::string packageNameCopy(requiredPackageName);
auto &addon = g_platformAddonRegistry.loadAddon(packageNameCopy, mergedSubpath);
const std::string requiredPathCopy(requiredPath);
auto &addon = g_platformAddonRegistry.loadAddon(packageNameCopy, requiredPathCopy);

// Create a `napi_env` and initialize the addon
exports = g_platformAddonRegistry.instantiateAddonInRuntime(rt, addon);
updateRequireCache(rt, requiredPackageName, mergedSubpath, exports);
updateRequireCache(rt, requiredPackageName, requiredPath, exports);
}

return std::move(exports);
Expand Down
6 changes: 3 additions & 3 deletions packages/host/cpp/CxxNodeApiHostModule.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ class JSI_EXPORT CxxNodeApiHostModule : public facebook::react::TurboModule {
facebook::jsi::Value requireNodeAddon(facebook::jsi::Runtime &rt,
const facebook::jsi::String &requiredPath,
const facebook::jsi::String &requiredPackageName,
const facebook::jsi::String &requiredFrom);
const facebook::jsi::String &originalId);
facebook::jsi::Value requireNodeAddon(facebook::jsi::Runtime &rt,
const std::string &requiredPath,
const std::string &requiredPackageName,
const std::string &requiredFrom);
const std::string &originalId);

facebook::jsi::Value resolveRelativePath(facebook::jsi::Runtime &rt,
const std::string_view &requiredPath,
const std::string_view &requiredPackageName,
const std::string_view &requiredFrom);
const std::string_view &originalId);

std::pair<facebook::jsi::Value, bool>
lookupRequireCache(facebook::jsi::Runtime &rt,
Expand Down
2 changes: 1 addition & 1 deletion packages/host/src/react-native/NativeNodeApiHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { TurboModule } from "react-native";
import { TurboModuleRegistry } from "react-native";

export interface Spec extends TurboModule {
requireNodeAddon(requiredPath: string, packageName?: string, requiredFrom?: string): void;
requireNodeAddon(requiredPath: string, packageName?: string, originalId?: string): void;
}

export default TurboModuleRegistry.getEnforcing<Spec>("NodeApiHost");