diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js index ca537078f..7292a85ce 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js @@ -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 }) { diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/HasteMap.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/HasteMap.js index 6d720ea50..6c8bdc0a4 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/HasteMap.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/HasteMap.js @@ -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; } } diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js index f3a755375..dda09abe8 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js @@ -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({ diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index 017ffe8e7..1a9350fa5 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -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'; diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js index a03cd3dbe..1baef52f3 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js @@ -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; + }); } }