Fix haste resolution (and better warnings)

Summary: @​public

Fix the haste resolution algorithm. Changed the map data structure from a list of modules, to a list of modules grouped by platform.
This considerably simplifies the "getModule" method and makes it easy to properly warn about colliding module names.
This also fixes a bug where we used to include `.web` files when we shouldn't (see task).

Reviewed By: @vjeux

Differential Revision: D2482969
This commit is contained in:
Amjad Masad 2015-09-25 20:17:24 -07:00 committed by facebook-github-bot-4
parent 179b9c07a9
commit 2a1d958251
2 changed files with 139 additions and 96 deletions

View File

@ -12,6 +12,8 @@ const chalk = require('chalk');
const path = require('path');
const getPlatformExtension = require('../../lib/getPlatformExtension');
const GENERIC_PLATFORM = 'generic';
class HasteMap {
constructor({ fastfs, moduleCache, helpers }) {
this._fastfs = fastfs;
@ -43,11 +45,14 @@ class HasteMap {
/*eslint no-labels: 0 */
if (type === 'delete' || type === 'change') {
loop: for (let name in this._map) {
let modules = this._map[name];
for (var i = 0; i < modules.length; i++) {
if (modules[i].path === absPath) {
modules.splice(i, 1);
break loop;
const modulesMap = this._map[name];
for (let platform in modulesMap) {
const modules = modulesMap[platform];
for (var i = 0; i < modules.length; i++) {
if (modules[i].path === absPath) {
modules.splice(i, 1);
break loop;
}
}
}
}
@ -69,36 +74,45 @@ class HasteMap {
}
getModule(name, platform = null) {
if (!this._map[name]) {
const modulesMap = this._map[name];
if (modulesMap == null) {
return null;
}
const modules = this._map[name];
if (platform != null) {
for (let i = 0; i < modules.length; i++) {
if (getPlatformExtension(modules[i].path) === platform) {
return modules[i];
}
// If no platform is given we choose the generic platform module list.
// If a platform is given and no modules exist we fallback
// to the generic platform module list.
let modules;
if (platform == null) {
modules = modulesMap[GENERIC_PLATFORM];
} else {
modules = modulesMap[platform];
if (modules == null) {
modules = modulesMap[GENERIC_PLATFORM];
}
}
if (modules == null) {
return null;
}
if (modules.length > 1) {
if (!this._warnedAbout[name]) {
this._warnedAbout[name] = true;
console.warn(
chalk.yellow(
'\nWARNING: Found multiple haste modules or packages ' +
'with the name `%s`. Please fix this by adding it to ' +
'the blacklist or deleting the modules keeping only one.\n' +
'One of the following modules will be selected at random:\n%s\n'
),
name,
modules.map(m => m.path).join('\n'),
);
}
if (modules.length > 1) {
if (!this._warnedAbout[name]) {
this._warnedAbout[name] = true;
console.warn(
chalk.yellow(
'\nWARNING: Found multiple haste modules or packages ' +
'with the name `%s`. Please fix this by adding it to ' +
'the blacklist or deleting the modules keeping only one.\n' +
'One of the following modules will be selected at random:\n%s\n'
),
name,
modules.map(m => m.path).join('\n'),
);
}
const randomIndex = Math.floor(Math.random() * modules.length);
return modules[randomIndex];
}
const randomIndex = Math.floor(Math.random() * modules.length);
return modules[randomIndex];
}
return modules[0];
@ -129,15 +143,17 @@ class HasteMap {
_updateHasteMap(name, mod) {
if (this._map[name] == null) {
this._map[name] = [];
this._map[name] = Object.create(null);
}
if (mod.type === 'Module') {
// Modules takes precendence over packages.
this._map[name].unshift(mod);
} else {
this._map[name].push(mod);
const moduleMap = this._map[name];
const modulePlatform = getPlatformExtension(mod.path) || GENERIC_PLATFORM;
if (!moduleMap[modulePlatform]) {
moduleMap[modulePlatform] = [];
}
moduleMap[modulePlatform].push(mod);
}
}

View File

@ -1134,66 +1134,6 @@ describe('DependencyGraph', function() {
});
});
pit('providesModule wins when conflict with package', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'b.js': [
'/**',
' * @providesModule aPackage',
' */',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: 'main.js'
}),
'main.js': 'lol'
}
}
});
var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher,
assetExts: ['png', 'jpg'],
cache: cache,
});
return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) {
expect(deps)
.toEqual([
{
id: 'index',
path: '/root/index.js',
dependencies: ['aPackage'],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
resolveDependency: undefined,
},
{
id: 'aPackage',
path: '/root/b.js',
dependencies: [],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
resolveDependency: undefined,
},
]);
});
});
pit('should be forgiving with missing requires', function() {
var root = '/root';
fs.__setMockFilesystem({
@ -2616,6 +2556,11 @@ describe('DependencyGraph', function() {
* @providesModule a
*/
`,
'a.web.js': `
/**
* @providesModule a
*/
`,
}
});
@ -3684,6 +3629,88 @@ describe('DependencyGraph', function() {
});
});
describe('warnings', () => {
let warn = console.warn;
beforeEach(() => {
console.warn = jest.genMockFn();
});
afterEach(() => {
console.warn = warn;
});
pit('should warn about colliding module names', function() {
fs.__setMockFilesystem({
'root': {
'index.js': `
/**
* @providesModule index
*/
require('a');
`,
'a.js': `
/**
* @providesModule a
*/
`,
'b.js': `
/**
* @providesModule a
*/
`,
}
});
var dgraph = new DependencyGraph({
roots: ['/root'],
fileWatcher: fileWatcher,
assetExts: ['png', 'jpg'],
cache: cache,
});
return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) {
expect(console.warn.mock.calls.length).toBe(1);
});
});
pit('should warn about colliding module names within a platform', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.ios.js': `
/**
* @providesModule index
*/
require('a');
`,
'a.ios.js': `
/**
* @providesModule a
*/
`,
'b.ios.js': `
/**
* @providesModule a
*/
`,
}
});
var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher,
assetExts: ['png', 'jpg'],
cache: cache,
});
return getOrderedDependenciesAsJSON(dgraph, '/root/index.ios.js', 'ios').then(function(deps) {
expect(console.warn.mock.calls.length).toBe(1);
});
});
});
describe('getAsyncDependencies', () => {
pit('should get dependencies', function() {
var root = '/root';