diff --git a/packages/metro-file-map/src/__tests__/__snapshots__/index-test.js.snap b/packages/metro-file-map/src/__tests__/__snapshots__/index-test.js.snap index fbf4e18264..9a2ca4eeb4 100644 --- a/packages/metro-file-map/src/__tests__/__snapshots__/index-test.js.snap +++ b/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: - * /fruits1/__mocks__/subdir/Blueberry.js - * /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: diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index 8652d2e19e..9845b1b48c 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/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' + + ' * /../../fruits1/__mocks__/subdir/Blueberry.js\n' + + ' * /../../fruits2/__mocks__/subdir/Blueberry.js\n', + ); }); test('warns on duplicate module ids', async () => { diff --git a/packages/metro-file-map/src/flow-types.js b/packages/metro-file-map/src/flow-types.js index 7293c6a9fc..41c448ae03 100644 --- a/packages/metro-file-map/src/flow-types.js +++ b/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; +export type RawMockMap = $ReadOnly<{ + duplicates: Map>, + mocks: Map, +}>; -export type ReadOnlyRawMockMap = $ReadOnlyMap; +export type ReadOnlyRawMockMap = $ReadOnly<{ + duplicates: $ReadOnlyMap>, + mocks: $ReadOnlyMap, +}>; export type WatchmanClockSpec = | string diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 47e2e4c9f2..82c62ef768 100644 --- a/packages/metro-file-map/src/index.js +++ b/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) => diff --git a/packages/metro-file-map/src/lib/MockMap.js b/packages/metro-file-map/src/lib/MockMap.js index 455a0873be..5bee425bcd 100644 --- a/packages/metro-file-map/src/lib/MockMap.js +++ b/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,12 +33,12 @@ 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); @@ -46,8 +46,12 @@ export default class MockMap implements IMockMap { } 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,30 +60,26 @@ 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:', - ' * ' + path.sep + existingMockPath, - ' * ' + 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 { @@ -87,14 +87,63 @@ export default class MockMap implements IMockMap { 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 { + return ( + 'Duplicate manual mock found for `' + + mockName + + '`:\n' + + [...duplicates] + .map( + relativePath => + ' * ' + + path.sep + + this.#pathUtils.absoluteToNormal(relativePath) + + '\n', + ) + .join('') + ); } } diff --git a/packages/metro-file-map/src/lib/__tests__/MockMap-test.js b/packages/metro-file-map/src/lib/__tests__/MockMap-test.js new file mode 100644 index 0000000000..aea88a6bf8 --- /dev/null +++ b/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; + + 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\`: + * /../../__mocks__/foo.js + * /../../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(), + }); + }); +}); diff --git a/packages/metro-file-map/types/flow-types.d.ts b/packages/metro-file-map/types/flow-types.d.ts index 902c738584..9a71baf7b2 100644 --- a/packages/metro-file-map/types/flow-types.d.ts +++ b/packages/metro-file-map/types/flow-types.d.ts @@ -43,7 +43,7 @@ export interface BuildResult { export interface CacheData { readonly clocks: WatchmanClocks; - readonly mocks: MockData; + readonly mocks: RawMockMap; readonly files: FileData; } @@ -278,7 +278,11 @@ export interface HasteMap { computeConflicts(): Array; } -export type MockData = Map; +export type RawMockMap = { + readonly mocks: Map; + readonly duplicates: Map>; +}; + export type HasteMapData = Map; export interface HasteMapItem {