Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify and enforce duplicate haste map errors #8002

Merged
merged 1 commit into from
Feb 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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}) => {
mjesun marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -432,27 +432,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 @@ -553,18 +557,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 @@ -1069,9 +1081,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 @@ -1081,6 +1106,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