mirror of https://github.com/status-im/metro.git
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:
parent
d475e14cc2
commit
5bb1226b64
|
@ -8,8 +8,9 @@
|
|||
*/
|
||||
'use strict';
|
||||
|
||||
const chalk = require('chalk');
|
||||
const path = require('path');
|
||||
const getPontentialPlatformExt = require('../../lib/getPlatformExtension');
|
||||
const getPlatformExtension = require('../../lib/getPlatformExtension');
|
||||
|
||||
class HasteMap {
|
||||
constructor({ fastfs, moduleCache, helpers }) {
|
||||
|
@ -17,6 +18,7 @@ class HasteMap {
|
|||
this._moduleCache = moduleCache;
|
||||
this._helpers = helpers;
|
||||
this._map = Object.create(null);
|
||||
this._warnedAbout = Object.create(null);
|
||||
}
|
||||
|
||||
build() {
|
||||
|
@ -35,6 +37,9 @@ class HasteMap {
|
|||
|
||||
processFileChange(type, absPath) {
|
||||
return Promise.resolve().then(() => {
|
||||
// Rewarn after file changes.
|
||||
this._warnedAbout = Object.create(null);
|
||||
|
||||
/*eslint no-labels: 0 */
|
||||
if (type === 'delete' || type === 'change') {
|
||||
loop: for (let name in this._map) {
|
||||
|
@ -64,19 +69,39 @@ class HasteMap {
|
|||
}
|
||||
|
||||
getModule(name, platform = null) {
|
||||
if (this._map[name]) {
|
||||
const modules = this._map[name];
|
||||
if (platform != null) {
|
||||
for (let i = 0; i < modules.length; i++) {
|
||||
if (getPontentialPlatformExt(modules[i].path) === platform) {
|
||||
return modules[i];
|
||||
}
|
||||
if (!this._map[name]) {
|
||||
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];
|
||||
}
|
||||
}
|
||||
|
||||
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) {
|
||||
|
|
|
@ -252,6 +252,11 @@ class ResolutionRequest {
|
|||
_loadAsFile(potentialModulePath) {
|
||||
return Promise.resolve().then(() => {
|
||||
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);
|
||||
|
||||
let pattern = '^' + name + '(@[\\d\\.]+x)?';
|
||||
|
@ -263,7 +268,7 @@ class ResolutionRequest {
|
|||
// We arbitrarly grab the first one, because scale selection
|
||||
// will happen somewhere
|
||||
const [assetFile] = this._fastfs.matches(
|
||||
path.dirname(potentialModulePath),
|
||||
dirname,
|
||||
new RegExp(pattern)
|
||||
);
|
||||
|
||||
|
|
|
@ -10,7 +10,7 @@
|
|||
|
||||
const path = require('path');
|
||||
|
||||
const SUPPORTED_PLATFORM_EXTS = ['android', 'ios'];
|
||||
const SUPPORTED_PLATFORM_EXTS = ['android', 'ios', 'web'];
|
||||
|
||||
const re = new RegExp(
|
||||
'[^\\.]+\\.(' + SUPPORTED_PLATFORM_EXTS.join('|') + ')\\.\\w+$'
|
||||
|
|
Loading…
Reference in New Issue