Skip to content

Commit

Permalink
Simplify and enforce duplicate haste map errors (#8002)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjesun authored Feb 28, 2019
1 parent 5d2d46f commit 1b2fffa
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 72 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- `[jest-transform]` Normalize config and remove unecessary checks, convert `TestUtils.js` to TypeScript ([#7801](https://github.com/facebook/jest/pull/7801))
- `[jest-worker]` Fix `jest-worker` when using pre-allocated jobs ([#7934](https://github.com/facebook/jest/pull/7934))
- `[jest-changed-files]` Fix `getChangedFilesFromRoots` to not return parts of the commit messages as if they were files, when the commit messages contained multiple paragraphs ([#7961](https://github.com/facebook/jest/pull/7961))
- `[jest-haste-map]` Enforce uniqueness in names (mocks and haste ids) ([#8002](https://github.com/facebook/jest/pull/8002))
- `[static]` Remove console log '-' on the front page

### Chore & Maintenance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,7 @@ exports[`HasteMap file system changes processing recovery from duplicate module
"
`;

exports[`HasteMap throws on duplicate module ids if "throwOnModuleCollision" is set to true 1`] = `
[Error: jest-haste-map: Haste module naming collision:
Duplicate module name: Strawberry
Paths: /project/fruits/another/Strawberry.js collides with /project/fruits/Strawberry.js
This error is caused by \`hasteImpl\` returning the same name for different files.]
`;
exports[`HasteMap throws on duplicate module ids if "throwOnModuleCollision" is set to true 1`] = `[Error: Duplicated files or mocks. Please check the console for more info]`;

exports[`HasteMap tries to crawl using node as a fallback 1`] = `
"jest-haste-map: Watchman crawl failed. Retrying once with node crawler.
Expand All @@ -31,23 +25,17 @@ exports[`HasteMap tries to crawl using node as a fallback 1`] = `
`;

exports[`HasteMap warns on duplicate mock files 1`] = `
"jest-haste-map: duplicate manual mock found:
Module name: subdir/Blueberry
Duplicate Mock path: /project/fruits2/__mocks__/subdir/Blueberry.js
This warning is caused by two manual mock files with the same file name.
Jest will use the mock file found in:
/project/fruits2/__mocks__/subdir/Blueberry.js
Please delete one of the following two files:
/project/fruits1/__mocks__/subdir/Blueberry.js
/project/fruits2/__mocks__/subdir/Blueberry.js
"jest-haste-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[`HasteMap warns on duplicate module ids 1`] = `
"jest-haste-map: Haste module naming collision:
Duplicate module name: Strawberry
Paths: /project/fruits/other/Strawberry.js collides with /project/fruits/Strawberry.js
This warning is caused by \`hasteImpl\` returning the same name for different files."
"jest-haste-map: Haste module naming collision: Strawberry
The following files share their name; please adjust your hasteImpl:
* <rootDir>/fruits/Strawberry.js
* <rootDir>/fruits/other/Strawberry.js
"
`;
17 changes: 14 additions & 3 deletions packages/jest-haste-map/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ const useBuitinsInContext = value => {
};

let consoleWarn;
let consoleError;
let defaultConfig;
let fs;
let H;
Expand Down Expand Up @@ -180,7 +181,10 @@ describe('HasteMap', () => {
fs = require('graceful-fs');

consoleWarn = console.warn;
consoleError = console.error;

console.warn = jest.fn();
console.error = jest.fn();

HasteMap = require('../');
H = HasteMap.H;
Expand All @@ -203,6 +207,7 @@ describe('HasteMap', () => {

afterEach(() => {
console.warn = consoleWarn;
console.error = consoleError;
});

it('exports constants', () => {
Expand Down Expand Up @@ -522,6 +527,8 @@ describe('HasteMap', () => {
});

it('warns on duplicate mock files', () => {
expect.assertions(1);

// Duplicate mock files for blueberry
mockFs['/project/fruits1/__mocks__/subdir/Blueberry.js'] = `
// Blueberry
Expand All @@ -530,10 +537,14 @@ describe('HasteMap', () => {
// Blueberry too!
`;

return new HasteMap({mocksPattern: '__mocks__', ...defaultConfig})
return new HasteMap({
mocksPattern: '__mocks__',
throwOnModuleCollision: true,
...defaultConfig,
})
.build()
.then(({__hasteMapForTest: data}) => {
expect(console.warn.mock.calls[0][0]).toMatchSnapshot();
.catch(({__hasteMapForTest: data}) => {
expect(console.error.mock.calls[0][0]).toMatchSnapshot();
});
});

Expand Down
70 changes: 48 additions & 22 deletions packages/jest-haste-map/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,27 +436,31 @@ class HasteMap extends EventEmitter {
H.GENERIC_PLATFORM;

const existingModule = moduleMap[platform];

if (existingModule && existingModule[H.PATH] !== module[H.PATH]) {
const message =
`jest-haste-map: Haste module naming collision:\n` +
` Duplicate module name: ${id}\n` +
` Paths: ${fastPath.resolve(
rootDir,
module[H.PATH],
)} collides with ` +
`${fastPath.resolve(rootDir, existingModule[H.PATH])}\n\nThis ` +
`${this._options.throwOnModuleCollision ? 'error' : 'warning'} ` +
`is caused by \`hasteImpl\` returning the same name for different` +
` files.`;
const method = this._options.throwOnModuleCollision ? 'error' : 'warn';

this._console[method](
[
'jest-haste-map: Haste module naming collision: ' + id,
' The following files share their name; please adjust your hasteImpl:',
' * <rootDir>' + path.sep + existingModule[H.PATH],
' * <rootDir>' + path.sep + module[H.PATH],
'',
].join('\n'),
);

if (this._options.throwOnModuleCollision) {
throw new Error(message);
throw new DuplicateError(existingModule[H.PATH], module[H.PATH]);
}
this._console.warn(message);

// We do NOT want consumers to use a module that is ambiguous.
delete moduleMap[platform];

if (Object.keys(moduleMap).length === 1) {
map.delete(id);
}

let dupsByPlatform = hasteMap.duplicates.get(id);
if (dupsByPlatform == null) {
dupsByPlatform = new Map();
Expand Down Expand Up @@ -557,18 +561,26 @@ class HasteMap extends EventEmitter {
) {
const mockPath = getMockName(filePath);
const existingMockPath = mocks.get(mockPath);

if (existingMockPath) {
this._console.warn(
`jest-haste-map: duplicate manual mock found:\n` +
` Module name: ${mockPath}\n` +
` Duplicate Mock path: ${filePath}\nThis warning ` +
`is caused by two manual mock files with the same file name.\n` +
`Jest will use the mock file found in: \n` +
`${filePath}\n` +
` Please delete one of the following two files: \n ` +
`${path.join(rootDir, existingMockPath)}\n${filePath}\n\n`,
const secondMockPath = fastPath.relative(rootDir, filePath);
const method = this._options.throwOnModuleCollision ? 'error' : 'warn';

this._console[method](
[
'jest-haste-map: duplicate manual mock found: ' + mockPath,
' The following files share their name; please delete one of them:',
' * <rootDir>' + path.sep + existingMockPath,
' * <rootDir>' + path.sep + secondMockPath,
'',
].join('\n'),
);

if (this._options.throwOnModuleCollision) {
throw new DuplicateError(existingMockPath, secondMockPath);
}
}

mocks.set(mockPath, relativeFilePath);
}

Expand Down Expand Up @@ -1079,9 +1091,22 @@ class HasteMap extends EventEmitter {
}

static H: HType;
static DuplicateError: typeof DuplicateError;
static ModuleMap: typeof HasteModuleMap;
}

class DuplicateError extends Error {
mockPath1: string;
mockPath2: string;

constructor(mockPath1: string, mockPath2: string) {
super('Duplicated files or mocks. Please check the console for more info');

this.mockPath1 = mockPath1;
this.mockPath2 = mockPath2;
}
}

function copy<T extends Object>(object: T): T {
return Object.assign(Object.create(null), object);
}
Expand All @@ -1091,6 +1116,7 @@ function copyMap<K, V>(input: Map<K, V>): Map<K, V> {
}

HasteMap.H = H;
HasteMap.DuplicateError = DuplicateError;
HasteMap.ModuleMap = HasteModuleMap;

export = HasteMap;
25 changes: 0 additions & 25 deletions packages/jest-runtime/src/__tests__/runtime_require_mock.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

'use strict';

const path = require('path');

let createRuntime;
const consoleWarn = console.warn;

Expand Down Expand Up @@ -137,29 +135,6 @@ describe('Runtime', () => {
}).toThrow();
}));

it('uses the closest manual mock when duplicates exist', () => {
console.warn = jest.fn();
return createRuntime(__filename, {
rootDir: path.resolve(
path.dirname(__filename),
'test_root_with_dup_mocks',
),
}).then(runtime => {
expect(console.warn).toBeCalled();
const exports1 = runtime.requireMock(
runtime.__mockRootPath,
'./subdir1/my_module',
);
expect(exports1.modulePath).toEqual('subdir1/__mocks__/my_module.js');

const exports2 = runtime.requireMock(
runtime.__mockRootPath,
'./subdir2/my_module',
);
expect(exports2.modulePath).toEqual('subdir2/__mocks__/my_module.js');
});
});

it('uses manual mocks when using a custom resolver', () =>
createRuntime(__filename, {
// using the default resolver as a custom resolver
Expand Down

0 comments on commit 1b2fffa

Please sign in to comment.