From 78e39f371d489f70fe78f42502b3710a1643bc0f Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 6 Jul 2017 11:54:27 +0100 Subject: [PATCH 1/3] jest-haste-map: throw when trying to get a duplicated module --- .../__snapshots__/index.test.js.snap | 16 ++ .../src/__tests__/index.test.js | 16 +- packages/jest-haste-map/src/index.js | 18 +- packages/jest-haste-map/src/module_map.js | 155 +++++++++++++++--- types/HasteMap.js | 6 +- 5 files changed, 183 insertions(+), 28 deletions(-) diff --git a/packages/jest-haste-map/src/__tests__/__snapshots__/index.test.js.snap b/packages/jest-haste-map/src/__tests__/__snapshots__/index.test.js.snap index 6bc5d020b4dd..4e1c96ddd868 100644 --- a/packages/jest-haste-map/src/__tests__/__snapshots__/index.test.js.snap +++ b/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 diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index b4356fcea25f..97c77e2e9195 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/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( diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index 67a283426232..723ebe2ddd4f 100644 --- a/packages/jest-haste-map/src/index.js +++ b/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 = []; } diff --git a/packages/jest-haste-map/src/module_map.js b/packages/jest-haste-map/src/module_map.js index 16467978c633..8dd0ea6306e4 100644 --- a/packages/jest-haste-map/src/module_map.js +++ b/packages/jest-haste-map/src/module_map.js @@ -10,21 +10,24 @@ 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; - constructor(map: ModuleMapData, mocks: MockData) { - this._map = map; - this._mocks = mocks; + constructor(raw: RawModuleMap) { + this._raw = raw; } getModule( @@ -36,20 +39,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 +59,131 @@ 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; diff --git a/types/HasteMap.js b/types/HasteMap.js index 2a545192248f..d847b22ae499 100644 --- a/types/HasteMap.js +++ b/types/HasteMap.js @@ -22,10 +22,9 @@ export type WatchmanClocks = {[filepath: Path]: string}; export type HasteRegExp = RegExp | ((str: string) => boolean); export type DuplicatesIndex = { - [id: string]: { - [platform: string]: {[filePath: string]: /* type */ number}, - }, + [id: string]: {[platform: string]: DuplicatesSet}, }; +export type DuplicatesSet = {[filePath: string]: /* type */ number}; export type InternalHasteMap = {| clocks: WatchmanClocks, @@ -42,6 +41,7 @@ export type HasteMap = {| |}; export type RawModuleMap = {| + duplicates: DuplicatesIndex, map: ModuleMapData, mocks: MockData, |}; From 3ed4efa673040aa144b42b5a6a747f04e01e1c57 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 6 Jul 2017 14:16:45 +0100 Subject: [PATCH 2/3] fix lint --- packages/jest-haste-map/src/module_map.js | 25 ++++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/jest-haste-map/src/module_map.js b/packages/jest-haste-map/src/module_map.js index 8dd0ea6306e4..19228f9ff148 100644 --- a/packages/jest-haste-map/src/module_map.js +++ b/packages/jest-haste-map/src/module_map.js @@ -12,8 +12,6 @@ import type {Path} from 'types/Config'; import type { DuplicatesSet, HTypeValue, - MockData, - ModuleMapData, ModuleMetaData, RawModuleMap, } from 'types/HasteMap'; @@ -123,7 +121,7 @@ class ModuleMap { name: string, platform: string, supportsNativePlatform: boolean, - set: ?DuplicatesSet + set: ?DuplicatesSet, ) { if (set == null) { return; @@ -147,19 +145,22 @@ class DuplicateHasteCandidatesError extends Error { name: string, platform: string, supportsNativePlatform: boolean, - duplicatesSet: DuplicatesSet + 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(''), + `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; From 66dd4f55417fb44f73930036cac23e175baa98ab Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 6 Jul 2017 14:55:19 +0100 Subject: [PATCH 3/3] move type up as per comment --- types/HasteMap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/HasteMap.js b/types/HasteMap.js index d847b22ae499..30f50a65758e 100644 --- a/types/HasteMap.js +++ b/types/HasteMap.js @@ -21,10 +21,10 @@ 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]: DuplicatesSet}, }; -export type DuplicatesSet = {[filePath: string]: /* type */ number}; export type InternalHasteMap = {| clocks: WatchmanClocks,