From bdfbe0f0c91f47a84fe1077e742de87d864779cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Jul 2025 20:50:51 +0200 Subject: [PATCH 1/5] Tread a failing opendir as an empty dir --- packages/host/src/node/path-utils.test.ts | 24 +++++++++++++++- packages/host/src/node/path-utils.ts | 34 +++++++++++++++-------- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index abaf9342..8f0392e7 100644 --- a/packages/host/src/node/path-utils.test.ts +++ b/packages/host/src/node/path-utils.test.ts @@ -62,9 +62,9 @@ describe("isNodeApiModule", () => { assert(isNodeApiModule(path.join(tempDirectoryPath, "addon.node"))); }); - // there is no way to set ACLs on directories in Node.js on Windows with brittle powershell commands it( "returns false when directory cannot be read due to permissions", + // Skipping on Windows because there is no way to set ACLs on directories in Node.js on Windows without brittle powershell commands { skip: process.platform === "win32" }, (context) => { const tempDirectoryPath = setupTempDirectory(context, { @@ -367,6 +367,28 @@ describe("findNodeApiModulePaths", () => { path.join(tempDir, "node_modules/root.apple.node"), ]); }); + + it( + "returns empty when directory cannot be read due to permissions", + // Skipping on Windows because there is no way to set ACLs on directories in Node.js on Windows without brittle powershell commands + { skip: process.platform === "win32" }, + async (context) => { + const tempDir = setupTempDirectory(context, { + "addon.apple.node/react-native-node-api-module": "", + }); + + removeReadPermissions(tempDir); + try { + const result = findNodeApiModulePaths({ + fromPath: tempDir, + platform: "apple", + }); + assert.deepEqual(await result, []); + } finally { + restoreReadPermissions(tempDir); + } + } + ); }); describe("determineModuleContext", () => { diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index fd352e34..f2e860d0 100644 --- a/packages/host/src/node/path-utils.ts +++ b/packages/host/src/node/path-utils.ts @@ -327,20 +327,32 @@ export async function findNodeApiModulePaths( const result: string[] = []; const pendingResults: Promise[] = []; - for await (const dirent of await fs.promises.opendir(candidatePath)) { + try { + for await (const dirent of await fs.promises.opendir(candidatePath)) { + if ( + dirent.isFile() && + dirent.name === MAGIC_FILENAME && + hasPlatformExtension(platform, candidatePath) + ) { + result.push(candidatePath); + } else if (dirent.isDirectory()) { + // Traverse into the child directory + // Pushing result into a list instead of awaiting immediately to parallelize the search + pendingResults.push( + findNodeApiModulePaths(options, path.join(suffix, dirent.name)) + ); + } + } + } catch (error) { if ( - dirent.isFile() && - dirent.name === MAGIC_FILENAME && - hasPlatformExtension(platform, candidatePath) + error instanceof Error && + "code" in error && + (error.code === "ENOENT" || error.code === "EACCES") ) { - result.push(candidatePath); - } else if (dirent.isDirectory()) { - // Traverse into the child directory - // Pushing result into a list instead of awaiting immediately to parallelize the search - pendingResults.push( - findNodeApiModulePaths(options, path.join(suffix, dirent.name)) - ); + // Gracefully handling issues with reading directories + return []; } + throw error; } const childResults = await Promise.all(pendingResults); result.push(...childResults.flatMap((filePath) => filePath)); From 606bb9ff9ce5d1370cb40695cbaa6c1acb1b447f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Jul 2025 20:51:43 +0200 Subject: [PATCH 2/5] Exclude specific packages --- packages/host/src/node/path-utils.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index f2e860d0..0ca2bfe9 100644 --- a/packages/host/src/node/path-utils.ts +++ b/packages/host/src/node/path-utils.ts @@ -279,7 +279,6 @@ export const MAGIC_FILENAME = "react-native-node-api-module"; * Default patterns to use when excluding paths from the search for Node-API modules. */ export const DEFAULT_EXCLUDE_PATTERNS = [ - /(^|\/)react-native-node-api\//, /(^|\/)node_modules\//, /(^|\/).git\//, ]; @@ -359,15 +358,25 @@ export async function findNodeApiModulePaths( return result; } +/** + * Default package names to use when excluding packages from the search for Node-API modules. + */ +export const DEFAULT_EXCLUDE_PACKAGES = [ + "react-native-node-api", // The host package itself + "react-native", // React Native core +]; + /** * Finds all dependencies of the app package and their xcframeworks. */ export async function findNodeApiModulePathsByDependency({ fromPath, includeSelf, + excludePackages = DEFAULT_EXCLUDE_PACKAGES, ...options }: FindNodeApiModuleOptions & { includeSelf: boolean; + excludePackages?: string[]; }) { // Find the location of each dependency const packagePathsByName = findPackageDependencyPaths(fromPath); @@ -380,8 +389,9 @@ export async function findNodeApiModulePathsByDependency({ // Find all their node api module paths const resultEntries = await Promise.all( - Object.entries(packagePathsByName).map( - async ([dependencyName, dependencyPath]) => { + Object.entries(packagePathsByName) + .filter(([name]) => !excludePackages.includes(name)) + .map(async ([dependencyName, dependencyPath]) => { // Make all the xcframeworks relative to the dependency path const absoluteModulePaths = await findNodeApiModulePaths({ fromPath: dependencyPath, @@ -396,8 +406,7 @@ export async function findNodeApiModulePathsByDependency({ ), }, ] as const; - } - ) + }) ); // Return an object by dependency name return Object.fromEntries( From f1720e275cdcbd2d43bcf3fa16e7b28e9697f398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Jul 2025 22:32:06 +0200 Subject: [PATCH 3/5] Add a changeset --- .changeset/stale-coins-drop.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/stale-coins-drop.md diff --git a/.changeset/stale-coins-drop.md b/.changeset/stale-coins-drop.md new file mode 100644 index 00000000..8d184cee --- /dev/null +++ b/.changeset/stale-coins-drop.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": patch +--- + +Treating failures when scanning filesystems for Node-API prebuilds more gracefully From 358dc16707bc7e4ce3f2409571b7d39816d14952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Jul 2025 11:12:06 +0200 Subject: [PATCH 4/5] Refactor setupTempDirectory to accept objects for dirs --- packages/host/src/node/path-utils.test.ts | 40 +++++++++++++---------- packages/host/src/node/test-utils.ts | 29 ++++++++++------ 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index 8f0392e7..c1cff20d 100644 --- a/packages/host/src/node/path-utils.test.ts +++ b/packages/host/src/node/path-utils.test.ts @@ -268,24 +268,30 @@ describe("getLibraryName", () => { describe("findPackageDependencyPaths", () => { it("should find package dependency paths", (context) => { const tempDir = setupTempDirectory(context, { - "node_modules/lib-a/package.json": JSON.stringify({ - name: "lib-a", - main: "index.js", - }), - "node_modules/lib-a/index.js": "", - "test-package/node_modules/lib-b/package.json": JSON.stringify({ - name: "lib-b", - main: "index.js", - }), - "test-package/node_modules/lib-b/index.js": "", - "test-package/package.json": JSON.stringify({ - name: "test-package", - dependencies: { - "lib-a": "^1.0.0", - "lib-b": "^1.0.0", + "node_modules/lib-a": { + "package.json": JSON.stringify({ + name: "lib-a", + main: "index.js", + }), + "index.js": "", + }, + "test-package": { + "package.json": JSON.stringify({ + name: "test-package", + dependencies: { + "lib-a": "^1.0.0", + "lib-b": "^1.0.0", + }, + }), + "src/index.js": "console.log('Hello, world!')", + "node_modules/lib-b": { + "package.json": JSON.stringify({ + name: "lib-b", + main: "index.js", + }), + "index.js": "", }, - }), - "test-package/src/index.js": "console.log('Hello, world!')", + }, }); const result = findPackageDependencyPaths( diff --git a/packages/host/src/node/test-utils.ts b/packages/host/src/node/test-utils.ts index 1876c724..3fd67386 100644 --- a/packages/host/src/node/test-utils.ts +++ b/packages/host/src/node/test-utils.ts @@ -3,23 +3,32 @@ import os from "node:os"; import fs from "node:fs"; import path from "node:path"; -export function setupTempDirectory( - context: TestContext, - files: Record -) { +export interface FileMap { + [key: string]: string | FileMap; +} + +function writeFiles(fromPath: string, files: FileMap) { + for (const [filePath, content] of Object.entries(files)) { + const fullPath = path.join(fromPath, filePath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + if (typeof content === "string") { + fs.writeFileSync(fullPath, content, "utf8"); + } else { + writeFiles(fullPath, content); + } + } +} + +export function setupTempDirectory(context: TestContext, files: FileMap) { const tempDirectoryPath = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), "babel-transform-test-")) + fs.mkdtempSync(path.join(os.tmpdir(), "react-native-node-api-test-")) ); context.after(() => { fs.rmSync(tempDirectoryPath, { recursive: true, force: true }); }); - for (const [filePath, content] of Object.entries(files)) { - const fullPath = path.join(tempDirectoryPath, filePath); - fs.mkdirSync(path.dirname(fullPath), { recursive: true }); - fs.writeFileSync(fullPath, content, "utf8"); - } + writeFiles(tempDirectoryPath, files); return tempDirectoryPath; } From 12e9c2083b198dbfded04d96e337daed3f2ca888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Jul 2025 11:12:23 +0200 Subject: [PATCH 5/5] Add simple test for findNodeApiModulePathsByDependency --- packages/host/src/node/path-utils.test.ts | 45 +++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index c1cff20d..3b1fafba 100644 --- a/packages/host/src/node/path-utils.test.ts +++ b/packages/host/src/node/path-utils.test.ts @@ -12,6 +12,7 @@ import { getLibraryName, isNodeApiModule, stripExtension, + findNodeApiModulePathsByDependency, } from "./path-utils.js"; import { setupTempDirectory } from "./test-utils.js"; @@ -397,6 +398,50 @@ describe("findNodeApiModulePaths", () => { ); }); +describe("findNodeApiModulePathsByDependency", () => { + it.only("should find Node-API paths by dependency (excluding certain packages)", async (context) => { + const packagesNames = ["lib-a", "lib-b", "lib-c"]; + const tempDir = setupTempDirectory(context, { + "app/package.json": JSON.stringify({ + name: "app", + dependencies: Object.fromEntries( + packagesNames.map((packageName) => [packageName, "^1.0.0"]) + ), + }), + ...Object.fromEntries( + packagesNames.map((packageName) => [ + `app/node_modules/${packageName}`, + { + "package.json": JSON.stringify({ + name: packageName, + main: "index.js", + }), + "index.js": "", + "addon.apple.node/react-native-node-api-module": "", + }, + ]) + ), + }); + + const result = await findNodeApiModulePathsByDependency({ + fromPath: path.join(tempDir, "app"), + platform: "apple", + includeSelf: false, + excludePackages: ["lib-a"], + }); + assert.deepEqual(result, { + "lib-b": { + path: path.join(tempDir, "app/node_modules/lib-b"), + modulePaths: ["addon.apple.node"], + }, + "lib-c": { + path: path.join(tempDir, "app/node_modules/lib-c"), + modulePaths: ["addon.apple.node"], + }, + }); + }); +}); + describe("determineModuleContext", () => { it("should read package.json only once across multiple module paths for the same package", (context) => { const tempDir = setupTempDirectory(context, {