From ffd7d6279e13c35ed1a0e7b67163ec95f3a7fc33 Mon Sep 17 00:00:00 2001 From: Rafael Oleza Date: Mon, 5 Nov 2018 15:20:18 +0100 Subject: [PATCH 1/3] Fix type of module when recovering from a haste collision --- .../src/__tests__/index.test.js | 65 ++++++++++++++++--- packages/jest-haste-map/src/index.js | 15 +++-- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index 3b1a49f37626..033209cf0b0b 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -784,7 +784,10 @@ describe('HasteMap', () => { expect(normalizeMap(data.duplicates)).toEqual( createMap({ Strawberry: { - g: {'fruits/Strawberry.js': 0, 'fruits/another/Strawberry.js': 0}, + g: { + 'fruits/Strawberry.js': H.MODULE, + 'fruits/another/Strawberry.js': H.MODULE, + }, }, }), ); @@ -806,10 +809,54 @@ describe('HasteMap', () => { ).build(); expect(normalizeMap(data.duplicates)).toEqual(new Map()); expect(data.map.get('Strawberry')).toEqual({ - g: ['fruits/Strawberry.js', 0], + g: ['fruits/Strawberry.js', H.MODULE], }); // Make sure the other files are not affected. - expect(data.map.get('Banana')).toEqual({g: ['fruits/Banana.js', 0]}); + expect(data.map.get('Banana')).toEqual({ + g: ['fruits/Banana.js', H.MODULE], + }); + }); + + it('recovers with the correct type when a duplicate file is deleted', async () => { + mockFs['/project/fruits/strawberryPackage/package.json'] = ` + {"name": "Strawberry"} + `; + + const {__hasteMapForTest: data} = await new HasteMap( + defaultConfig, + ).build(); + + expect(normalizeMap(data.duplicates)).toEqual( + createMap({ + Strawberry: { + g: { + 'fruits/Strawberry.js': H.MODULE, + 'fruits/another/Strawberry.js': H.MODULE, + 'fruits/strawberryPackage/package.json': H.PACKAGE, + }, + }, + }), + ); + + delete mockFs['/project/fruits/another/Strawberry.js']; + delete mockFs['/project/fruits/strawberryPackage/package.json']; + + mockChangedFiles = object({ + '/project/fruits/another/Strawberry.js': null, + '/project/fruits/strawberryPackage/package.json': null, + }); + mockClocks = createMap({ + fruits: 'c:fake-clock:4', + }); + + const {__hasteMapForTest: correctData} = await new HasteMap( + defaultConfig, + ).build(); + + expect(normalizeMap(correctData.duplicates)).toEqual(new Map()); + expect(correctData.map.get('Strawberry')).toEqual({ + g: ['fruits/Strawberry.js', H.MODULE], + }); }); it('recovers when a duplicate module is renamed', async () => { @@ -829,13 +876,15 @@ describe('HasteMap', () => { ).build(); expect(normalizeMap(data.duplicates)).toEqual(new Map()); expect(data.map.get('Strawberry')).toEqual({ - g: ['fruits/Strawberry.js', 0], + g: ['fruits/Strawberry.js', H.MODULE], }); expect(data.map.get('Pineapple')).toEqual({ - g: ['fruits/another/Pineapple.js', 0], + g: ['fruits/another/Pineapple.js', H.MODULE], }); // Make sure the other files are not affected. - expect(data.map.get('Banana')).toEqual({g: ['fruits/Banana.js', 0]}); + expect(data.map.get('Banana')).toEqual({ + g: ['fruits/Banana.js', H.MODULE], + }); }); }); @@ -1244,8 +1293,8 @@ describe('HasteMap', () => { expect(error.platform).toBe('g'); expect(error.supportsNativePlatform).toBe(false); expect(error.duplicatesSet).toEqual({ - '/project/fruits/Pear.js': 0, - '/project/fruits/another/Pear.js': 0, + '/project/fruits/Pear.js': H.MODULE, + '/project/fruits/another/Pear.js': H.MODULE, }); expect(error.message).toMatchSnapshot(); } diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index fabadb3b33ee..7bfedb3a30a3 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -36,6 +36,7 @@ import type {Console} from 'console'; import type {Mapper} from './types'; import type {Path} from 'types/Config'; import type { + DuplicatesSet, HasteMap as HasteMapObject, InternalHasteMap, ModuleMetaData, @@ -935,22 +936,26 @@ class HasteMap extends EventEmitter { dupsByPlatform = copy(dupsByPlatform); hasteMap.duplicates.set(moduleName, dupsByPlatform); - dups = copy(dups); - dupsByPlatform[platform] = dups; - const dedupType = dups[relativeFilePath]; + dups = (copy(dups): DuplicatesSet); + dupsByPlatform[platform] = dups; delete dups[relativeFilePath]; + const filePaths = Object.keys(dups); - if (filePaths.length > 1) { + // flow types Object.values<> as Array + const fileTypes: Array = (Object.values(dups): any); + + if (filePaths.length > 1 || fileTypes.length > 1) { return; } let dedupMap = hasteMap.map.get(moduleName); + if (dedupMap == null) { dedupMap = Object.create(null); hasteMap.map.set(moduleName, dedupMap); } - dedupMap[platform] = [filePaths[0], dedupType]; + dedupMap[platform] = [filePaths[0], fileTypes[0]]; delete dupsByPlatform[platform]; if (Object.keys(dupsByPlatform).length === 0) { hasteMap.duplicates.delete(moduleName); From 51e2a6eb891e3c110356ee07959a973823c40c96 Mon Sep 17 00:00:00 2001 From: Rafael Oleza Date: Mon, 5 Nov 2018 16:54:22 +0100 Subject: [PATCH 2/3] Start using Maps for duplicates data structure --- packages/jest-haste-map/src/ModuleMap.js | 35 +++++++------- .../src/__tests__/index.test.js | 26 +++++----- packages/jest-haste-map/src/index.js | 47 +++++++++++-------- types/HasteMap.js | 10 +--- 4 files changed, 63 insertions(+), 55 deletions(-) diff --git a/packages/jest-haste-map/src/ModuleMap.js b/packages/jest-haste-map/src/ModuleMap.js index a305d04c720a..1f5351613ed4 100644 --- a/packages/jest-haste-map/src/ModuleMap.js +++ b/packages/jest-haste-map/src/ModuleMap.js @@ -21,7 +21,8 @@ import type { import * as fastPath from './lib/fast_path'; import H from './constants'; -const EMPTY_MAP = {}; +const EMPTY_OBJ = {}; +const EMPTY_MAP = new Map(); export opaque type SerializableModuleMap = { // There is no easier way to extract the type of the entries of a Map @@ -117,14 +118,14 @@ export default class ModuleMap { platform: ?string, supportsNativePlatform: boolean, ): ?ModuleMetaData { - const map = this._raw.map.get(name) || EMPTY_MAP; + const map = this._raw.map.get(name) || EMPTY_OBJ; const dupMap = this._raw.duplicates.get(name) || EMPTY_MAP; if (platform != null) { this._assertNoDuplicates( name, platform, supportsNativePlatform, - dupMap[platform], + dupMap.get(platform), ); if (map[platform] != null) { return map[platform]; @@ -135,7 +136,7 @@ export default class ModuleMap { name, H.NATIVE_PLATFORM, supportsNativePlatform, - dupMap[H.NATIVE_PLATFORM], + dupMap.get(H.NATIVE_PLATFORM), ); if (map[H.NATIVE_PLATFORM]) { return map[H.NATIVE_PLATFORM]; @@ -145,7 +146,7 @@ export default class ModuleMap { name, H.GENERIC_PLATFORM, supportsNativePlatform, - dupMap[H.GENERIC_PLATFORM], + dupMap.get(H.GENERIC_PLATFORM), ); if (map[H.GENERIC_PLATFORM]) { return map[H.GENERIC_PLATFORM]; @@ -163,17 +164,19 @@ export default class ModuleMap { return; } // Force flow refinement - const previousSet: DuplicatesSet = relativePathSet; - const set = Object.keys(previousSet).reduce((set, relativePath) => { + const previousSet = relativePathSet; + const duplicates = new Map(); + + for (const [relativePath, type] of previousSet) { const duplicatePath = fastPath.resolve(this._raw.rootDir, relativePath); - set[duplicatePath] = previousSet[relativePath]; - return set; - }, Object.create(null)); + duplicates.set(duplicatePath, type); + } + throw new DuplicateHasteCandidatesError( name, platform, supportsNativePlatform, - set, + duplicates, ); } @@ -206,12 +209,12 @@ class DuplicateHasteCandidatesError extends Error { `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) + Array.from(duplicatesSet) + .map( + ([dupFilePath, dupFileType]) => + ` * \`${dupFilePath}\` (${getTypeMessage(dupFileType)})\n`, + ) .sort() - .map(dupFilePath => { - const typeMessage = getTypeMessage(duplicatesSet[dupFilePath]); - return ` * \`${dupFilePath}\` (${typeMessage})\n`; - }) .join(''), ); this.hasteName = name; diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index 033209cf0b0b..ad07671d329f 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -783,12 +783,12 @@ describe('HasteMap', () => { ).build(); expect(normalizeMap(data.duplicates)).toEqual( createMap({ - Strawberry: { - g: { + Strawberry: createMap({ + g: createMap({ 'fruits/Strawberry.js': H.MODULE, 'fruits/another/Strawberry.js': H.MODULE, - }, - }, + }), + }), }), ); expect(data.map.get('Strawberry')).toEqual({}); @@ -828,13 +828,13 @@ describe('HasteMap', () => { expect(normalizeMap(data.duplicates)).toEqual( createMap({ - Strawberry: { - g: { + Strawberry: createMap({ + g: createMap({ 'fruits/Strawberry.js': H.MODULE, 'fruits/another/Strawberry.js': H.MODULE, 'fruits/strawberryPackage/package.json': H.PACKAGE, - }, - }, + }), + }), }), ); @@ -1292,10 +1292,12 @@ describe('HasteMap', () => { expect(error.hasteName).toBe('Pear'); expect(error.platform).toBe('g'); expect(error.supportsNativePlatform).toBe(false); - expect(error.duplicatesSet).toEqual({ - '/project/fruits/Pear.js': H.MODULE, - '/project/fruits/another/Pear.js': H.MODULE, - }); + expect(error.duplicatesSet).toEqual( + createMap({ + '/project/fruits/Pear.js': H.MODULE, + '/project/fruits/another/Pear.js': H.MODULE, + }), + ); expect(error.message).toMatchSnapshot(); } } diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index 7bfedb3a30a3..46cf53458f43 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -36,7 +36,6 @@ import type {Console} from 'console'; import type {Mapper} from './types'; import type {Path} from 'types/Config'; import type { - DuplicatesSet, HasteMap as HasteMapObject, InternalHasteMap, ModuleMetaData, @@ -431,20 +430,24 @@ class HasteMap extends EventEmitter { } let dupsByPlatform = hasteMap.duplicates.get(id); if (dupsByPlatform == null) { - dupsByPlatform = Object.create(null); + dupsByPlatform = new Map(); hasteMap.duplicates.set(id, dupsByPlatform); } - const dups = (dupsByPlatform[platform] = Object.create(null)); - dups[module[H.PATH]] = module[H.TYPE]; - dups[existingModule[H.PATH]] = existingModule[H.TYPE]; + + const dups = new Map([ + [module[H.PATH], module[H.TYPE]], + [existingModule[H.PATH], existingModule[H.TYPE]], + ]); + dupsByPlatform.set(platform, dups); + return; } const dupsByPlatform = hasteMap.duplicates.get(id); if (dupsByPlatform != null) { - const dups = dupsByPlatform[platform]; + const dups = dupsByPlatform.get(platform); if (dups != null) { - dups[module[H.PATH]] = module[H.TYPE]; + dups.set(module[H.PATH], module[H.TYPE]); } return; } @@ -929,23 +932,25 @@ class HasteMap extends EventEmitter { const platform = getPlatformExtension(relativeFilePath, this._options.platforms) || H.GENERIC_PLATFORM; - let dups = dupsByPlatform[platform]; + let dups = dupsByPlatform.get(platform); if (dups == null) { return; } - dupsByPlatform = copy(dupsByPlatform); + dupsByPlatform = copyMap(dupsByPlatform); hasteMap.duplicates.set(moduleName, dupsByPlatform); - dups = (copy(dups): DuplicatesSet); - dupsByPlatform[platform] = dups; - delete dups[relativeFilePath]; + dups = copyMap(dups); + dupsByPlatform.set(platform, dups); + dups.delete(relativeFilePath); - const filePaths = Object.keys(dups); - // flow types Object.values<> as Array - const fileTypes: Array = (Object.values(dups): any); + if (dups.size !== 1) { + return; + } - if (filePaths.length > 1 || fileTypes.length > 1) { + const uniqueModule = dups.entries().next().value; + + if (!uniqueModule) { return; } @@ -955,9 +960,9 @@ class HasteMap extends EventEmitter { dedupMap = Object.create(null); hasteMap.map.set(moduleName, dedupMap); } - dedupMap[platform] = [filePaths[0], fileTypes[0]]; - delete dupsByPlatform[platform]; - if (Object.keys(dupsByPlatform).length === 0) { + dedupMap[platform] = uniqueModule; + dupsByPlatform.delete(platform); + if (dupsByPlatform.size === 0) { hasteMap.duplicates.delete(moduleName); } } @@ -1031,6 +1036,10 @@ class HasteMap extends EventEmitter { const copy = object => Object.assign(Object.create(null), object); +function copyMap(input: Map): Map { + return new Map(input); +} + HasteMap.H = H; HasteMap.ModuleMap = HasteModuleMap; diff --git a/types/HasteMap.js b/types/HasteMap.js index 01865ad57790..ed9825535b4f 100644 --- a/types/HasteMap.js +++ b/types/HasteMap.js @@ -24,14 +24,8 @@ export type ModuleMapData = Map; export type WatchmanClocks = Map; export type HasteRegExp = RegExp | ((str: string) => boolean); -export type DuplicatesSet = { - [filePath: string]: /* type */ number, - __proto__: null, -}; -export type DuplicatesIndex = Map< - string, - {[platform: string]: DuplicatesSet, __proto__: null}, ->; +export type DuplicatesSet = Map; +export type DuplicatesIndex = Map>; export type InternalHasteMap = {| clocks: WatchmanClocks, From e2f21d41df37d924c0edc26fd901c025bf55200b Mon Sep 17 00:00:00 2001 From: Rafael Oleza Date: Mon, 5 Nov 2018 16:54:30 +0100 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1f08794bd71..669a81e49efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ - `[jest-runtime]` Ensure error message text is not lost on errors with code frames ([#7319](https://github.com/facebook/jest/pull/7319)) - `[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)) ### Chore & Maintenance