Warn and randomize colliding name selection

Summary: @​public

The issue of colliding haste modules have came up many times and have wasted countless engineering hours. This will start warning about it and will also start selecting modules at random so that people don't depend on undefined behavior.

Additionally, this surfaced an issue where with assets we may fatally throw if the directory doesn't exist. This is fixed by checking the existence of the directory before trying to match files in it.

Reviewed By: @jingc

Differential Revision: D2478480
This commit is contained in:
Amjad Masad 2015-09-25 00:17:15 -07:00 committed by facebook-github-bot-8
parent bf88e46ebb
commit 9293e54085
3 changed files with 42 additions and 12 deletions

View File

@ -8,8 +8,9 @@
*/ */
'use strict'; 'use strict';
const chalk = require('chalk');
const path = require('path'); const path = require('path');
const getPontentialPlatformExt = require('../../lib/getPlatformExtension'); const getPlatformExtension = require('../../lib/getPlatformExtension');
class HasteMap { class HasteMap {
constructor({ fastfs, moduleCache, helpers }) { constructor({ fastfs, moduleCache, helpers }) {
@ -17,6 +18,7 @@ class HasteMap {
this._moduleCache = moduleCache; this._moduleCache = moduleCache;
this._helpers = helpers; this._helpers = helpers;
this._map = Object.create(null); this._map = Object.create(null);
this._warnedAbout = Object.create(null);
} }
build() { build() {
@ -35,6 +37,9 @@ class HasteMap {
processFileChange(type, absPath) { processFileChange(type, absPath) {
return Promise.resolve().then(() => { return Promise.resolve().then(() => {
// Rewarn after file changes.
this._warnedAbout = Object.create(null);
/*eslint no-labels: 0 */ /*eslint no-labels: 0 */
if (type === 'delete' || type === 'change') { if (type === 'delete' || type === 'change') {
loop: for (let name in this._map) { loop: for (let name in this._map) {
@ -64,19 +69,39 @@ class HasteMap {
} }
getModule(name, platform = null) { getModule(name, platform = null) {
if (this._map[name]) { if (!this._map[name]) {
const modules = this._map[name]; return null;
if (platform != null) { }
for (let i = 0; i < modules.length; i++) {
if (getPontentialPlatformExt(modules[i].path) === platform) { const modules = this._map[name];
return modules[i]; if (platform != null) {
} for (let i = 0; i < modules.length; i++) {
if (getPlatformExtension(modules[i].path) === platform) {
return modules[i];
} }
} }
return modules[0]; 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];
}
} }
return null;
return modules[0];
} }
_processHasteModule(file) { _processHasteModule(file) {

View File

@ -252,6 +252,11 @@ class ResolutionRequest {
_loadAsFile(potentialModulePath) { _loadAsFile(potentialModulePath) {
return Promise.resolve().then(() => { return Promise.resolve().then(() => {
if (this._helpers.isAssetFile(potentialModulePath)) { if (this._helpers.isAssetFile(potentialModulePath)) {
const dirname = path.dirname(potentialModulePath);
if (!this._fastfs.dirExists(dirname)) {
throw new UnableToResolveError(`Directory ${dirname} doesn't exist`);
}
const {name, type} = getAssetDataFromName(potentialModulePath); const {name, type} = getAssetDataFromName(potentialModulePath);
let pattern = '^' + name + '(@[\\d\\.]+x)?'; let pattern = '^' + name + '(@[\\d\\.]+x)?';
@ -263,7 +268,7 @@ class ResolutionRequest {
// We arbitrarly grab the first one, because scale selection // We arbitrarly grab the first one, because scale selection
// will happen somewhere // will happen somewhere
const [assetFile] = this._fastfs.matches( const [assetFile] = this._fastfs.matches(
path.dirname(potentialModulePath), dirname,
new RegExp(pattern) new RegExp(pattern)
); );

View File

@ -10,7 +10,7 @@
const path = require('path'); const path = require('path');
const SUPPORTED_PLATFORM_EXTS = ['android', 'ios']; const SUPPORTED_PLATFORM_EXTS = ['android', 'ios', 'web'];
const re = new RegExp( const re = new RegExp(
'[^\\.]+\\.(' + SUPPORTED_PLATFORM_EXTS.join('|') + ')\\.\\w+$' '[^\\.]+\\.(' + SUPPORTED_PLATFORM_EXTS.join('|') + ')\\.\\w+$'