Skip to content

Commit

Permalink
jest-haste-map: deprecate functional ignorePattern and use it in cach…
Browse files Browse the repository at this point in the history
…e key (#4063)

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.
  • Loading branch information
jeanlauliac committed Jul 18, 2017
1 parent 7b9b5b3 commit bf58182
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
9 changes: 5 additions & 4 deletions packages/jest-haste-map/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ let mockClocks;
let mockEmitters;
let object;
let workerFarmMock;
let getCacheFilePath;

describe('HasteMap', () => {
skipOnWindows.suite();
Expand Down Expand Up @@ -152,6 +153,7 @@ describe('HasteMap', () => {
HasteMap = require('../');
H = HasteMap.H;

getCacheFilePath = HasteMap.getCacheFilePath;
HasteMap.getCacheFilePath = jest.fn(() => cacheFilePath);

defaultConfig = {
Expand Down Expand Up @@ -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();

Expand All @@ -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);
});
});
});
Expand Down
7 changes: 7 additions & 0 deletions packages/jest-haste-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -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;
Expand Down

0 comments on commit bf58182

Please sign in to comment.