From 0a106e54757905cb61b97fa97fe18bf984c17279 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 21 May 2025 00:02:30 +0200 Subject: [PATCH 01/24] feat(babel): add 2 params to requireNodeAddon(); respect package entrypoints This big squashed change makes our Babel plugin to replace `require()` calls with `requireNodeAddons()` with 3 parameters, as initially described in https://github.com/callstackincubator/react-native-node-api-modules/issues/91 While squashing, I also removed my attempt to reimplement Node's resolution algorithm (https://nodejs.org/api/modules.html#all-together) and replaced it with a call to `require.resolve()`, which basically seems to do the same, just better with less code. This code has been tested with package that uses "main" entry point and has no JavaScript files. Finally, it commit will resolve the TODO regarding scanning the filesystem for addons when using `bindings`. Instead of globbing over the directories, I'm scanning the most popular directories where addons will be placed. Such list of directories was heavily influenced by `node-bindings` itself (https://github.com/TooTallNate/node-bindings/blob/v1.3.0/bindings.js#L21). --- .../src/node/babel-plugin/plugin.ts | 93 +++++++++++++++---- 1 file changed, 76 insertions(+), 17 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 2431af69..fefc9129 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -3,6 +3,8 @@ import path from "node:path"; import type { PluginObj, NodePath } from "@babel/core"; import * as t from "@babel/types"; +import { packageDirectorySync } from "pkg-dir"; +import { readPackageSync } from "read-pkg"; import { getLibraryName, isNodeApiModule, NamingStrategy } from "../path-utils"; @@ -20,12 +22,65 @@ function assertOptions(opts: unknown): asserts opts is PluginOptions { } } -export function replaceWithRequireNodeAddon( +// This function should work with both CommonJS and ECMAScript modules, +// (pretending that addons are supported with ES module imports), hence it +// must accept following import specifiers: +// - "Relative specifiers" (e.g. `./build/Release/addon.node`) +// - "Bare specifiers", in particular +// - to an entry point (e.g. `@callstack/example-addon`) +// - any specific exported feature within +// - "Absolute specifiers" like `node:fs/promise` and URLs. +// +// This function should also respect the Package entry points defined in the +// respective "package.json" file using "main" or "exports" and "imports" +// fields (including conditional exports and subpath imports). +// - https://nodejs.org/api/packages.html#package-entry-points +// - https://nodejs.org/api/packages.html#subpath-imports +function tryResolveModulePath(id: string, from: string): string | undefined { + if (id.includes(":")) { + // This must be a prefixed "Absolute specifier". We assume its a built-in + // module and pass it through without any changes. For security reasons, + // we don't support URLs to dynamic libraries (like Node-API addons). + return undefined; + } else { + // TODO: Stay compatible with https://nodejs.org/api/modules.html#all-together + try { + return require.resolve(id, { paths: [from] }); + } catch { + return undefined; + } + } +} + +const nodeBindingsSubdirs = [ + "./", + "./build/Release", + "./build/Debug", + "./build", + "./out/Release", + "./out/Debug", + "./Release", + "./Debug", +]; +function findNodeAddonForBindings(id: string, fromDir: string) { + +} + +export function replaceWithRequireNodeAddon3( p: NodePath, - modulePath: string, - naming: NamingStrategy + resolvedPath: string, + requiredFrom: string ) { - const requireCallArgument = getLibraryName(modulePath, naming); + const pkgRoot = packageDirectorySync({ cwd: resolvedPath }); + if (pkgRoot === undefined) { + throw new Error("Unable to locate package directory!"); + } + + const subpath = path.relative(pkgRoot, resolvedPath); + const dottedSubpath = subpath.startsWith("./") ? subpath : `./${subpath}`; + const fromRelative = path.relative(pkgRoot, requiredFrom); // TODO: might escape package + const { name } = readPackageSync({ cwd: pkgRoot }); + p.replaceWith( t.callExpression( t.memberExpression( @@ -34,7 +89,8 @@ export function replaceWithRequireNodeAddon( ]), t.identifier("requireNodeAddon") ), - [t.stringLiteral(requireCallArgument)] + [dottedSubpath, name, fromRelative] + .map(t.stringLiteral), ) ); } @@ -64,20 +120,23 @@ export function plugin(): PluginObj { const [argument] = p.parent.arguments; if (argument.type === "StringLiteral") { const id = argument.value; - const relativePath = path.join(from, id); - // TODO: Support traversing the filesystem to find the Node-API module - if (isNodeApiModule(relativePath)) { - replaceWithRequireNodeAddon(p.parentPath, relativePath, { - stripPathSuffix, - }); + const idWithExt = id.endsWith(".node") ? id : `${id}.node`; + // Support traversing the filesystem to find the Node-API module. + // Currently, we check the most common directories from `bindings`. + for (const subdir of nodeBindingsSubdirs) { + const resolvedPath = path.join(from, subdir, idWithExt); + if (isNodeApiModule(resolvedPath)) { + replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, this.filename); + break; + } } } - } else if ( - !path.isAbsolute(id) && - isNodeApiModule(path.join(from, id)) - ) { - const relativePath = path.join(from, id); - replaceWithRequireNodeAddon(p, relativePath, { stripPathSuffix }); + } else { + // This should handle "bare specifiers" and "private imports" that start with `#` + const resolvedPath = tryResolveModulePath(id, from); + if (!!resolvedPath && isNodeApiModule(resolvedPath)) { + replaceWithRequireNodeAddon3(p, resolvedPath, this.filename); + } } } }, From 67251250cef6a40caa16ae4deb52cdd32db6e81d Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 21 May 2025 00:39:18 +0200 Subject: [PATCH 02/24] refactor: extract `findNodeAddonForBindings()` --- .../src/node/babel-plugin/plugin.ts | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index fefc9129..5eb712fb 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -63,7 +63,16 @@ const nodeBindingsSubdirs = [ "./Debug", ]; function findNodeAddonForBindings(id: string, fromDir: string) { - + const idWithExt = id.endsWith(".node") ? id : `${id}.node`; + // Support traversing the filesystem to find the Node-API module. + // Currently, we check the most common directories like `bindings` does. + for (const subdir of nodeBindingsSubdirs) { + const resolvedPath = path.join(fromDir, subdir, idWithExt); + if (isNodeApiModule(resolvedPath)) { + return resolvedPath; + } + } + return undefined; } export function replaceWithRequireNodeAddon3( @@ -120,15 +129,9 @@ export function plugin(): PluginObj { const [argument] = p.parent.arguments; if (argument.type === "StringLiteral") { const id = argument.value; - const idWithExt = id.endsWith(".node") ? id : `${id}.node`; - // Support traversing the filesystem to find the Node-API module. - // Currently, we check the most common directories from `bindings`. - for (const subdir of nodeBindingsSubdirs) { - const resolvedPath = path.join(from, subdir, idWithExt); - if (isNodeApiModule(resolvedPath)) { - replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, this.filename); - break; - } + const resolvedPath = findNodeAddonForBindings(id, from); + if (resolvedPath !== undefined) { + replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, this.filename); } } } else { From c64891f16cdd631e48517a72b91ab9efd9bf446c Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 21 May 2025 00:59:06 +0200 Subject: [PATCH 03/24] feat: add shortcut to `isNodeApiModule()` If the path points to a file with a `.node` extension, then it must be a Node addon --- .../react-native-node-api-modules/src/node/path-utils.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index e09b31ac..4d852ea5 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -31,6 +31,13 @@ const packageNameCache = new Map(); * TODO: Consider checking for a specific platform extension. */ export function isNodeApiModule(modulePath: string): boolean { + // HACK: Take a shortcut (if applicable) + if (modulePath.endsWith('.node')) { + try { + fs.accessSync(modulePath); + return true; + } catch {} + } const dir = path.dirname(modulePath); const baseName = path.basename(modulePath, ".node"); let entries: string[]; From 2d17eb69b28a0e88cdb670457132ec6b7b2add77 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 21 May 2025 14:32:43 +0200 Subject: [PATCH 04/24] feat: relax the condition for CJS modules without file exts This small change relaxes the condition for taking the shortcut, as CommonJS modules (in contrast to ESM) do not require developers to explicitly include the file extensions. The Node.js module resolution algorithm (https://nodejs.org/api/modules.html#all-together) in step 4 of LOAD_AS_FILE(X) would try appending the `.node` extension. In theory, we should make sure that other extensions checked in previous steps are not present, but given that we are implementing it for `requireNodeAddon()`, it should be safe to skip those. --- .../react-native-node-api-modules/src/node/path-utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index 4d852ea5..9c2d3a21 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -31,10 +31,10 @@ const packageNameCache = new Map(); * TODO: Consider checking for a specific platform extension. */ export function isNodeApiModule(modulePath: string): boolean { - // HACK: Take a shortcut (if applicable) - if (modulePath.endsWith('.node')) { + { + // HACK: Take a shortcut (if applicable): existing `.node` files are addons try { - fs.accessSync(modulePath); + fs.accessSync(modulePath.endsWith(".node") ? modulePath : `${modulePath}.node`); return true; } catch {} } From 8bb3e772e7f15176c68d4c32a452ae7e9397216c Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 21 May 2025 14:36:10 +0200 Subject: [PATCH 05/24] style: address linter issues --- .../src/node/babel-plugin/plugin.ts | 3 +-- packages/react-native-node-api-modules/src/node/path-utils.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 5eb712fb..fa5646c5 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -6,7 +6,7 @@ import * as t from "@babel/types"; import { packageDirectorySync } from "pkg-dir"; import { readPackageSync } from "read-pkg"; -import { getLibraryName, isNodeApiModule, NamingStrategy } from "../path-utils"; +import { isNodeApiModule } from "../path-utils"; type PluginOptions = { stripPathSuffix?: boolean; @@ -109,7 +109,6 @@ export function plugin(): PluginObj { visitor: { CallExpression(p) { assertOptions(this.opts); - const { stripPathSuffix = false } = this.opts; if (typeof this.filename !== "string") { // This transformation only works when the filename is known return; diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index 9c2d3a21..5b7974e9 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -36,7 +36,7 @@ export function isNodeApiModule(modulePath: string): boolean { try { fs.accessSync(modulePath.endsWith(".node") ? modulePath : `${modulePath}.node`); return true; - } catch {} + } catch { /* empty */ } } const dir = path.dirname(modulePath); const baseName = path.basename(modulePath, ".node"); From 0ed525985e5688f95df4667aa9ee56457d25cf8a Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 12:19:43 +0200 Subject: [PATCH 06/24] feat: extract path normalization from `determineModuleContext()` --- .../src/node/path-utils.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index 5b7974e9..af5cbcc1 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -134,10 +134,23 @@ export function determineModuleContext( packageNameCache.set(pkgDir, pkgName); } // Compute module-relative path - const relPath = normalizeModulePath(path.relative(pkgDir, originalPath)); + const relPath = path.relative(pkgDir, originalPath); return { packageName: pkgName, relativePath: relPath }; } +/** + * Traverse the filesystem upward to find a name for the package that which contains a file. + * This variant normalizes the module path. + */ +export function determineNormalizedModuleContext( + modulePath: string, + originalPath = modulePath +): ModuleContext { + const { packageName, relativePath } = determineModuleContext(modulePath, originalPath); + const relPath = normalizeModulePath(relativePath); + return { packageName, relativePath: relPath }; +} + export function normalizeModulePath(modulePath: string) { const dirname = path.normalize(path.dirname(modulePath)); const basename = path.basename(modulePath); @@ -154,7 +167,7 @@ export function escapePath(modulePath: string) { * Get the name of the library which will be used when the module is linked in. */ export function getLibraryName(modulePath: string, naming: NamingStrategy) { - const { packageName, relativePath } = determineModuleContext(modulePath); + const { packageName, relativePath } = determineNormalizedModuleContext(modulePath); return naming.stripPathSuffix ? packageName : `${packageName}--${escapePath(relativePath)}`; From 4dcf2fdad170253618cde7746e7a2f5e6a64e3d3 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 12:25:38 +0200 Subject: [PATCH 07/24] feat: update tests to cover `determineNormalizedModuleContext()` --- .../src/node/path-utils.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/path-utils.test.ts b/packages/react-native-node-api-modules/src/node/path-utils.test.ts index 08a788ff..ad1f095e 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.test.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.test.ts @@ -6,6 +6,7 @@ import fswin from "fswin"; import { determineModuleContext, + determineNormalizedModuleContext, findNodeApiModulePaths, findPackageDependencyPaths, getLibraryName, @@ -135,14 +136,14 @@ describe("stripExtension", () => { }); }); -describe("determineModuleContext", () => { +describe("determineNormalizedModuleContext", () => { it("strips the file extension", (context) => { const tempDirectoryPath = setupTempDirectory(context, { "package.json": `{ "name": "my-package" }`, }); { - const { packageName, relativePath } = determineModuleContext( + const { packageName, relativePath } = determineNormalizedModuleContext( path.join(tempDirectoryPath, "some-dir/some-file.node") ); assert.equal(packageName, "my-package"); @@ -156,14 +157,16 @@ describe("determineModuleContext", () => { }); { - const { packageName, relativePath } = determineModuleContext( + const { packageName, relativePath } = determineNormalizedModuleContext( path.join(tempDirectoryPath, "some-dir/libsome-file.node") ); assert.equal(packageName, "my-package"); assert.equal(relativePath, "some-dir/some-file"); } }); +}); +describe("determineModuleContext", () => { it("resolves the correct package name", (context) => { const tempDirectoryPath = setupTempDirectory(context, { "package.json": `{ "name": "root-package" }`, @@ -177,7 +180,7 @@ describe("determineModuleContext", () => { path.join(tempDirectoryPath, "sub-package-a/some-file.node") ); assert.equal(packageName, "my-sub-package-a"); - assert.equal(relativePath, "some-file"); + assert.equal(relativePath, "some-file.node"); } { @@ -185,7 +188,7 @@ describe("determineModuleContext", () => { path.join(tempDirectoryPath, "sub-package-b/some-file.node") ); assert.equal(packageName, "my-sub-package-b"); - assert.equal(relativePath, "some-file"); + assert.equal(relativePath, "some-file.node"); } }); }); From 9bb9eec590e73b797eba54d8e05c2cbbb7978dd0 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 12:36:08 +0200 Subject: [PATCH 08/24] feat: add test for scoped package names --- .../src/node/path-utils.test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/react-native-node-api-modules/src/node/path-utils.test.ts b/packages/react-native-node-api-modules/src/node/path-utils.test.ts index ad1f095e..f1180030 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.test.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.test.ts @@ -167,7 +167,7 @@ describe("determineNormalizedModuleContext", () => { }); describe("determineModuleContext", () => { - it("resolves the correct package name", (context) => { + it("resolves the correct unscoped package name", (context) => { const tempDirectoryPath = setupTempDirectory(context, { "package.json": `{ "name": "root-package" }`, // Two sub-packages with the same name @@ -191,6 +191,31 @@ describe("determineModuleContext", () => { assert.equal(relativePath, "some-file.node"); } }); + + it("resolves the correct scoped package name", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "package.json": `{ "name": "root-package" }`, + // Two sub-packages with the same name + "sub-package-a/package.json": `{ "name": "@root-package/my-sub-package-a" }`, + "sub-package-b/package.json": `{ "name": "@root-package/my-sub-package-b" }`, + }); + + { + const { packageName, relativePath } = determineModuleContext( + path.join(tempDirectoryPath, "sub-package-a/some-file.node") + ); + assert.equal(packageName, "@root-package/my-sub-package-a"); + assert.equal(relativePath, "some-file.node"); + } + + { + const { packageName, relativePath } = determineModuleContext( + path.join(tempDirectoryPath, "sub-package-b/some-file.node") + ); + assert.equal(packageName, "@root-package/my-sub-package-b"); + assert.equal(relativePath, "some-file.node"); + } + }); }); describe("getLibraryName", () => { From 4154615b95a4936280d6936c47bdc91158ad72ba Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 13:55:23 +0200 Subject: [PATCH 09/24] feat: update tests for 3 arg `requireNodeAddon()` --- .../src/node/babel-plugin/plugin.test.ts | 75 ++++++------------- .../src/node/babel-plugin/plugin.ts | 2 +- 2 files changed, 23 insertions(+), 54 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 73e884aa..6d287e54 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -4,9 +4,8 @@ import path from "node:path"; import { transformFileSync } from "@babel/core"; -import { plugin } from "./plugin.js"; +import { plugin, type PluginOptions } from "./plugin.js"; import { setupTempDirectory } from "../test-utils.js"; -import { getLibraryName } from "../path-utils.js"; describe("plugin", () => { it("transforms require calls, regardless", (context) => { @@ -38,66 +37,36 @@ describe("plugin", () => { `, }); - const ADDON_1_REQUIRE_ARG = getLibraryName( - path.join(tempDirectoryPath, "addon-1"), - { stripPathSuffix: false } - ); - const ADDON_2_REQUIRE_ARG = getLibraryName( - path.join(tempDirectoryPath, "addon-2"), - { stripPathSuffix: false } - ); + const EXPECTED_PKG_NAME = "my-package"; - { - const result = transformFileSync( - path.join(tempDirectoryPath, "./addon-1.js"), - { plugins: [[plugin, { stripPathSuffix: false }]] } - ); - assert(result); - const { code } = result; - assert( - code && code.includes(`requireNodeAddon("${ADDON_1_REQUIRE_ARG}")`), - `Unexpected code: ${code}` - ); - } - - { - const result = transformFileSync( - path.join(tempDirectoryPath, "./addon-2.js"), - { plugins: [[plugin, { naming: "hash" }]] } - ); - assert(result); - const { code } = result; - assert( - code && code.includes(`requireNodeAddon("${ADDON_2_REQUIRE_ARG}")`), - `Unexpected code: ${code}` - ); - } - - { + type TestCaseParams = { + resolvedPath: string; + originalPath: string; + inputFile: string; + options?: PluginOptions; + }; + const runTestCase = ({ + resolvedPath, + originalPath, + inputFile, + options, + }: TestCaseParams) => { const result = transformFileSync( - path.join(tempDirectoryPath, "./sub-directory/addon-1.js"), - { plugins: [[plugin, { naming: "hash" }]] } + path.join(tempDirectoryPath, inputFile), + { plugins: [[plugin, options]] } ); assert(result); const { code } = result; assert( - code && code.includes(`requireNodeAddon("${ADDON_1_REQUIRE_ARG}")`), + code && code.includes(`requireNodeAddon("${resolvedPath}", "${EXPECTED_PKG_NAME}", "${originalPath}")`), `Unexpected code: ${code}` ); - } + }; - { - const result = transformFileSync( - path.join(tempDirectoryPath, "./addon-1-bindings.js"), - { plugins: [[plugin, { naming: "hash" }]] } - ); - assert(result); - const { code } = result; - assert( - code && code.includes(`requireNodeAddon("${ADDON_1_REQUIRE_ARG}")`), - `Unexpected code: ${code}` - ); - } + runTestCase({ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }); + runTestCase({ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js", options: { naming: "hash" } }); + runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js", options: { naming: "hash" } }); + runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js", options: { naming: "hash" } }); { const result = transformFileSync( diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index fa5646c5..bcfb543c 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -8,7 +8,7 @@ import { readPackageSync } from "read-pkg"; import { isNodeApiModule } from "../path-utils"; -type PluginOptions = { +export type PluginOptions = { stripPathSuffix?: boolean; }; From ee87dd0c5acbadf5cf9c65b5492a32917e12e3de Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 13:55:56 +0200 Subject: [PATCH 10/24] feat: remove xcframework dir from test code --- .../src/node/babel-plugin/plugin.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 6d287e54..209a02d0 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -11,9 +11,9 @@ describe("plugin", () => { it("transforms require calls, regardless", (context) => { const tempDirectoryPath = setupTempDirectory(context, { "package.json": `{ "name": "my-package" }`, - "addon-1.apple.node/addon-1.node": + "addon-1.node": "// This is supposed to be a binary file", - "addon-2.apple.node/addon-2.node": + "addon-2.node": "// This is supposed to be a binary file", "addon-1.js": ` const addon = require('./addon-1.node'); From a60c848bc04dcfe63281edca4a504fb7b655bc59 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 13:57:54 +0200 Subject: [PATCH 11/24] feat: change the third param to original path --- .../src/node/babel-plugin/plugin.ts | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index bcfb543c..afa0f932 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -3,10 +3,8 @@ import path from "node:path"; import type { PluginObj, NodePath } from "@babel/core"; import * as t from "@babel/types"; -import { packageDirectorySync } from "pkg-dir"; -import { readPackageSync } from "read-pkg"; -import { isNodeApiModule } from "../path-utils"; +import { determineModuleContext, isNodeApiModule } from "../path-utils"; export type PluginOptions = { stripPathSuffix?: boolean; @@ -78,17 +76,13 @@ function findNodeAddonForBindings(id: string, fromDir: string) { export function replaceWithRequireNodeAddon3( p: NodePath, resolvedPath: string, + originalPath: string, requiredFrom: string ) { - const pkgRoot = packageDirectorySync({ cwd: resolvedPath }); - if (pkgRoot === undefined) { - throw new Error("Unable to locate package directory!"); - } - - const subpath = path.relative(pkgRoot, resolvedPath); - const dottedSubpath = subpath.startsWith("./") ? subpath : `./${subpath}`; - const fromRelative = path.relative(pkgRoot, requiredFrom); // TODO: might escape package - const { name } = readPackageSync({ cwd: pkgRoot }); + const { packageName, relativePath } = determineModuleContext(resolvedPath); + const finalRelPath = relativePath.startsWith("./") + ? relativePath + : `./${relativePath}`; p.replaceWith( t.callExpression( @@ -98,7 +92,7 @@ export function replaceWithRequireNodeAddon3( ]), t.identifier("requireNodeAddon") ), - [dottedSubpath, name, fromRelative] + [finalRelPath, packageName, originalPath] .map(t.stringLiteral), ) ); @@ -130,14 +124,14 @@ export function plugin(): PluginObj { const id = argument.value; const resolvedPath = findNodeAddonForBindings(id, from); if (resolvedPath !== undefined) { - replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, this.filename); + replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, id, this.filename); } } } else { // This should handle "bare specifiers" and "private imports" that start with `#` const resolvedPath = tryResolveModulePath(id, from); if (!!resolvedPath && isNodeApiModule(resolvedPath)) { - replaceWithRequireNodeAddon3(p, resolvedPath, this.filename); + replaceWithRequireNodeAddon3(p, resolvedPath, id, this.filename); } } } From 41d4ebb4ae9396e94929f713457ffbb3d938678c Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 14:10:09 +0200 Subject: [PATCH 12/24] feat: add test case for require with "main" entry point --- .../src/node/babel-plugin/plugin.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 209a02d0..bb8e087e 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -81,4 +81,34 @@ describe("plugin", () => { ); } }); + + it("transforms require calls to packages with native entry point", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "node_modules/@scope/my-package/package.json": + `{ "name": "@scope/my-package", "main": "./build/Release/addon-1.node" }`, + "node_modules/@scope/my-package/build/Release/addon-1.node": + "// This is supposed to be a binary file", + "package.json": `{ "name": "my-consumer" }`, + "test.js": ` + const addon = require('@scope/my-package'); + console.log(addon); + ` + }); + + const EXPECTED_PKG_NAME = "@scope/my-package"; + const EXPECTED_PATH = "./build/Release/addon-1.node"; + + { + const result = transformFileSync( + path.join(tempDirectoryPath, "test.js"), + { plugins: [[plugin]] } + ); + assert(result); + const { code } = result; + assert( + code && code.includes(`requireNodeAddon("${EXPECTED_PATH}", "${EXPECTED_PKG_NAME}", "${EXPECTED_PKG_NAME}")`), + `Unexpected code: ${code}` + ); + }; + }); }); From 55962735a3b7ee4e4efd98af3f9d935b0fce9ab8 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 16:56:50 +0200 Subject: [PATCH 13/24] fix: remove unused `requiredFrom` parameter --- .../src/node/babel-plugin/plugin.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index afa0f932..6f13523a 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -76,8 +76,7 @@ function findNodeAddonForBindings(id: string, fromDir: string) { export function replaceWithRequireNodeAddon3( p: NodePath, resolvedPath: string, - originalPath: string, - requiredFrom: string + originalPath: string ) { const { packageName, relativePath } = determineModuleContext(resolvedPath); const finalRelPath = relativePath.startsWith("./") @@ -124,14 +123,14 @@ export function plugin(): PluginObj { const id = argument.value; const resolvedPath = findNodeAddonForBindings(id, from); if (resolvedPath !== undefined) { - replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, id, this.filename); + replaceWithRequireNodeAddon3(p.parentPath, resolvedPath, id); } } } else { // This should handle "bare specifiers" and "private imports" that start with `#` const resolvedPath = tryResolveModulePath(id, from); if (!!resolvedPath && isNodeApiModule(resolvedPath)) { - replaceWithRequireNodeAddon3(p, resolvedPath, id, this.filename); + replaceWithRequireNodeAddon3(p, resolvedPath, id); } } } From d23fe0b3b2195dd2309e72f501d49a5852e7543a Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 16:58:06 +0200 Subject: [PATCH 14/24] fix: remove unsupported babel option "naming" --- .../src/node/babel-plugin/plugin.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index bb8e087e..73e757d2 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -64,9 +64,9 @@ describe("plugin", () => { }; runTestCase({ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }); - runTestCase({ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js", options: { naming: "hash" } }); - runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js", options: { naming: "hash" } }); - runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js", options: { naming: "hash" } }); + runTestCase({ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" }); + runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }); + runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }); { const result = transformFileSync( From 63725af3988a8991befa8e38a7a1f51089eea3c1 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 17:20:04 +0200 Subject: [PATCH 15/24] feat: add tests for `findNodeAddonsForBindings()` --- .../src/node/babel-plugin/plugin.test.ts | 30 ++++++++++++++++++- .../src/node/babel-plugin/plugin.ts | 2 +- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 73e757d2..004cb762 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { transformFileSync } from "@babel/core"; -import { plugin, type PluginOptions } from "./plugin.js"; +import { plugin, findNodeAddonForBindings, type PluginOptions } from "./plugin.js"; import { setupTempDirectory } from "../test-utils.js"; describe("plugin", () => { @@ -112,3 +112,31 @@ describe("plugin", () => { }; }); }); + +describe("findNodeAddonForBindings()", () => { + it("should look for addons in common paths", (context) => { + // Arrange + const expectedPaths = { + "addon_1": "addon_1.node", + "addon_2": "build/Release/addon_2.node", + "addon_3": "build/Debug/addon_3.node", + "addon_4": "build/addon_4.node", + "addon_5": "out/Release/addon_5.node", + "addon_6": "out/Debug/addon_6.node", + "addon_7": "Release/addon_7.node", + "addon_8": "Debug/addon_8.node", + }; + const tempDirectoryPath = setupTempDirectory(context, + Object.fromEntries( + Object.values(expectedPaths) + .map((p) => [p, "// This is supposed to be a binary file"]) + ) + ); + // Act & Assert + Object.entries(expectedPaths).forEach(([name, relPath]) => { + const expectedPath = path.join(tempDirectoryPath, relPath); + const actualPath = findNodeAddonForBindings(name, tempDirectoryPath); + assert.equal(actualPath, expectedPath); + }); + }); +}); diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 6f13523a..09c1b560 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -60,7 +60,7 @@ const nodeBindingsSubdirs = [ "./Release", "./Debug", ]; -function findNodeAddonForBindings(id: string, fromDir: string) { +export function findNodeAddonForBindings(id: string, fromDir: string) { const idWithExt = id.endsWith(".node") ? id : `${id}.node`; // Support traversing the filesystem to find the Node-API module. // Currently, we check the most common directories like `bindings` does. From 25b55dac6d3a92e2503321636faf3333fec1e854 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 17:33:29 +0200 Subject: [PATCH 16/24] fix: ensure that module paths use only forward slash (esp. on Windows) --- .../src/node/babel-plugin/plugin.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 09c1b560..042c77db 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -79,9 +79,8 @@ export function replaceWithRequireNodeAddon3( originalPath: string ) { const { packageName, relativePath } = determineModuleContext(resolvedPath); - const finalRelPath = relativePath.startsWith("./") - ? relativePath - : `./${relativePath}`; + const relPath = relativePath.replaceAll("\\", "/"); + const finalRelPath = relPath.startsWith("./") ? relPath : `./${relPath}`; p.replaceWith( t.callExpression( From e5c28a7e50864489dc6fae0716609a49426afdff Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 4 Jun 2025 18:09:13 +0200 Subject: [PATCH 17/24] feat: add test case for addon that "escaped" its package In this particular case we don't want to get packageName="sub-package" with path "../addon-2.node" --- .../src/node/babel-plugin/plugin.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 004cb762..b3b9b2aa 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -27,6 +27,11 @@ describe("plugin", () => { const addon = require('../addon-1.node'); console.log(addon); `, + "sub-directory-3/package.json": `{ "name": "sub-package" }`, + "sub-directory-3/addon-outside.js": ` + const addon = require('../addon-2.node'); + console.log(addon); + `, "addon-1-bindings.js": ` const addon = require('bindings')('addon-1'); console.log(addon); @@ -66,6 +71,7 @@ describe("plugin", () => { runTestCase({ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }); runTestCase({ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" }); runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }); + runTestCase({ resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" }); runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }); { From 7106775b7e4e1267432cb17715bd8ba315631f7a Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 17:23:44 +0200 Subject: [PATCH 18/24] feat: make `resolvedPath` optional as suggested by Kraen --- .../src/node/babel-plugin/plugin.test.ts | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index b3b9b2aa..99b94f68 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -45,7 +45,7 @@ describe("plugin", () => { const EXPECTED_PKG_NAME = "my-package"; type TestCaseParams = { - resolvedPath: string; + resolvedPath?: string; originalPath: string; inputFile: string; options?: PluginOptions; @@ -62,10 +62,17 @@ describe("plugin", () => { ); assert(result); const { code } = result; - assert( - code && code.includes(`requireNodeAddon("${resolvedPath}", "${EXPECTED_PKG_NAME}", "${originalPath}")`), - `Unexpected code: ${code}` - ); + if (!resolvedPath) { + assert( + code && !code.includes(`requireNodeAddon`), + `Unexpected code: ${code}` + ); + } else { + assert( + code && code.includes(`requireNodeAddon("${resolvedPath}", "${EXPECTED_PKG_NAME}", "${originalPath}")`), + `Unexpected code: ${code}` + ); + } }; runTestCase({ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }); @@ -73,19 +80,7 @@ describe("plugin", () => { runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }); runTestCase({ resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" }); runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }); - - { - const result = transformFileSync( - path.join(tempDirectoryPath, "./require-js-file.js"), - { plugins: [[plugin, { naming: "hash" }]] } - ); - assert(result); - const { code } = result; - assert( - code && !code.includes(`requireNodeAddon`), - `Unexpected code: ${code}` - ); - } + runTestCase({ resolvedPath: undefined, originalPath: "./addon-1.js", inputFile: "./require-js-file.js" }); }); it("transforms require calls to packages with native entry point", (context) => { From 47db30275c0cfd0750d632fdb9f93e60ed5d263f Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 17:58:07 +0200 Subject: [PATCH 19/24] feat: run each test case as separate call to `it()` --- .../src/node/babel-plugin/plugin.test.ts | 130 +++++++++--------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index 99b94f68..a1371d45 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -8,79 +8,79 @@ import { plugin, findNodeAddonForBindings, type PluginOptions } from "./plugin.j import { setupTempDirectory } from "../test-utils.js"; describe("plugin", () => { - it("transforms require calls, regardless", (context) => { - const tempDirectoryPath = setupTempDirectory(context, { - "package.json": `{ "name": "my-package" }`, - "addon-1.node": - "// This is supposed to be a binary file", - "addon-2.node": - "// This is supposed to be a binary file", - "addon-1.js": ` - const addon = require('./addon-1.node'); - console.log(addon); - `, - "addon-2.js": ` - const addon = require('./addon-2.node'); - console.log(addon); - `, - "sub-directory/addon-1.js": ` - const addon = require('../addon-1.node'); - console.log(addon); - `, - "sub-directory-3/package.json": `{ "name": "sub-package" }`, - "sub-directory-3/addon-outside.js": ` - const addon = require('../addon-2.node'); - console.log(addon); - `, - "addon-1-bindings.js": ` - const addon = require('bindings')('addon-1'); - console.log(addon); - `, - "require-js-file.js": ` - const addon = require('./addon-1.js'); - console.log(addon); - `, - }); - + describe("transforms require calls, regardless", () => { const EXPECTED_PKG_NAME = "my-package"; type TestCaseParams = { resolvedPath?: string; originalPath: string; inputFile: string; - options?: PluginOptions; - }; - const runTestCase = ({ - resolvedPath, - originalPath, - inputFile, - options, - }: TestCaseParams) => { - const result = transformFileSync( - path.join(tempDirectoryPath, inputFile), - { plugins: [[plugin, options]] } - ); - assert(result); - const { code } = result; - if (!resolvedPath) { - assert( - code && !code.includes(`requireNodeAddon`), - `Unexpected code: ${code}` - ); - } else { - assert( - code && code.includes(`requireNodeAddon("${resolvedPath}", "${EXPECTED_PKG_NAME}", "${originalPath}")`), - `Unexpected code: ${code}` - ); - } }; - runTestCase({ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }); - runTestCase({ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" }); - runTestCase({ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }); - runTestCase({ resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" }); - runTestCase({ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }); - runTestCase({ resolvedPath: undefined, originalPath: "./addon-1.js", inputFile: "./require-js-file.js" }); + ([ + { resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }, + { resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" }, + { resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }, + { resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" }, + { resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }, + { resolvedPath: undefined, originalPath: "./addon-1.js", inputFile: "./require-js-file.js" }, + ] as TestCaseParams[]).forEach(({ resolvedPath, originalPath, inputFile }) => { + const expectedMessage = resolvedPath + ? `transform to requireNodeAddon() with "${resolvedPath}"` + : "NOT transform to requireNodeAddon()"; + + it(`${inputFile} should ${expectedMessage}`, (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "package.json": `{ "name": "${EXPECTED_PKG_NAME}" }`, + "addon-1.node": + "// This is supposed to be a binary file", + "addon-2.node": + "// This is supposed to be a binary file", + "addon-1.js": ` + const addon = require('./addon-1.node'); + console.log(addon); + `, + "addon-2.js": ` + const addon = require('./addon-2.node'); + console.log(addon); + `, + "sub-directory/addon-1.js": ` + const addon = require('../addon-1.node'); + console.log(addon); + `, + "sub-directory-3/package.json": `{ "name": "sub-package" }`, + "sub-directory-3/addon-outside.js": ` + const addon = require('../addon-2.node'); + console.log(addon); + `, + "addon-1-bindings.js": ` + const addon = require('bindings')('addon-1'); + console.log(addon); + `, + "require-js-file.js": ` + const addon = require('./addon-1.js'); + console.log(addon); + `, + }); + const result = transformFileSync( + path.join(tempDirectoryPath, inputFile), + { plugins: [[plugin, {}]] } + ); + assert(result); + const { code } = result; + if (!resolvedPath) { + assert( + code && !code.includes(`requireNodeAddon`), + `Unexpected code: ${code}` + ); + } else { + assert( + code && code.includes(`requireNodeAddon("${resolvedPath}", "${EXPECTED_PKG_NAME}", "${originalPath}")`), + `Unexpected code: ${code}` + ); + } + }); + }); }); it("transforms require calls to packages with native entry point", (context) => { From 8e1c278b63a39742c7526bd8002ba06b12dd05fe Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 18:16:45 +0200 Subject: [PATCH 20/24] chore: move `findNodeAddonForBindings()` to path-utils --- .../src/node/babel-plugin/plugin.test.ts | 30 +------------------ .../src/node/babel-plugin/plugin.ts | 29 ++++-------------- .../src/node/path-utils.test.ts | 29 ++++++++++++++++++ .../src/node/path-utils.ts | 26 ++++++++++++++++ 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index a1371d45..b1a59d6f 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { transformFileSync } from "@babel/core"; -import { plugin, findNodeAddonForBindings, type PluginOptions } from "./plugin.js"; +import { plugin } from "./plugin.js"; import { setupTempDirectory } from "../test-utils.js"; describe("plugin", () => { @@ -113,31 +113,3 @@ describe("plugin", () => { }; }); }); - -describe("findNodeAddonForBindings()", () => { - it("should look for addons in common paths", (context) => { - // Arrange - const expectedPaths = { - "addon_1": "addon_1.node", - "addon_2": "build/Release/addon_2.node", - "addon_3": "build/Debug/addon_3.node", - "addon_4": "build/addon_4.node", - "addon_5": "out/Release/addon_5.node", - "addon_6": "out/Debug/addon_6.node", - "addon_7": "Release/addon_7.node", - "addon_8": "Debug/addon_8.node", - }; - const tempDirectoryPath = setupTempDirectory(context, - Object.fromEntries( - Object.values(expectedPaths) - .map((p) => [p, "// This is supposed to be a binary file"]) - ) - ); - // Act & Assert - Object.entries(expectedPaths).forEach(([name, relPath]) => { - const expectedPath = path.join(tempDirectoryPath, relPath); - const actualPath = findNodeAddonForBindings(name, tempDirectoryPath); - assert.equal(actualPath, expectedPath); - }); - }); -}); diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 042c77db..edaf4db1 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -4,7 +4,11 @@ import path from "node:path"; import type { PluginObj, NodePath } from "@babel/core"; import * as t from "@babel/types"; -import { determineModuleContext, isNodeApiModule } from "../path-utils"; +import { + determineModuleContext, + isNodeApiModule, + findNodeAddonForBindings, +} from "../path-utils"; export type PluginOptions = { stripPathSuffix?: boolean; @@ -50,29 +54,6 @@ function tryResolveModulePath(id: string, from: string): string | undefined { } } -const nodeBindingsSubdirs = [ - "./", - "./build/Release", - "./build/Debug", - "./build", - "./out/Release", - "./out/Debug", - "./Release", - "./Debug", -]; -export function findNodeAddonForBindings(id: string, fromDir: string) { - const idWithExt = id.endsWith(".node") ? id : `${id}.node`; - // Support traversing the filesystem to find the Node-API module. - // Currently, we check the most common directories like `bindings` does. - for (const subdir of nodeBindingsSubdirs) { - const resolvedPath = path.join(fromDir, subdir, idWithExt); - if (isNodeApiModule(resolvedPath)) { - return resolvedPath; - } - } - return undefined; -} - export function replaceWithRequireNodeAddon3( p: NodePath, resolvedPath: string, diff --git a/packages/react-native-node-api-modules/src/node/path-utils.test.ts b/packages/react-native-node-api-modules/src/node/path-utils.test.ts index f1180030..211e29c9 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.test.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.test.ts @@ -7,6 +7,7 @@ import fswin from "fswin"; import { determineModuleContext, determineNormalizedModuleContext, + findNodeAddonForBindings, findNodeApiModulePaths, findPackageDependencyPaths, getLibraryName, @@ -400,3 +401,31 @@ describe("determineModuleContext", () => { assert.equal(readCount, 1); }); }); + +describe("findNodeAddonForBindings()", () => { + it("should look for addons in common paths", (context) => { + // Arrange + const expectedPaths = { + "addon_1": "addon_1.node", + "addon_2": "build/Release/addon_2.node", + "addon_3": "build/Debug/addon_3.node", + "addon_4": "build/addon_4.node", + "addon_5": "out/Release/addon_5.node", + "addon_6": "out/Debug/addon_6.node", + "addon_7": "Release/addon_7.node", + "addon_8": "Debug/addon_8.node", + }; + const tempDirectoryPath = setupTempDirectory(context, + Object.fromEntries( + Object.values(expectedPaths) + .map((p) => [p, "// This is supposed to be a binary file"]) + ) + ); + // Act & Assert + Object.entries(expectedPaths).forEach(([name, relPath]) => { + const expectedPath = path.join(tempDirectoryPath, relPath); + const actualPath = findNodeAddonForBindings(name, tempDirectoryPath); + assert.equal(actualPath, expectedPath); + }); + }); +}); \ No newline at end of file diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index af5cbcc1..a499b717 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -404,3 +404,29 @@ export function getLatestMtime(fromPath: string): number { return latest; } + +// NOTE: List of paths influenced by `node-bindings` itself +// https://github.com/TooTallNate/node-bindings/blob/v1.3.0/bindings.js#L21 +const nodeBindingsSubdirs = [ + "./", + "./build/Release", + "./build/Debug", + "./build", + "./out/Release", + "./out/Debug", + "./Release", + "./Debug", +]; + +export function findNodeAddonForBindings(id: string, fromDir: string) { + const idWithExt = id.endsWith(".node") ? id : `${id}.node`; + // Support traversing the filesystem to find the Node-API module. + // Currently, we check the most common directories like `bindings` does. + for (const subdir of nodeBindingsSubdirs) { + const resolvedPath = path.join(fromDir, subdir, idWithExt); + if (isNodeApiModule(resolvedPath)) { + return resolvedPath; + } + } + return undefined; +} \ No newline at end of file From 4aa85491ff9696dd025974250cbf55533a8d695e Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 18:21:58 +0200 Subject: [PATCH 21/24] chore: rename `originalPath` parameter to `originalId` --- .../src/node/babel-plugin/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index edaf4db1..16b6706d 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -57,7 +57,7 @@ function tryResolveModulePath(id: string, from: string): string | undefined { export function replaceWithRequireNodeAddon3( p: NodePath, resolvedPath: string, - originalPath: string + originalId: string ) { const { packageName, relativePath } = determineModuleContext(resolvedPath); const relPath = relativePath.replaceAll("\\", "/"); @@ -71,7 +71,7 @@ export function replaceWithRequireNodeAddon3( ]), t.identifier("requireNodeAddon") ), - [finalRelPath, packageName, originalPath] + [finalRelPath, packageName, originalId] .map(t.stringLiteral), ) ); From a431e8efc5920928c55cd5cd47572fb18cdae0b3 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 21:33:16 +0200 Subject: [PATCH 22/24] chore: replace `[].forEach()` with for-of --- .../src/node/babel-plugin/plugin.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts index b1a59d6f..c95b15d0 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts @@ -17,14 +17,15 @@ describe("plugin", () => { inputFile: string; }; - ([ + const testCases: ReadonlyArray = [ { resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" }, { resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" }, { resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" }, { resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" }, { resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" }, { resolvedPath: undefined, originalPath: "./addon-1.js", inputFile: "./require-js-file.js" }, - ] as TestCaseParams[]).forEach(({ resolvedPath, originalPath, inputFile }) => { + ]; + for (const { resolvedPath, originalPath, inputFile } of testCases) { const expectedMessage = resolvedPath ? `transform to requireNodeAddon() with "${resolvedPath}"` : "NOT transform to requireNodeAddon()"; @@ -80,7 +81,7 @@ describe("plugin", () => { ); } }); - }); + } }); it("transforms require calls to packages with native entry point", (context) => { From 446b1a4e7d7ee4510897eacf97e5001f57b9fff5 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 21:43:30 +0200 Subject: [PATCH 23/24] feat: move path slash normalization to determineModuleContext() --- .../src/node/babel-plugin/plugin.ts | 3 +-- packages/react-native-node-api-modules/src/node/path-utils.ts | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index 16b6706d..e6e2fb70 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -60,8 +60,7 @@ export function replaceWithRequireNodeAddon3( originalId: string ) { const { packageName, relativePath } = determineModuleContext(resolvedPath); - const relPath = relativePath.replaceAll("\\", "/"); - const finalRelPath = relPath.startsWith("./") ? relPath : `./${relPath}`; + const finalRelPath = relativePath.startsWith("./") ? relativePath : `./${relativePath}`; p.replaceWith( t.callExpression( diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index a499b717..22c8f144 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -134,7 +134,8 @@ export function determineModuleContext( packageNameCache.set(pkgDir, pkgName); } // Compute module-relative path - const relPath = path.relative(pkgDir, originalPath); + const relPath = path.relative(pkgDir, originalPath) + .replaceAll("\\", "/"); return { packageName: pkgName, relativePath: relPath }; } From cdf13168e2e3076322ffd08c6c5aed0dbc058467 Mon Sep 17 00:00:00 2001 From: Mariusz Pasinski Date: Wed, 11 Jun 2025 21:44:30 +0200 Subject: [PATCH 24/24] chore: use more descriptive variable names --- .../src/node/babel-plugin/plugin.ts | 4 ++-- packages/react-native-node-api-modules/src/node/path-utils.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts index e6e2fb70..8b24afa3 100644 --- a/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts +++ b/packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts @@ -60,7 +60,7 @@ export function replaceWithRequireNodeAddon3( originalId: string ) { const { packageName, relativePath } = determineModuleContext(resolvedPath); - const finalRelPath = relativePath.startsWith("./") ? relativePath : `./${relativePath}`; + const dotRelativePath = relativePath.startsWith("./") ? relativePath : `./${relativePath}`; p.replaceWith( t.callExpression( @@ -70,7 +70,7 @@ export function replaceWithRequireNodeAddon3( ]), t.identifier("requireNodeAddon") ), - [finalRelPath, packageName, originalId] + [dotRelativePath, packageName, originalId] .map(t.stringLiteral), ) ); diff --git a/packages/react-native-node-api-modules/src/node/path-utils.ts b/packages/react-native-node-api-modules/src/node/path-utils.ts index 22c8f144..9f3b4734 100644 --- a/packages/react-native-node-api-modules/src/node/path-utils.ts +++ b/packages/react-native-node-api-modules/src/node/path-utils.ts @@ -134,9 +134,9 @@ export function determineModuleContext( packageNameCache.set(pkgDir, pkgName); } // Compute module-relative path - const relPath = path.relative(pkgDir, originalPath) + const relativePath = path.relative(pkgDir, originalPath) .replaceAll("\\", "/"); - return { packageName: pkgName, relativePath: relPath }; + return { packageName: pkgName, relativePath }; } /**