From 3cc83da40318a314c30c8b7f8a979c4347142b40 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 20 Jul 2017 10:13:58 -0700 Subject: [PATCH] metro-bundler: Server: report ambiguous module resolution better Reviewed By: mjesun Differential Revision: D5444129 fbshipit-source-id: 311d20c7ee4c00ec2d4c52d83bd6c5a94221b552 --- packages/metro-bundler/src/Server/index.js | 20 ++++++++++ .../metro-bundler/src/lib/TerminalReporter.js | 25 +++++++++++- .../DependencyGraph/ResolutionRequest.js | 40 ++++++++++++++++++- .../__tests__/DependencyGraph-test.js | 6 +-- 4 files changed, 85 insertions(+), 6 deletions(-) diff --git a/packages/metro-bundler/src/Server/index.js b/packages/metro-bundler/src/Server/index.js index 202960a4..385b1f6e 100644 --- a/packages/metro-bundler/src/Server/index.js +++ b/packages/metro-bundler/src/Server/index.js @@ -26,6 +26,10 @@ const path = require('path'); const symbolicate = require('./symbolicate'); const url = require('url'); +const { + AmbiguousModuleResolutionError, +} = require('../node-haste/DependencyGraph/ResolutionRequest'); + import type Module, {HasteImpl} from '../node-haste/Module'; import type {IncomingMessage, ServerResponse} from 'http'; import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionResponse'; @@ -890,6 +894,22 @@ class Server { 'Content-Type': 'application/json; charset=UTF-8', }); + if (error instanceof AmbiguousModuleResolutionError) { + const he = error.hasteError; + const message = + "Ambiguous resolution: module '" + + `${error.fromModulePath}\' tries to require \'${he.hasteName}\', but ` + + `there are several files providing this module. You can delete or ` + + 'fix them: \n\n' + + Object.keys(he.duplicatesSet) + .sort() + .map(dupFilePath => `${dupFilePath}`) + .join('\n\n'); + res.end(JSON.stringify({message, errors: [{description: message}]})); + this._reporter.update({error, type: 'bundling_error'}); + return; + } + if ( error instanceof Error && (error.type === 'TransformError' || diff --git a/packages/metro-bundler/src/lib/TerminalReporter.js b/packages/metro-bundler/src/lib/TerminalReporter.js index 5798a1bc..4abe1aa0 100644 --- a/packages/metro-bundler/src/lib/TerminalReporter.js +++ b/packages/metro-bundler/src/lib/TerminalReporter.js @@ -18,6 +18,10 @@ const reporting = require('./reporting'); const throttle = require('lodash/throttle'); const util = require('util'); +const { + AmbiguousModuleResolutionError, +} = require('../node-haste/DependencyGraph/ResolutionRequest'); + import type Transformer from '../JSTransformer'; import type {BundleOptions} from '../Server'; import type Terminal from './Terminal'; @@ -250,6 +254,21 @@ class TerminalReporter { * is not actionable to end users. */ _logBundlingError(error: Error | Transformer.TransformError) { + if (error instanceof AmbiguousModuleResolutionError) { + const he = error.hasteError; + const message = + 'ambiguous resolution: module `' + + `${error.fromModulePath}\` tries to require \`${he.hasteName}\`, but ` + + `there are several files providing this module. You can delete or ` + + 'fix them: \n\n' + + Object.keys(he.duplicatesSet) + .sort() + .map(dupFilePath => ` * \`${dupFilePath}\`\n`) + .join(''); + this._logBundlingErrorMessage(message); + return; + } + //$FlowFixMe T19379628 let message = error.message; //$FlowFixMe T19379628 @@ -264,7 +283,11 @@ class TerminalReporter { str += '\n' + error.snippet; } - reporting.logError(this.terminal, 'bundling failed: %s', str); + this._logBundlingErrorMessage(str); + } + + _logBundlingErrorMessage(message: string) { + reporting.logError(this.terminal, 'bundling failed: %s', message); } _logWorkerChunk(origin: 'stdout' | 'stderr', chunk: string) { diff --git a/packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionRequest.js b/packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionRequest.js index d132b7d9..71d268fb 100644 --- a/packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -20,6 +20,10 @@ const debug = require('debug')('Metro:DependencyGraph'); const isAbsolutePath = require('absolute-path'); const path = require('path'); +const { + DuplicateHasteCandidatesError, +} = require('jest-haste-map/build/module_map'); + import type DependencyGraphHelpers from './DependencyGraphHelpers'; import type ResolutionResponse from './ResolutionResponse'; import type {Options as TransformWorkerOptions} from '../../JSTransformer/worker'; @@ -68,6 +72,7 @@ type Options = {| class ResolutionRequest { _immediateResolutionCache: {[key: string]: TModule}; _options: Options; + static AmbiguousModuleResolutionError: Class; constructor(options: Options) { this._options = options; @@ -95,8 +100,7 @@ class ResolutionRequest { !(isRelativeImport(toModuleName) || isAbsolutePath(toModuleName)) ) { const result = ModuleResolution.tryResolveSync( - () => - resolver.resolveHasteDependency(fromModule, toModuleName, platform), + () => this._resolveHasteDependency(fromModule, toModuleName, platform), () => resolver.resolveNodeDependency(fromModule, toModuleName, platform), ); @@ -108,6 +112,22 @@ class ResolutionRequest { ); } + _resolveHasteDependency( + fromModule: TModule, + toModuleName: string, + platform: string | null, + ): TModule { + const rs = this._options.moduleResolver; + try { + return rs.resolveHasteDependency(fromModule, toModuleName, platform); + } catch (error) { + if (error instanceof DuplicateHasteCandidatesError) { + throw new AmbiguousModuleResolutionError(fromModule.path, error); + } + throw error; + } + } + resolveModuleDependencies( module: TModule, dependencyNames: $ReadOnlyArray, @@ -350,4 +370,20 @@ function resolutionHash(modulePath, depName) { return `${path.resolve(modulePath)}:${depName}`; } +class AmbiguousModuleResolutionError extends Error { + fromModulePath: string; + hasteError: DuplicateHasteCandidatesError; + + constructor( + fromModulePath: string, + hasteError: DuplicateHasteCandidatesError, + ) { + super(); + this.fromModulePath = fromModulePath; + this.hasteError = hasteError; + } +} + +ResolutionRequest.AmbiguousModuleResolutionError = AmbiguousModuleResolutionError; + module.exports = ResolutionRequest; diff --git a/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js b/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js index ae8cfe32..954e162f 100644 --- a/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js +++ b/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js @@ -5295,9 +5295,9 @@ describe('DependencyGraph', function() { throw new Error('expected `getOrderedDependenciesAsJSON` to fail'); } catch (error) { const { - DuplicateHasteCandidatesError, - } = require('jest-haste-map/build/module_map'); - if (!(error instanceof DuplicateHasteCandidatesError)) { + AmbiguousModuleResolutionError, + } = require('../DependencyGraph/ResolutionRequest'); + if (!(error instanceof AmbiguousModuleResolutionError)) { throw error; } expect(console.warn).toBeCalled();