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 diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index abaf9342..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"; @@ -62,9 +63,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, { @@ -268,24 +269,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( @@ -367,6 +374,72 @@ 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("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", () => { diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index fd352e34..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\//, ]; @@ -327,35 +326,57 @@ 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)); 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); @@ -368,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, @@ -384,8 +406,7 @@ export async function findNodeApiModulePathsByDependency({ ), }, ] as const; - } - ) + }) ); // Return an object by dependency name return Object.fromEntries( 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; }