status-im
/
metro
mirror of https://github.com/status-im/metro.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Error on name collisions 

Summary: @​public
This moves us from warnings on name collisions to errors. If the error happens in initialization it will fatal out.

However, if the error happens while working (after initialization) then I did my best to make it recoverable. The rational behind this is that if you're working and you're changing names, you may introduce a duplication while moving things around. It will suck if you have to restart the server every time you do that.

Reviewed By: @frantic

Differential Revision: D2493098
This commit is contained in:
Amjad Masad 2015-09-30 19:02:52 -07:00 committed by facebook-github-bot-5
parent af90440f97
commit 289989dc3f
5 changed files with 58 additions and 169 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js vendored
View File

@ -13,6 +13,7 @@ const AssetModule_DEPRECATED = require('../AssetModule_DEPRECATED');
const Fastfs = require('../fastfs');
const debug = require('debug')('ReactNativePackager:DependencyGraph');
const path = require('path');
const Promise = require('promise');
class DeprecatedAssetMap {
constructor({ fsCrawl, roots, assetExts, fileWatcher, ignoreFilePath, helpers }) {

61
react-packager/src/DependencyResolver/DependencyGraph/HasteMap.js vendored
View File

@ -8,9 +8,9 @@
*/
'use strict';
const chalk = require('chalk');
const path = require('path');
const getPlatformExtension = require('../../lib/getPlatformExtension');
const Promise = require('promise');
const GENERIC_PLATFORM = 'generic';
@ -19,11 +19,11 @@ class HasteMap {
this._fastfs = fastfs;
this._moduleCache = moduleCache;
this._helpers = helpers;
this._map = Object.create(null);
this._warnedAbout = Object.create(null);
}
build() {
this._map = Object.create(null);
let promises = this._fastfs.findFilesByExt('js', {
ignore: (file) => this._helpers.isNodeModulesDir(file)
}).map(file => this._processHasteModule(file));
@ -39,20 +39,15 @@ 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) {
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;
}
const module = modulesMap[platform];
if (module.path === absPath) {
delete modulesMap[platform];
break loop;
}
}
}
@ -82,38 +77,15 @@ class HasteMap {
// 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];
return modulesMap[GENERIC_PLATFORM];
} else {
modules = modulesMap[platform];
if (modules == null) {
modules = modulesMap[GENERIC_PLATFORM];
let module = modulesMap[platform];
if (module == null) {
module = modulesMap[GENERIC_PLATFORM];
}
return module;
}
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'
),
name,
modules.map(m => m.path).join('\n'),
);
}
return modules[0];
}
return modules[0];
}
_processHasteModule(file) {
@ -147,11 +119,14 @@ class HasteMap {
const moduleMap = this._map[name];
const modulePlatform = getPlatformExtension(mod.path) || GENERIC_PLATFORM;
if (!moduleMap[modulePlatform]) {
moduleMap[modulePlatform] = [];
if (moduleMap[modulePlatform]) {
throw new Error(
`Naming collision detected: ${mod.path} ` +
`collides with ${moduleMap[modulePlatform].path}`
);
}
moduleMap[modulePlatform].push(mod);
moduleMap[modulePlatform] = mod;
}
}

1
react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js vendored
View File

@ -13,6 +13,7 @@ const util = require('util');
const path = require('path');
const isAbsolutePath = require('absolute-path');
const getAssetDataFromName = require('../../lib/getAssetDataFromName');
const Promise = require('promise');
class ResolutionRequest {
constructor({

136
react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js vendored
View File

@ -1070,7 +1070,7 @@ describe('DependencyGraph', function() {
});
});
pit('can have multiple modules with the same name', function() {
pit('should fatal on multiple modules with the same name', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
@ -1078,59 +1078,33 @@ describe('DependencyGraph', function() {
'/**',
' * @providesModule index',
' */',
'require("b")',
].join('\n'),
'b.js': [
'/**',
' * @providesModule b',
' * @providesModule index',
' */',
].join('\n'),
'c.js': [
'/**',
' * @providesModule c',
' */',
].join('\n'),
'somedir': {
'somefile.js': [
'/**',
' * @providesModule index',
' */',
'require("c")',
].join('\n')
}
}
});
const _exit = process.exit;
const _error = console.error;
process.exit = jest.genMockFn();
console.error = jest.genMockFn();
var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher,
assetExts: ['png', 'jpg'],
cache: cache,
});
return getOrderedDependenciesAsJSON(dgraph, '/root/somedir/somefile.js').then(function(deps) {
expect(deps)
.toEqual([
{
id: 'index',
path: '/root/somedir/somefile.js',
dependencies: ['c'],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
},
{
id: 'c',
path: '/root/c.js',
dependencies: [],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
},
]);
return dgraph.load().catch(() => {
expect(process.exit).toBeCalledWith(1);
expect(console.error).toBeCalled();
process.exit = _exit;
console.error = _error;
});
});
@ -3629,88 +3603,6 @@ 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';

28
react-packager/src/DependencyResolver/DependencyGraph/index.js vendored
View File

@ -73,7 +73,11 @@ class DependencyGraph {
this._opts = validateOpts(options);
this._cache = this._opts.cache;
this._helpers = new Helpers(this._opts);
this.load();
this.load().catch((err) => {
// This only happens at initialization. Live errors are easier to recover from.
console.error('Error building DepdendencyGraph:\n', err.stack);
process.exit(1);
});
}
load() {
@ -198,9 +202,25 @@ class DependencyGraph {
return;
}
this._loading = this._loading.then(
() => this._hasteMap.processFileChange(type, absPath)
);
// Ok, this is some tricky promise code. Our requirements are:
// * we need to report back failures
// * failures shouldn't block recovery
// * Errors can leave `hasteMap` in an incorrect state, and we need to rebuild
// After we process a file change we record any errors which will also be
// reported via the next request. On the next file change, we'll see that
// we are in an error state and we should decide to do a full rebuild.
this._loading = this._loading.finally(() => {
if (this._hasteMapError) {
this._hasteMapError = null;
// Rebuild the entire map if last change resulted in an error.
console.warn('Rebuilding haste map to recover from error');
this._loading = this._hasteMap.build();
} else {
this._loading = this._hasteMap.processFileChange(type, absPath);
this._loading.catch((e) => this._hasteMapError = e);
}
return this._loading;
});
}
}