Filter node_modules from package roots (#178)

* Fix findNodeApiModulePaths to exclude from root node_modules of the app itself

* Turn findNodeApiModulePaths async

* Add changeset

* Commit change to package-lock.json

Author: kraenhansen

.changeset/three-pugs-relate.md

@@ -0,0 +1,5 @@
+---
+"react-native-node-api": patch
+---
+
+Fix hasDuplicateLibraryNames by filtering out node_modules in package rootse

package-lock.json

@@ -61,17 +61,18 @@ export async function linkModules({
   linker,
 }: LinkModulesOptions): Promise<ModuleOutput[]> {
   // Find all their xcframeworks
-  const dependenciesByName = findNodeApiModulePathsByDependency({
+  const dependenciesByName = await findNodeApiModulePathsByDependency({
     fromPath,
     platform,
     includeSelf: true,
   });
 
   // Find absolute paths to xcframeworks
   const absoluteModulePaths = Object.values(dependenciesByName).flatMap(
-    (dependency) => dependency.modulePaths.map(
-      (modulePath) => path.join(dependency.path, modulePath)
-    )
+    (dependency) =>
+      dependency.modulePaths.map((modulePath) =>
+        path.join(dependency.path, modulePath)
+      )
   );
 
   if (hasDuplicateLibraryNames(absoluteModulePaths, naming)) {

packages/host/src/node/cli/link-modules.ts

@@ -169,7 +169,7 @@ program
   .addOption(pathSuffixOption)
   .action(async (fromArg, { json, pathSuffix }) => {
     const rootPath = path.resolve(fromArg);
-    const dependencies = findNodeApiModulePathsByDependency({
+    const dependencies = await findNodeApiModulePathsByDependency({
       fromPath: rootPath,
       platform: PLATFORMS,
       includeSelf: true,

packages/host/src/node/cli/program.ts

@@ -300,13 +300,13 @@ describe("findPackageDependencyPaths", () => {
 });
 
 describe("findNodeApiModulePaths", () => {
-  it("should find .apple.node paths", (context) => {
+  it("should find .apple.node paths", async (context) => {
     const tempDir = setupTempDirectory(context, {
       "root.apple.node/react-native-node-api-module": "",
       "sub-directory/lib-a.apple.node/react-native-node-api-module": "",
       "sub-directory/lib-b.apple.node/react-native-node-api-module": "",
     });
-    const result = findNodeApiModulePaths({
+    const result = await findNodeApiModulePaths({
       fromPath: tempDir,
       platform: "apple",
     });
@@ -317,14 +317,15 @@ describe("findNodeApiModulePaths", () => {
     ]);
   });
 
-  it("respects default exclude patterns", (context) => {
+  it("respects default exclude patterns", async (context) => {
     const tempDir = setupTempDirectory(context, {
       "root.apple.node/react-native-node-api-module": "",
+      "node_modules/dependency/lib.apple.node/react-native-node-api-module": "",
       "child-dir/dependency/lib.apple.node/react-native-node-api-module": "",
       "child-dir/node_modules/dependency/lib.apple.node/react-native-node-api-module":
         "",
     });
-    const result = findNodeApiModulePaths({
+    const result = await findNodeApiModulePaths({
       fromPath: tempDir,
       platform: "apple",
     });
@@ -334,14 +335,14 @@ describe("findNodeApiModulePaths", () => {
     ]);
   });
 
-  it("respects explicit exclude patterns", (context) => {
+  it("respects explicit exclude patterns", async (context) => {
     const tempDir = setupTempDirectory(context, {
       "root.apple.node/react-native-node-api-module": "",
       "child-dir/dependency/lib.apple.node/react-native-node-api-module": "",
       "child-dir/node_modules/dependency/lib.apple.node/react-native-node-api-module":
         "",
     });
-    const result = findNodeApiModulePaths({
+    const result = await findNodeApiModulePaths({
       fromPath: tempDir,
       platform: "apple",
       excludePatterns: [/root/],
@@ -352,13 +353,13 @@ describe("findNodeApiModulePaths", () => {
     ]);
   });
 
-  it("disregards parts futher up in filesystem when excluding", (context) => {
+  it("disregards parts futher up in filesystem when excluding", async (context) => {
     const tempDir = setupTempDirectory(context, {
       "node_modules/root.apple.node/react-native-node-api-module": "",
       "node_modules/child-dir/node_modules/dependency/lib.apple.node/react-native-node-api-module":
         "",
     });
-    const result = findNodeApiModulePaths({
+    const result = await findNodeApiModulePaths({
       fromPath: path.join(tempDir, "node_modules"),
       platform: "apple",
     });
@@ -415,13 +416,16 @@ describe("findNodeAddonForBindings()", () => {
   };
 
   for (const [name, relPath] of Object.entries(expectedPaths)) {
-    it(`should look for addons in common paths (${name} in "${relPath}")`, (context) => {
+    it(`should look for addons in common paths (${name} in "${relPath}")`, async (context) => {
       // Arrange
       const tempDirectoryPath = setupTempDirectory(context, {
         [relPath]: "// This is supposed to be a binary file",
       });
       // Act
-      const actualPath = findNodeAddonForBindings(name, tempDirectoryPath);
+      const actualPath = await findNodeAddonForBindings(
+        name,
+        tempDirectoryPath
+      );
       // Assert
       const expectedAbsPath = path.join(tempDirectoryPath, relPath);
       assert.equal(actualPath, expectedAbsPath);

packages/host/src/node/path-utils.test.ts

@@ -279,9 +279,9 @@ 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\//,
+  /(^|\/)react-native-node-api\//,
+  /(^|\/)node_modules\//,
+  /(^|\/).git\//,
 ];
 
 export function hasPlatformExtension(
@@ -302,13 +302,12 @@ export type FindNodeApiModuleOptions = {
 };
 
 /**
- * Recursively search into a directory for xcframeworks containing Node-API modules.
- * TODO: Turn this asynchronous
+ * Recursively search into a directory for directories containing Node-API modules.
  */
-export function findNodeApiModulePaths(
+export async function findNodeApiModulePaths(
   options: FindNodeApiModuleOptions,
   suffix = ""
-): string[] {
+): Promise<string[]> {
   const {
     fromPath,
     platform,
@@ -325,27 +324,33 @@ export function findNodeApiModulePaths(
     return [];
   }
 
-  return fs
-    .readdirSync(candidatePath, { withFileTypes: true })
-    .flatMap((file) => {
-      if (
-        file.isFile() &&
-        file.name === MAGIC_FILENAME &&
-        hasPlatformExtension(platform, candidatePath)
-      ) {
-        return [candidatePath];
-      } else if (file.isDirectory()) {
-        // Traverse into the child directory
-        return findNodeApiModulePaths(options, path.join(suffix, file.name));
-      }
-      return [];
-    });
+  const result: string[] = [];
+  const pendingResults: Promise<string[]>[] = [];
+
+  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))
+      );
+    }
+  }
+  const childResults = await Promise.all(pendingResults);
+  result.push(...childResults.flatMap((filePath) => filePath));
+  return result;
 }
 
 /**
  * Finds all dependencies of the app package and their xcframeworks.
  */
-export function findNodeApiModulePathsByDependency({
+export async function findNodeApiModulePathsByDependency({
   fromPath,
   includeSelf,
   ...options
@@ -360,25 +365,32 @@ export function findNodeApiModulePathsByDependency({
     const { name } = readPackageSync({ cwd: packageRoot });
     packagePathsByName[name] = packageRoot;
   }
-  // Find all their xcframeworks
-  return Object.fromEntries(
-    Object.entries(packagePathsByName)
-      .map(([dependencyName, dependencyPath]) => {
+
+  // Find all their node api module paths
+  const resultEntries = await Promise.all(
+    Object.entries(packagePathsByName).map(
+      async ([dependencyName, dependencyPath]) => {
         // Make all the xcframeworks relative to the dependency path
-        const modulePaths = findNodeApiModulePaths({
+        const absoluteModulePaths = await findNodeApiModulePaths({
           fromPath: dependencyPath,
           ...options,
-        }).map((p) => path.relative(dependencyPath, p));
+        });
         return [
           dependencyName,
           {
             path: dependencyPath,
-            modulePaths,
+            modulePaths: absoluteModulePaths.map((p) =>
+              path.relative(dependencyPath, p)
+            ),
           },
         ] as const;
-      })
-      // Remove any dependencies without module paths
-      .filter(([, { modulePaths }]) => modulePaths.length > 0)
+      }
+    )
+  );
+  // Return an object by dependency name
+  return Object.fromEntries(
+    // Remove any dependencies without Node-API module paths
+    resultEntries.filter(([, { modulePaths }]) => modulePaths.length > 0)
   );
 }