From bf58182ec328f6f7db1b9d60927a34622f21ff2f Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Tue, 18 Jul 2017 17:53:50 +0100 Subject: [PATCH] jest-haste-map: deprecate functional ignorePattern and use it in cache key (#4063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I plan to revert `ignorePattern` to only being a strict `RegExp`, after removing the function use case from React Native and Metro bundler. The reason for getting rid of the function use case is that `ignorePattern` should be part of the cache key, and a function is not analysable at runtime. If it can only be a `RegExp`, then we can use `ignorePattern.toString()` as part of the cache key. The reason it needs to be part of the cache key is because the cache needs to be discarded—or at least reevaluated—when the ignore pattern changes. Otherwise, we can be missing some new modules that should be included, or the reverse, we can be including modules that should now be ignored. I have observed considerable trouble caused by this issue. For example, in React Native, people would reload a project and it wouldn't find a module, or, duplicates modules would be detected while in fact one of them should have been ignored. This changeset add a deprecation notice for the functional use case (so that we can release this as a minor/revision), and starts using the string representation of the ignore pattern in the cache key (so that we can get a correct behavior as soon as possible for callsites that do already use a `RegExp`). See also #2957, that introduced `ignorePattern` as a function. Alternatively, we could require callsites to provide their own cache key if they do want to use a function, but this makes it more complicated and I'm not sure there's really any other callsites than React Native that does that, that we will fix ourselves as well. --- packages/jest-haste-map/src/__tests__/index.test.js | 9 +++++---- packages/jest-haste-map/src/index.js | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index 7c61b632bd1c..32f416720fa1 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -100,6 +100,7 @@ let mockClocks; let mockEmitters; let object; let workerFarmMock; +let getCacheFilePath; describe('HasteMap', () => { skipOnWindows.suite(); @@ -152,6 +153,7 @@ describe('HasteMap', () => { HasteMap = require('../'); H = HasteMap.H; + getCacheFilePath = HasteMap.getCacheFilePath; HasteMap.getCacheFilePath = jest.fn(() => cacheFilePath); defaultConfig = { @@ -547,7 +549,8 @@ describe('HasteMap', () => { }); }); - it('discards the cache when configuration changes (broken)', () => { + it('discards the cache when configuration changes', () => { + HasteMap.getCacheFilePath = getCacheFilePath; return new HasteMap(defaultConfig).build().then(() => { fs.readFileSync.mockClear(); @@ -564,9 +567,7 @@ describe('HasteMap', () => { ignorePattern: /kiwi|pear/, }); return new HasteMap(config).build().then(({moduleMap}) => { - // `getModule` should actually return `null` here, because Pear - // should get ignored by the pattern. - expect(typeof moduleMap.getModule('Pear')).toBe('string'); + expect(moduleMap.getModule('Pear')).toBe(null); }); }); }); diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index 4dc3ae4d078a..b38df77b2ce7 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -224,6 +224,12 @@ class HasteMap extends EventEmitter { watch: !!options.watch, }; this._console = options.console || global.console; + if (!(options.ignorePattern instanceof RegExp)) { + this._console.warn( + 'jest-haste-map: the `ignorePattern` options as a function is being ' + + 'deprecated. Provide a RegExp instead. See https://github.com/facebook/jest/pull/4063.', + ); + } this._cachePath = HasteMap.getCacheFilePath( this._options.cacheDirectory, `haste-map-${this._options.name}`, @@ -232,6 +238,7 @@ class HasteMap extends EventEmitter { this._options.extensions.join(':'), this._options.platforms.join(':'), options.mocksPattern || '', + options.ignorePattern.toString(), ); this._whitelist = getWhiteList(options.providesModuleNodeModules); this._buildPromise = null;