Skip to content

Commit

Permalink
Track duplicates in MockMap, refactor setThrowOnModuleCollision -> as…
Browse files Browse the repository at this point in the history
…sertValid (#1406)

Summary:
- Refactors mock duplicate handling from "throw the next time you see an error" to "throw now if there are errors"
 - The new structure is more generic and more plugin-friendly. We'll use plugins for two main purporses:
   - Remove non-Metro concerns from Metro.
   - Extensibility, for using metro-file-map outside Metro for other purposes - eg processing Buck file changes.
 - We'll make the same change for Haste duplicates, so that Haste and Mocks are each (eventually) plugins to metro-file-map.

Changelog: Internal

Pull Request resolved: #1406

Test Plan:
- Not observable in Metro since we set `ignorePatterns: ''` - the mock map is always null.
 - New unit tests, existing coverage of mock behaviour.

Reviewed By: huntie

Differential Revision: D67277387

Pulled By: robhogan

fbshipit-source-id: 8bb8384eb7ab7a425abf49e009c478b62d13f82c
  • Loading branch information
robhogan authored and facebook-github-bot committed Dec 16, 2024
1 parent eb17b98 commit b63a043
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
30 changes: 14 additions & 16 deletions packages/metro-file-map/src/__tests__/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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 () => {
Expand Down
12 changes: 9 additions & 3 deletions packages/metro-file-map/src/flow-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export type BuildResult = {

export type CacheData = $ReadOnly<{
clocks: WatchmanClocks,
mocks: RawMockMap,
mocks: ?RawMockMap,
fileSystemData: mixed,
}>;

Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions packages/metro-file-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
})
Expand All @@ -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(),
Expand Down Expand Up @@ -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},
);
Expand Down Expand Up @@ -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) =>
Expand Down
101 changes: 75 additions & 26 deletions packages/metro-file-map/src/lib/MockMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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('')
);
}
}
92 changes: 92 additions & 0 deletions packages/metro-file-map/src/lib/__tests__/MockMap-test.js
Original file line number Diff line number Diff line change
@@ -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(),
});
});
});
Loading

0 comments on commit b63a043

Please sign in to comment.