Fix duplicate module detection (#7333)

* Fix duplicate module detection in haste map * Fix failing test due to differences in contexts
jestjs · Nov 7, 2018 · 2b18a26 · 2b18a26
1 parent fa0e814
commit 2b18a26
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 +70,7 @@
 - `[jest-haste-map]` Fix to resolve path that is start with words same as rootDir ([#7324](https://github.com/facebook/jest/pull/7324))
 - `[expect]` Fix toMatchObject matcher when used with `Object.create(null)` ([#7334](https://github.com/facebook/jest/pull/7334))
 - `[jest-haste-map]` [**BREAKING**] Recover files correctly after haste name collisions are fixed ([#7329](https://github.com/facebook/jest/pull/7329))
+- `[jest-haste-map]` Remove legacy condition for duplicate module detection ([#7333](https://github.com/facebook/jest/pull/7333))
 
 ### Chore & Maintenance
 

diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js
@@ -108,18 +108,25 @@ const object = data => Object.assign(Object.create(null), data);
 const createMap = obj => new Map(Object.keys(obj).map(key => [key, obj[key]]));
 
 // Jest toEqual does not match Map instances from different contexts
-const normalizePersisted = hasteMap => ({
-  clocks: normalizeMap(hasteMap.clocks),
-  duplicates: normalizeMap(hasteMap.duplicates),
-  files: normalizeMap(hasteMap.files),
-  map: normalizeMap(hasteMap.map),
-  mocks: normalizeMap(hasteMap.mocks),
-});
-const normalizeMap = map => {
-  if (Object.prototype.toString.call(map) !== '[object Map]') {
-    throw new TypeError('expected map instance');
+// This normalizes them for the uses cases in this test
+const useBuitinsInContext = value => {
+  const stringTag = Object.prototype.toString.call(value);
+  switch (stringTag) {
+    case '[object Map]':
+      return new Map(
+        Array.from(value).map(([k, v]) => [
+          useBuitinsInContext(k),
+          useBuitinsInContext(v),
+        ]),
+      );
+    case '[object Object]':
+      return Object.keys(value).reduce((obj, key) => {
+        obj[key] = useBuitinsInContext(value[key]);
+        return obj;
+      }, {});
+    default:
+      return value;
   }
-  return new Map(map);
 };
 
 let consoleWarn;
@@ -359,7 +366,7 @@ describe('HasteMap', () => {
 
       // The cache file must exactly mirror the data structure returned from a
       // build
-      expect(normalizePersisted(hasteMap.read())).toEqual(data);
+      expect(useBuitinsInContext(hasteMap.read())).toEqual(data);
     });
   });
 
@@ -433,7 +440,7 @@ describe('HasteMap', () => {
           }),
         );
 
-        expect(normalizePersisted(hasteMap.read())).toEqual(data);
+        expect(useBuitinsInContext(hasteMap.read())).toEqual(data);
       });
     });
   });
@@ -523,6 +530,18 @@ describe('HasteMap', () => {
       });
   });
 
+  it('warns on duplicate module ids only once', async () => {
+    mockFs['/project/fruits/other/Strawberry.js'] = `
+      const Banana = require("Banana");
+    `;
+
+    await new HasteMap(defaultConfig).build();
+    expect(console.warn).toHaveBeenCalledTimes(1);
+
+    await new HasteMap(defaultConfig).build();
+    expect(console.warn).toHaveBeenCalledTimes(1);
+  });
+
   it('throws on duplicate module ids if "throwOnModuleCollision" is set to true', () => {
     // Raspberry thinks it is a Strawberry
     mockFs['/project/fruits/another/Strawberry.js'] = `
@@ -615,9 +634,9 @@ describe('HasteMap', () => {
             } else {
               expect(fs.readFileSync).toBeCalledWith(cacheFilePath, 'utf8');
             }
-            expect(normalizeMap(data.clocks)).toEqual(mockClocks);
-            expect(normalizeMap(data.files)).toEqual(initialData.files);
-            expect(normalizeMap(data.map)).toEqual(initialData.map);
+            expect(useBuitinsInContext(data.clocks)).toEqual(mockClocks);
+            expect(useBuitinsInContext(data.files)).toEqual(initialData.files);
+            expect(useBuitinsInContext(data.map)).toEqual(initialData.map);
           });
       }));
 
@@ -655,15 +674,15 @@ describe('HasteMap', () => {
               'utf8',
             );
 
-            expect(normalizeMap(data.clocks)).toEqual(mockClocks);
+            expect(useBuitinsInContext(data.clocks)).toEqual(mockClocks);
 
             const files = new Map(initialData.files);
             files.set('fruits/Banana.js', ['Banana', 32, 1, ['Kiwi'], null]);
 
-            expect(normalizeMap(data.files)).toEqual(files);
+            expect(useBuitinsInContext(data.files)).toEqual(files);
 
             const map = new Map(initialData.map);
-            expect(normalizeMap(data.map)).toEqual(map);
+            expect(useBuitinsInContext(data.map)).toEqual(map);
           });
       }));
 
@@ -690,11 +709,11 @@ describe('HasteMap', () => {
           .then(({__hasteMapForTest: data}) => {
             const files = new Map(initialData.files);
             files.delete('fruits/Banana.js');
-            expect(normalizeMap(data.files)).toEqual(files);
+            expect(useBuitinsInContext(data.files)).toEqual(files);
 
             const map = new Map(initialData.map);
             map.delete('Banana');
-            expect(normalizeMap(data.map)).toEqual(map);
+            expect(useBuitinsInContext(data.map)).toEqual(map);
           });
       }));
 
@@ -781,7 +800,7 @@ describe('HasteMap', () => {
       const {__hasteMapForTest: data} = await new HasteMap(
         defaultConfig,
       ).build();
-      expect(normalizeMap(data.duplicates)).toEqual(
+      expect(useBuitinsInContext(data.duplicates)).toEqual(
         createMap({
           Strawberry: createMap({
             g: createMap({
@@ -807,7 +826,7 @@ describe('HasteMap', () => {
       const {__hasteMapForTest: data} = await new HasteMap(
         defaultConfig,
       ).build();
-      expect(normalizeMap(data.duplicates)).toEqual(new Map());
+      expect(useBuitinsInContext(data.duplicates)).toEqual(new Map());
       expect(data.map.get('Strawberry')).toEqual({
         g: ['fruits/Strawberry.js', H.MODULE],
       });
@@ -826,7 +845,7 @@ describe('HasteMap', () => {
         defaultConfig,
       ).build();
 
-      expect(normalizeMap(data.duplicates)).toEqual(
+      expect(useBuitinsInContext(data.duplicates)).toEqual(
         createMap({
           Strawberry: createMap({
             g: createMap({
@@ -853,7 +872,7 @@ describe('HasteMap', () => {
         defaultConfig,
       ).build();
 
-      expect(normalizeMap(correctData.duplicates)).toEqual(new Map());
+      expect(useBuitinsInContext(correctData.duplicates)).toEqual(new Map());
       expect(correctData.map.get('Strawberry')).toEqual({
         g: ['fruits/Strawberry.js', H.MODULE],
       });
@@ -874,7 +893,7 @@ describe('HasteMap', () => {
       const {__hasteMapForTest: data} = await new HasteMap(
         defaultConfig,
       ).build();
-      expect(normalizeMap(data.duplicates)).toEqual(new Map());
+      expect(useBuitinsInContext(data.duplicates)).toEqual(new Map());
       expect(data.map.get('Strawberry')).toEqual({
         g: ['fruits/Strawberry.js', H.MODULE],
       });

diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js
@@ -375,10 +375,9 @@ class HasteMap extends EventEmitter {
           cachedFiles.push({moduleName, path: relativeFilePath});
         }
         return this._crawl(cachedHasteMap).then(hasteMap => {
-          const deprecatedFiles = cachedFiles.filter(file => {
-            const fileData = hasteMap.files.get(file.path);
-            return fileData == null || file.moduleName !== fileData[H.ID];
-          });
+          const deprecatedFiles = cachedFiles.filter(
+            file => !hasteMap.files.has(file.path),
+          );
           return {deprecatedFiles, hasteMap};
         });
       });