feat(metro-plugin-duplicates-checker): differentiate between banned and duplicate modules (#1886)

BREAKING CHANGES: Signature of `checkForDuplicatePackages` changed to
return an object to differentiate between banned and duplicate modules.

.changeset/spotty-trainers-cheat.md

@@ -0,0 +1,5 @@
+---
+"@rnx-kit/metro-plugin-duplicates-checker": major
+---
+
+Breaking: Changed `checkForDuplicatePackages` to return an object to differentiate between banned and duplicate modules

packages/metro-plugin-duplicates-checker/src/checkForDuplicatePackages.ts

@@ -13,6 +13,11 @@ export type Options = {
   throwOnError?: boolean;
 };
 
+export type Result = {
+  banned: number;
+  duplicates: number;
+};
+
 export const defaultOptions: Options = {
   ignoredModules: [],
   bannedModules: [],
@@ -25,7 +30,7 @@ export function countCopies(module: ModuleMap[string]): number {
   }, 0);
 }
 
-export function printDuplicates(module: ModuleMap[string]): void {
+export function printModule(module: ModuleMap[string]): void {
   Object.keys(module)
     .sort()
     .forEach((version) => {
@@ -38,42 +43,46 @@ export function printDuplicates(module: ModuleMap[string]): void {
 export function detectDuplicatePackages(
   bundledModules: ModuleMap,
   { ignoredModules = [], bannedModules = [] }: Options
-): number {
-  return Object.keys(bundledModules).reduce((count, name) => {
+): Result {
+  let numBanned = 0;
+  let numDupes = 0;
+
+  for (const name of Object.keys(bundledModules)) {
+    if (bannedModules.includes(name)) {
+      error(`${name} (banned)`);
+      printModule(bundledModules[name]);
+      numBanned += 1;
+      continue;
+    }
+
     if (ignoredModules.includes(name)) {
-      return count;
+      continue;
     }
 
     const currentModule = bundledModules[name];
-
-    if (bannedModules.includes(name)) {
-      error(`${name} (banned)`);
-      printDuplicates(currentModule);
-      return count + 1;
-    }
-
     const numCopies = countCopies(currentModule);
     if (numCopies > 1) {
       error(`${name} (found ${numCopies} copies)`);
-      printDuplicates(currentModule);
-      return count + 1;
+      printModule(currentModule);
+      numDupes += 1;
+      continue;
     }
+  }
 
-    return count;
-  }, 0);
+  return { banned: numBanned, duplicates: numDupes };
 }
 
 export function checkForDuplicateDependencies(
   graph: Graph,
   options: Options = defaultOptions
-): number {
+): Result {
   return detectDuplicatePackages(gatherModulesFromGraph(graph, {}), options);
 }
 
 export function checkForDuplicatePackages(
   sourceMap: MixedSourceMap,
   options: Options = defaultOptions
-): number {
+): Result {
   return detectDuplicatePackages(
     gatherModulesFromSourceMap(sourceMap, {}),
     options

packages/metro-plugin-duplicates-checker/src/index.ts

@@ -3,15 +3,30 @@ import type { MetroPlugin } from "@rnx-kit/metro-serializer";
 import { readFile } from "fs";
 import type { Graph, Module, SerializerOptions } from "metro";
 import type { MixedSourceMap } from "metro-source-map";
+import type { Options, Result } from "./checkForDuplicatePackages";
 import {
   checkForDuplicateDependencies,
   checkForDuplicatePackages,
   defaultOptions,
-  Options,
 } from "./checkForDuplicatePackages";
 
 export { checkForDuplicatePackages };
-export type { Options };
+export type { Options, Result };
+
+function getErrorMessage({ banned, duplicates }: Result): string | undefined {
+  if (duplicates > 0) {
+    if (banned > 0) {
+      return `Found ${banned} banned module(s) and ${duplicates} duplicate(s)`;
+    }
+    return `Found ${duplicates} duplicate(s)`;
+  }
+
+  if (banned > 0) {
+    return `Found ${banned} banned module(s)`;
+  }
+
+  return undefined;
+}
 
 export function checkForDuplicatePackagesInFile(
   sourceMap: string,
@@ -25,13 +40,14 @@ export function checkForDuplicatePackagesInFile(
       }
 
       const sourceMap = JSON.parse(data) as MixedSourceMap;
-      const count = checkForDuplicatePackages(sourceMap, options);
-      if (count > 0) {
+      const result = checkForDuplicatePackages(sourceMap, options);
+      const errorMessage = getErrorMessage(result);
+      if (errorMessage) {
         const { throwOnError = true } = options;
         if (throwOnError) {
-          reject(new Error("Duplicates found"));
+          reject(new Error(errorMessage));
         } else {
-          error("Duplicates found!");
+          error(errorMessage);
           resolve();
         }
       } else {
@@ -50,14 +66,15 @@ export function DuplicateDependencies(
     graph: Graph,
     _options: SerializerOptions
   ) => {
-    const count = checkForDuplicateDependencies(graph, pluginOptions);
-    if (count > 0) {
+    const result = checkForDuplicateDependencies(graph, pluginOptions);
+    const errorMessage = getErrorMessage(result);
+    if (errorMessage) {
       const { throwOnError = true } = pluginOptions;
       if (throwOnError) {
-        throw new Error("Duplicates found");
+        throw new Error(errorMessage);
       }
 
-      error("Duplicates found!");
+      error(errorMessage);
     }
   };
 }
@@ -65,8 +82,8 @@ export function DuplicateDependencies(
 if (require.main === module) {
   checkForDuplicatePackagesInFile(process.argv[2]).catch((error) => {
     if (error) {
+      process.exitCode = 1;
       error(error.message);
-      process.exit(1);
     }
   });
 }

packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts

@@ -4,7 +4,7 @@ import {
   countCopies,
   detectDuplicatePackages,
   Options,
-  printDuplicates,
+  printModule,
 } from "../src/checkForDuplicatePackages";
 import {
   bundleGraph,
@@ -90,7 +90,7 @@ describe("countCopies()", () => {
   });
 });
 
-describe("printDuplicates()", () => {
+describe("printModules()", () => {
   const consoleWarnSpy = jest.spyOn(global.console, "warn");
 
   beforeEach(() => {
@@ -102,15 +102,15 @@ describe("printDuplicates()", () => {
   });
 
   test("prints all versions and locations of a package", () => {
-    printDuplicates(testModuleMap["fbjs"]);
+    printModule(testModuleMap["fbjs"]);
     expect(consoleWarnSpy).toBeCalledTimes(2);
     consoleWarnSpy.mockReset();
 
-    printDuplicates(testModuleMap["metro"]);
+    printModule(testModuleMap["metro"]);
     expect(consoleWarnSpy).toBeCalledTimes(2);
     consoleWarnSpy.mockReset();
 
-    printDuplicates(testModuleMap["react-native"]);
+    printModule(testModuleMap["react-native"]);
     expect(consoleWarnSpy).toBeCalledTimes(1);
     consoleWarnSpy.mockReset();
   });
@@ -130,18 +130,21 @@ describe("detectDuplicatePackages()", () => {
   });
 
   test("returns number of duplicated packages", () => {
-    expect(detectDuplicatePackages(testModuleMap, defaultOptions)).toBe(2);
+    expect(detectDuplicatePackages(testModuleMap, defaultOptions)).toEqual({
+      banned: 0,
+      duplicates: 2,
+    });
   });
 
   test("ignores specified packages", () => {
     expect(
       detectDuplicatePackages(testModuleMap, { ignoredModules: ["fbjs"] })
-    ).toBe(1);
+    ).toEqual({ banned: 0, duplicates: 1 });
     expect(
       detectDuplicatePackages(testModuleMap, {
         ignoredModules: ["fbjs", "metro"],
       })
-    ).toBe(0);
+    ).toEqual({ banned: 0, duplicates: 0 });
   });
 
   test("counts banned packages", () => {
@@ -149,7 +152,7 @@ describe("detectDuplicatePackages()", () => {
       detectDuplicatePackages(testModuleMap, {
         bannedModules: ["react", "react-native"],
       })
-    ).toBe(4);
+    ).toEqual({ banned: 2, duplicates: 2 });
   });
 
   test("prints the duplicated packages", () => {
@@ -173,11 +176,17 @@ describe("checkForDuplicateDependencies()", () => {
   });
 
   test("checkForDuplicateDependencies", () => {
-    expect(checkForDuplicateDependencies(bundleGraph)).toBe(0);
+    expect(checkForDuplicateDependencies(bundleGraph)).toEqual({
+      banned: 0,
+      duplicates: 0,
+    });
     expect(consoleErrorSpy).not.toHaveBeenCalled();
     expect(consoleWarnSpy).not.toHaveBeenCalled();
 
-    expect(checkForDuplicateDependencies(bundleGraphWithDuplicates)).toBe(1);
+    expect(checkForDuplicateDependencies(bundleGraphWithDuplicates)).toEqual({
+      banned: 0,
+      duplicates: 1,
+    });
     expect(consoleErrorSpy).toBeCalledTimes(1);
     expect(consoleWarnSpy).toBeCalledTimes(2);
   });
@@ -185,7 +194,13 @@ describe("checkForDuplicateDependencies()", () => {
 
 describe("checkForDuplicatePackages()", () => {
   test("checkForDuplicatePackages", () => {
-    expect(checkForDuplicatePackages(bundleSourceMap)).toBe(0);
-    expect(checkForDuplicatePackages(bundleSourceMapWithDuplicates)).toBe(1);
+    expect(checkForDuplicatePackages(bundleSourceMap)).toEqual({
+      banned: 0,
+      duplicates: 0,
+    });
+    expect(checkForDuplicatePackages(bundleSourceMapWithDuplicates)).toEqual({
+      banned: 0,
+      duplicates: 1,
+    });
   });
 });

packages/metro-plugin-duplicates-checker/test/testData.ts

@@ -1,6 +1,7 @@
 // istanbul ignore file
 
 import type { Graph, Module } from "@rnx-kit/metro-serializer";
+import type { BasicSourceMap } from "metro-source-map";
 import path from "path";
 
 const mockModule = {} as Module;
@@ -32,7 +33,7 @@ export const bundleGraphWithDuplicates: Graph = {
   entryPoints: [],
 };
 
-export const bundleSourceMap = {
+export const bundleSourceMap: BasicSourceMap = {
   version: 3,
   sources: [
     "__prelude__",
@@ -524,9 +525,11 @@ export const bundleSourceMap = {
     `${repoRoot}/node_modules/react-native/Libraries/NewAppScreen/components/ReloadInstructions.js`,
     `${repoRoot}/packages/test-app/lib/app.json`,
   ],
+  mappings: "",
+  names: [],
 };
 
-export const bundleSourceMapWithDuplicates = {
+export const bundleSourceMapWithDuplicates: BasicSourceMap = {
   version: 3,
   sources: [
     `${repoRoot}/packages/test-app/lib/src/index.js`,
@@ -534,4 +537,6 @@ export const bundleSourceMapWithDuplicates = {
     `${repoRoot}/node_modules/@react-native/polyfills/index.js`,
     `${repoRoot}/node_modules/react-native/index.js`,
   ],
+  mappings: "",
+  names: [],
 };