jest-haste-map: throw when trying to get a duplicated module (jestjs#…

…3976)

* jest-haste-map: throw when trying to get a duplicated module

* fix lint

* move type up as per comment

packages/jest-haste-map/src/__tests__/__snapshots__/index.test.js.snap

@@ -1,5 +1,21 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`HasteMap file system changes processing recovery from duplicate module IDs recovers when the most recent duplicate is fixed 1`] = `
+"The name \`Pear\` was looked up in the Haste module map. It cannot be resolved, because there exists several different files, or packages, that provide a module for that particular name and platform. The platform is generic (no extension). You must delete or blacklist files until there remains only one of these:
+
+  * \`/fruits/blueberry.js\` (module)
+  * \`/fruits/pear.js\` (module)
+"
+`;
+
+exports[`HasteMap file system changes processing recovery from duplicate module IDs recovers when the oldest version of the duplicates is fixed 1`] = `
+"The name \`Pear\` was looked up in the Haste module map. It cannot be resolved, because there exists several different files, or packages, that provide a module for that particular name and platform. The platform is generic (no extension). You must delete or blacklist files until there remains only one of these:
+
+  * \`/fruits/blueberry.js\` (module)
+  * \`/fruits/pear.js\` (module)
+"
+`;
+
 exports[`HasteMap throws on duplicate module ids if "throwOnModuleCollision" is set to true 1`] = `
 [Error: jest-haste-map: @providesModule naming collision:
   Duplicate module name: Strawberry

packages/jest-haste-map/src/__tests__/index.test.js

@@ -824,7 +824,21 @@ describe('HasteMap', () => {
         e.emit('all', 'add', 'blueberry.js', '/fruits', MOCK_STAT);
         const {hasteFS, moduleMap} = await waitForItToChange(hm);
         expect(hasteFS.exists('/fruits/blueberry.js')).toBe(true);
-        expect(moduleMap.getModule('Pear')).toBe(null);
+        try {
+          moduleMap.getModule('Pear');
+          throw new Error('should be unreachable');
+        } catch (error) {
+          const {DuplicateHasteCandidatesError} = require('../module_map');
+          expect(error).toBeInstanceOf(DuplicateHasteCandidatesError);
+          expect(error.hasteName).toBe('Pear');
+          expect(error.platform).toBe('g');
+          expect(error.supportsNativePlatform).toBe(false);
+          expect(error.duplicatesSet).toEqual({
+            '/fruits/blueberry.js': 0,
+            '/fruits/pear.js': 0,
+          });
+          expect(error.message).toMatchSnapshot();
+        }
       }
 
       hm_it(

packages/jest-haste-map/src/index.js

@@ -259,7 +259,11 @@ class HasteMap extends EventEmitter {
         .then(hasteMap => {
           this._persist(hasteMap);
           const hasteFS = new HasteFS(hasteMap.files);
-          const moduleMap = new HasteModuleMap(hasteMap.map, hasteMap.mocks);
+          const moduleMap = new HasteModuleMap({
+            duplicates: hasteMap.duplicates,
+            map: hasteMap.map,
+            mocks: hasteMap.mocks,
+          });
           const __hasteMapForTest =
             (process.env.NODE_ENV === 'test' && hasteMap) || null;
           return this._watch(hasteMap, hasteFS, moduleMap).then(() => ({
@@ -281,7 +285,11 @@ class HasteMap extends EventEmitter {
 
   readModuleMap(): ModuleMap {
     const data = this.read();
-    return new HasteModuleMap(data.map, data.mocks);
+    return new HasteModuleMap({
+      duplicates: data.duplicates,
+      map: data.map,
+      mocks: data.mocks,
+    });
   }
 
   /**
@@ -605,7 +613,11 @@ class HasteMap extends EventEmitter {
         this.emit('change', {
           eventsQueue,
           hasteFS: new HasteFS(hasteMap.files),
-          moduleMap: new HasteModuleMap(hasteMap.map, hasteMap.mocks),
+          moduleMap: new HasteModuleMap({
+            duplicates: hasteMap.duplicates,
+            map: hasteMap.map,
+            mocks: hasteMap.mocks,
+          }),
         });
         eventsQueue = [];
       }

packages/jest-haste-map/src/module_map.js

@@ -10,21 +10,22 @@
 
 import type {Path} from 'types/Config';
 import type {
+  DuplicatesSet,
   HTypeValue,
-  MockData,
-  ModuleMapData,
+  ModuleMetaData,
   RawModuleMap,
 } from 'types/HasteMap';
 
 import H from './constants';
 
+const EMPTY_MAP = {};
+
 class ModuleMap {
-  _map: ModuleMapData;
-  _mocks: MockData;
+  _raw: RawModuleMap;
+  static DuplicateHasteCandidatesError: Class<DuplicateHasteCandidatesError>;
 
-  constructor(map: ModuleMapData, mocks: MockData) {
-    this._map = map;
-    this._mocks = mocks;
+  constructor(raw: RawModuleMap) {
+    this._raw = raw;
   }
 
   getModule(
@@ -36,20 +37,14 @@ class ModuleMap {
     if (!type) {
       type = H.MODULE;
     }
-
-    const map = this._map[name];
-    if (map) {
-      let module = platform && map[platform];
-      if (!module && map[H.NATIVE_PLATFORM] && supportsNativePlatform) {
-        module = map[H.NATIVE_PLATFORM];
-      } else if (!module) {
-        module = map[H.GENERIC_PLATFORM];
-      }
-      if (module && module[H.TYPE] === type) {
-        return module[H.PATH];
-      }
+    const module = this._getModuleMetadata(
+      name,
+      platform,
+      !!supportsNativePlatform,
+    );
+    if (module && module[H.TYPE] === type) {
+      return module[H.PATH];
     }
-
     return null;
   }
 
@@ -62,15 +57,134 @@ class ModuleMap {
   }
 
   getMockModule(name: string): ?Path {
-    return this._mocks[name];
+    return this._raw.mocks[name];
   }
 
   getRawModuleMap(): RawModuleMap {
     return {
-      map: this._map,
-      mocks: this._mocks,
+      duplicates: this._raw.duplicates,
+      map: this._raw.map,
+      mocks: this._raw.mocks,
     };
   }
+
+  /**
+   * When looking up a module's data, we walk through each eligible platform for
+   * the query. For each platform, we want to check if there are known
+   * duplicates for that name+platform pair. The duplication logic normally
+   * removes elements from the `map` object, but we want to check upfront to be
+   * extra sure. If metadata exists both in the `duplicates` object and the
+   * `map`, this would be a bug.
+   */
+  _getModuleMetadata(
+    name: string,
+    platform: ?string,
+    supportsNativePlatform: boolean,
+  ): ?ModuleMetaData {
+    const map = this._raw.map[name] || EMPTY_MAP;
+    const dupMap = this._raw.duplicates[name] || EMPTY_MAP;
+    if (platform != null) {
+      this._assertNoDuplicates(
+        name,
+        platform,
+        supportsNativePlatform,
+        dupMap[platform],
+      );
+      if (map[platform] != null) {
+        return map[platform];
+      }
+    }
+    if (supportsNativePlatform) {
+      this._assertNoDuplicates(
+        name,
+        H.NATIVE_PLATFORM,
+        supportsNativePlatform,
+        dupMap[H.NATIVE_PLATFORM],
+      );
+      if (map[H.NATIVE_PLATFORM]) {
+        return map[H.NATIVE_PLATFORM];
+      }
+    }
+    this._assertNoDuplicates(
+      name,
+      H.GENERIC_PLATFORM,
+      supportsNativePlatform,
+      dupMap[H.GENERIC_PLATFORM],
+    );
+    if (map[H.GENERIC_PLATFORM]) {
+      return map[H.GENERIC_PLATFORM];
+    }
+    return null;
+  }
+
+  _assertNoDuplicates(
+    name: string,
+    platform: string,
+    supportsNativePlatform: boolean,
+    set: ?DuplicatesSet,
+  ) {
+    if (set == null) {
+      return;
+    }
+    throw new DuplicateHasteCandidatesError(
+      name,
+      platform,
+      supportsNativePlatform,
+      set,
+    );
+  }
+}
+
+class DuplicateHasteCandidatesError extends Error {
+  hasteName: string;
+  platform: ?string;
+  supportsNativePlatform: boolean;
+  duplicatesSet: DuplicatesSet;
+
+  constructor(
+    name: string,
+    platform: string,
+    supportsNativePlatform: boolean,
+    duplicatesSet: DuplicatesSet,
+  ) {
+    const platformMessage = getPlatformMessage(platform);
+    super(
+      `The name \`${name}\` was looked up in the Haste module map. It ` +
+        `cannot be resolved, because there exists several different ` +
+        `files, or packages, that provide a module for ` +
+        `that particular name and platform. ${platformMessage} You must ` +
+        `delete or blacklist files until there remains only one of these:\n\n` +
+        Object.keys(duplicatesSet)
+          .sort()
+          .map(dupFilePath => {
+            const typeMessage = getTypeMessage(duplicatesSet[dupFilePath]);
+            return `  * \`${dupFilePath}\` (${typeMessage})\n`;
+          })
+          .join(''),
+    );
+    this.hasteName = name;
+    this.platform = platform;
+    this.supportsNativePlatform = supportsNativePlatform;
+    this.duplicatesSet = duplicatesSet;
+  }
+}
+
+function getPlatformMessage(platform: string) {
+  if (platform === H.GENERIC_PLATFORM) {
+    return 'The platform is generic (no extension).';
+  }
+  return `The platform extension is \`${platform}\`.`;
+}
+
+function getTypeMessage(type: number) {
+  switch (type) {
+    case H.MODULE:
+      return 'module';
+    case H.PACKAGE:
+      return 'package';
+  }
+  return 'unknown';
 }
 
+ModuleMap.DuplicateHasteCandidatesError = DuplicateHasteCandidatesError;
 module.exports = ModuleMap;

types/HasteMap.js

@@ -21,10 +21,9 @@ export type ModuleMapData = {[id: string]: ModuleMapItem};
 export type WatchmanClocks = {[filepath: Path]: string};
 export type HasteRegExp = RegExp | ((str: string) => boolean);
 
+export type DuplicatesSet = {[filePath: string]: /* type */ number};
 export type DuplicatesIndex = {
-  [id: string]: {
-    [platform: string]: {[filePath: string]: /* type */ number},
-  },
+  [id: string]: {[platform: string]: DuplicatesSet},
 };
 
 export type InternalHasteMap = {|
@@ -42,6 +41,7 @@ export type HasteMap = {|
 |};
 
 export type RawModuleMap = {|
+  duplicates: DuplicatesIndex,
   map: ModuleMapData,
   mocks: MockData,
 |};