Track duplicates in MockMap, refactor setThrowOnModuleCollision -> assertValid

packages/metro-file-map/src/__tests__/__snapshots__/index-test.js.snap

@@ -22,14 +22,6 @@ exports[`FileMap tries to crawl using node as a fallback 1`] = `
   Error: watchman error"
 `;
 
-exports[`FileMap warns on duplicate mock files 1`] = `
-"metro-file-map: duplicate manual mock found: subdir/Blueberry
-  The following files share their name; please delete one of them:
-    * <rootDir>/fruits1/__mocks__/subdir/Blueberry.js
-    * <rootDir>/fruits2/__mocks__/subdir/Blueberry.js
-"
-`;
-
 exports[`FileMap warns on duplicate module ids 1`] = `
 "metro-file-map: Haste module naming collision: Strawberry
   The following files share their name; please adjust your hasteImpl:

packages/metro-file-map/src/__tests__/index-test.js

@@ -486,11 +486,10 @@ describe('FileMap', () => {
       path.resolve(defaultConfig.rootDir, 'fruits', '__mocks__', 'Pear.js'),
     );
 
-    expect(cacheContent.mocks).toEqual(
-      createMap({
-        Pear: path.join('fruits', '__mocks__', 'Pear.js'),
-      }),
-    );
+    expect(cacheContent.mocks).toEqual({
+      mocks: new Map([['Pear', path.join('fruits', '__mocks__', 'Pear.js')]]),
+      duplicates: new Map(),
+    });
 
     // The cache file must exactly mirror the data structure returned from a
     // read
@@ -772,9 +771,7 @@ describe('FileMap', () => {
     expect(mockMap).toBeNull();
   });
 
-  test('warns on duplicate mock files', async () => {
-    expect.assertions(1);
-
+  test('throws on duplicate mock files when throwOnModuleCollision', async () => {
     // Duplicate mock files for blueberry
     mockFs[
       path.join(
@@ -801,17 +798,18 @@ describe('FileMap', () => {
       // Blueberry too!
     `;
 
-    try {
-      await new FileMap({
+    expect(() =>
+      new FileMap({
         mocksPattern: '__mocks__',
         throwOnModuleCollision: true,
         ...defaultConfig,
-      }).build();
-    } catch {
-      expect(
-        console.error.mock.calls[0][0].replaceAll('\\', '/'),
-      ).toMatchSnapshot();
-    }
+      }).build(),
+    ).rejects.toThrowError(
+      'Mock map has 1 error:\n' +
+        'Duplicate manual mock found for `subdir/Blueberry`:\n' +
+        '    * <rootDir>/../../fruits1/__mocks__/subdir/Blueberry.js\n' +
+        '    * <rootDir>/../../fruits2/__mocks__/subdir/Blueberry.js\n',
+    );
   });
 
   test('warns on duplicate module ids', async () => {

packages/metro-file-map/src/flow-types.js

@@ -48,7 +48,7 @@ export type BuildResult = {
 
 export type CacheData = $ReadOnly<{
   clocks: WatchmanClocks,
-  mocks: RawMockMap,
+  mocks: ?RawMockMap,
   fileSystemData: mixed,
 }>;
 
@@ -312,9 +312,15 @@ export interface MutableFileSystem extends FileSystem {
 
 export type Path = string;
 
-export type RawMockMap = Map<string, Path>;
+export type RawMockMap = $ReadOnly<{
+  duplicates: Map<string, Set<string>>,
+  mocks: Map<string, Path>,
+}>;
 
-export type ReadOnlyRawMockMap = $ReadOnlyMap<string, Path>;
+export type ReadOnlyRawMockMap = $ReadOnly<{
+  duplicates: $ReadOnlyMap<string, $ReadOnlySet<string>>,
+  mocks: $ReadOnlyMap<string, Path>,
+}>;
 
 export type WatchmanClockSpec =
   | string

packages/metro-file-map/src/index.js

@@ -141,7 +141,7 @@ export type {
 // This should be bumped whenever a code change to `metro-file-map` itself
 // would cause a change to the cache data structure and/or content (for a given
 // filesystem state and build parameters).
-const CACHE_BREAKER = '7';
+const CACHE_BREAKER = '8';
 
 const CHANGE_INTERVAL = 30;
 // Periodically yield to the event loop to allow parallel I/O, etc.
@@ -380,7 +380,7 @@ export default class FileMap extends EventEmitter {
             ? new MockMapImpl({
                 console: this._console,
                 mocksPattern: this._options.mocksPattern,
-                rawMockMap: initialData?.mocks ?? new Map(),
+                rawMockMap: initialData?.mocks,
                 rootDir,
                 throwOnModuleCollision: this._options.throwOnModuleCollision,
               })
@@ -389,6 +389,9 @@ export default class FileMap extends EventEmitter {
         // Update `fileSystem`, `hasteMap` and `mocks` based on the file delta.
         await this._applyFileDelta(fileSystem, hasteMap, mockMap, fileDelta);
 
+        // Validate the state of the mock map before persisting it.
+        mockMap?.assertValid();
+
         await this._takeSnapshotAndPersist(
           fileSystem,
           fileDelta.clocks ?? new Map(),
@@ -740,7 +743,7 @@ export default class FileMap extends EventEmitter {
       {
         fileSystemData: fileSystem.getSerializableSnapshot(),
         clocks: new Map(clocks),
-        mocks: mockMap ? mockMap.getSerializableSnapshot() : new Map(),
+        mocks: mockMap ? mockMap.getSerializableSnapshot() : null,
       },
       {changed, removed},
     );
@@ -820,7 +823,6 @@ export default class FileMap extends EventEmitter {
     // In watch mode, we'll only warn about module collisions and we'll retain
     // all files, even changes to node_modules.
     hasteMap.setThrowOnModuleCollision(false);
-    mockMap?.setThrowOnModuleCollision(false);
     this._options.retainAllFiles = true;
 
     const hasWatchedExtension = (filePath: string) =>

packages/metro-file-map/src/lib/MockMap.js

@@ -12,8 +12,8 @@
 import type {MockMap as IMockMap, Path, RawMockMap} from '../flow-types';
 
 import getMockName from '../getMockName';
-import {DuplicateError} from './DuplicateError';
 import {RootPathUtils} from './RootPathUtils';
+import nullthrows from 'nullthrows';
 import path from 'path';
 
 export default class MockMap implements IMockMap {
@@ -33,21 +33,25 @@ export default class MockMap implements IMockMap {
   }: {
     console: typeof console,
     mocksPattern: RegExp,
-    rawMockMap: RawMockMap,
+    rawMockMap?: ?RawMockMap,
     rootDir: Path,
     throwOnModuleCollision: boolean,
   }) {
     this.#mocksPattern = mocksPattern;
-    this.#raw = rawMockMap;
+    this.#raw = rawMockMap ?? {mocks: new Map(), duplicates: new Map()};
     this.#rootDir = rootDir;
     this.#console = console;
     this.#pathUtils = new RootPathUtils(rootDir);
     this.#throwOnModuleCollision = throwOnModuleCollision;
   }
 
   getMockModule(name: string): ?Path {
-    const mockPath = this.#raw.get(name) || this.#raw.get(name + '/index');
-    return mockPath != null ? this.#pathUtils.normalToAbsolute(mockPath) : null;
+    const mockPath =
+      this.#raw.mocks.get(name) || this.#raw.mocks.get(name + '/index');
+    if (typeof mockPath !== 'string') {
+      return null;
+    }
+    return this.#pathUtils.normalToAbsolute(mockPath);
   }
 
   onNewOrModifiedFile(absoluteFilePath: Path): void {
@@ -56,45 +60,90 @@ export default class MockMap implements IMockMap {
     }
 
     const mockName = getMockName(absoluteFilePath);
-    const existingMockPath = this.#raw.get(mockName);
+    const existingMockPath = this.#raw.mocks.get(mockName);
     const newMockPath = this.#pathUtils.absoluteToNormal(absoluteFilePath);
 
     if (existingMockPath != null) {
       if (existingMockPath !== newMockPath) {
-        const method = this.#throwOnModuleCollision ? 'error' : 'warn';
-
-        this.#console[method](
-          [
-            'metro-file-map: duplicate manual mock found: ' + mockName,
-            '  The following files share their name; please delete one of them:',
-            '    * <rootDir>' + path.sep + existingMockPath,
-            '    * <rootDir>' + path.sep + newMockPath,
-            '',
-          ].join('\n'),
-        );
-
-        if (this.#throwOnModuleCollision) {
-          throw new DuplicateError(existingMockPath, newMockPath);
+        let duplicates = this.#raw.duplicates.get(mockName);
+        if (duplicates == null) {
+          duplicates = new Set([existingMockPath, newMockPath]);
+          this.#raw.duplicates.set(mockName, duplicates);
+        } else {
+          duplicates.add(newMockPath);
         }
+
+        this.#console.warn(this.#getMessageForDuplicates(mockName, duplicates));
       }
     }
 
-    this.#raw.set(mockName, newMockPath);
+    // If there are duplicates and we don't throw, the latest mock wins.
+    // This is to preserve backwards compatibility, but it's unpredictable.
+    this.#raw.mocks.set(mockName, newMockPath);
   }
 
   onRemovedFile(absoluteFilePath: Path): void {
     if (!this.#mocksPattern.test(absoluteFilePath)) {
       return;
     }
     const mockName = getMockName(absoluteFilePath);
-    this.#raw.delete(mockName);
+    const duplicates = this.#raw.duplicates.get(mockName);
+    if (duplicates != null) {
+      const relativePath = this.#pathUtils.absoluteToNormal(absoluteFilePath);
+      duplicates.delete(relativePath);
+      if (duplicates.size === 1) {
+        this.#raw.duplicates.delete(mockName);
+      }
+      // Set the mock to a remaining duplicate. Should never be empty.
+      const remaining = nullthrows(duplicates.values().next().value);
+      this.#raw.mocks.set(mockName, remaining);
+    } else {
+      this.#raw.mocks.delete(mockName);
+    }
   }
 
-  setThrowOnModuleCollision(throwOnModuleCollision: boolean): void {
-    this.#throwOnModuleCollision = throwOnModuleCollision;
+  getSerializableSnapshot(): RawMockMap {
+    return {
+      mocks: new Map(this.#raw.mocks),
+      duplicates: new Map(
+        [...this.#raw.duplicates].map(([k, v]) => [k, new Set(v)]),
+      ),
+    };
   }
 
-  getSerializableSnapshot(): RawMockMap {
-    return new Map(this.#raw);
+  assertValid(): void {
+    if (!this.#throwOnModuleCollision) {
+      return;
+    }
+    // Throw an aggregate error for each duplicate.
+    const errors = [];
+    for (const [mockName, relativePaths] of this.#raw.duplicates) {
+      errors.push(this.#getMessageForDuplicates(mockName, relativePaths));
+    }
+    if (errors.length > 0) {
+      throw new Error(
+        `Mock map has ${errors.length} error${errors.length > 1 ? 's' : ''}:\n${errors.join('\n')}`,
+      );
+    }
+  }
+
+  #getMessageForDuplicates(
+    mockName: string,
+    duplicates: $ReadOnlySet<string>,
+  ): string {
+    return (
+      'Duplicate manual mock found for `' +
+      mockName +
+      '`:\n' +
+      [...duplicates]
+        .map(
+          relativePath =>
+            '    * <rootDir>' +
+            path.sep +
+            this.#pathUtils.absoluteToNormal(relativePath) +
+            '\n',
+        )
+        .join('')
+    );
   }
 }

packages/metro-file-map/src/lib/__tests__/MockMap-test.js

@@ -0,0 +1,92 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ * @oncall react_native
+ */
+
+import type MockMapType from '../MockMap';
+
+let mockPathModule;
+jest.mock('path', () => mockPathModule);
+
+describe.each([['win32'], ['posix']])('MockMap on %s', platform => {
+  const p: string => string = filePath =>
+    platform === 'win32'
+      ? filePath.replace(/\//g, '\\').replace(/^\\/, 'C:\\')
+      : filePath;
+
+  let MockMap: Class<MockMapType>;
+
+  const opts = {
+    console,
+    mocksPattern: /__mocks__[\/\\].+\.(js|json)$/,
+    rootDir: p('/root'),
+    throwOnModuleCollision: true,
+  };
+
+  beforeEach(() => {
+    jest.resetModules();
+    mockPathModule = jest.requireActual<{}>('path')[platform];
+    MockMap = require('../MockMap').default;
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    jest.clearAllMocks();
+  });
+
+  test('set and get a mock module', () => {
+    const mockMap = new MockMap(opts);
+    mockMap.onNewOrModifiedFile(p('/root/__mocks__/foo.js'));
+    expect(mockMap.getMockModule('foo')).toBe(p('/root/__mocks__/foo.js'));
+  });
+
+  test('assertValid throws on duplicates', () => {
+    const mockMap = new MockMap(opts);
+    mockMap.onNewOrModifiedFile(p('/root/__mocks__/foo.js'));
+    mockMap.onNewOrModifiedFile(p('/root/other/__mocks__/foo.js'));
+
+    expect(console.warn).toHaveBeenCalledTimes(1);
+    expect(() => mockMap.assertValid()).toThrowError(
+      `Mock map has 1 error:
+Duplicate manual mock found for \`foo\`:
+    * <rootDir>/../../__mocks__/foo.js
+    * <rootDir>/../../other/__mocks__/foo.js
+`.replaceAll('/', mockPathModule.sep),
+    );
+  });
+
+  test('recovers from duplicates', () => {
+    const mockMap = new MockMap(opts);
+    mockMap.onNewOrModifiedFile(p('/root/__mocks__/foo.js'));
+    mockMap.onNewOrModifiedFile(p('/root/other/__mocks__/foo.js'));
+
+    expect(() => mockMap.assertValid()).toThrow();
+
+    // Latest mock wins
+    expect(mockMap.getMockModule('foo')).toBe(
+      p('/root/other/__mocks__/foo.js'),
+    );
+
+    expect(mockMap.getSerializableSnapshot()).toEqual({
+      mocks: new Map([['foo', p('other/__mocks__/foo.js')]]),
+      duplicates: new Map([
+        ['foo', new Set([p('other/__mocks__/foo.js'), p('__mocks__/foo.js')])],
+      ]),
+    });
+
+    mockMap.onRemovedFile(p('/root/other/__mocks__/foo.js'));
+
+    expect(() => mockMap.assertValid()).not.toThrow();
+
+    // Recovery after the latest mock is deleted
+    expect(mockMap.getMockModule('foo')).toBe(p('/root/__mocks__/foo.js'));
+
+    expect(mockMap.getSerializableSnapshot()).toEqual({
+      mocks: new Map([['foo', p('__mocks__/foo.js')]]),
+      duplicates: new Map(),
+    });
+  });
+});