Fix cache of transformed files with multiple projects (#7186)

jestjs · Oct 17, 2018 · 8a7b4a9 · 8a7b4a9
1 parent 7a64497
commit 8a7b4a9
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,7 @@
 - `[jest-runtime]` Throw an explicit error if `js` is missing from `moduleFileExtensions` ([#7160](https://github.com/facebook/jest/pull/7160))
 - `[jest-runtime]` Fix missing coverage when using negative glob pattern in `testMatch` ([#7170](https://github.com/facebook/jest/pull/7170))
 - `[*]` Ensure `maxWorkers` is at least 1 (was 0 in some cases where there was only 1 CPU) ([#7182](https://github.com/facebook/jest/pull/7182))
+- `[jest-runtime]` Fix transform cache invalidation when requiring a test file from multiple projects ([#7186](https://github.com/facebook/jest/pull/7186))
 
 ### Chore & Maintenance
 

diff --git a/e2e/__tests__/multi_project_runner.test.js b/e2e/__tests__/multi_project_runner.test.js
@@ -350,3 +350,53 @@ test('resolves projects and their <rootDir> properly', () => {
   );
   expect(stderr).toMatch(/banana/);
 });
+
+test('Does transform files with the corresponding project transformer', () => {
+  writeFiles(DIR, {
+    '.watchmanconfig': '',
+    'file.js': SAMPLE_FILE_CONTENT,
+    'package.json': '{}',
+    'project1/__tests__/project1.test.js': `
+      const file = require('../../file.js');
+      test('file', () => expect(file).toBe('PROJECT1'));
+    `,
+    'project1/jest.config.js': `
+      module.exports = {
+        rootDir: './',
+        transform: {'file\.js': './transformer.js'},
+      };`,
+    'project1/transformer.js': `
+      module.exports = {
+        process: () => 'module.exports = "PROJECT1";',
+        getCacheKey: () => 'PROJECT1_CACHE_KEY',
+      }
+    `,
+    'project2/__tests__/project2.test.js': `
+      const file = require('../../file.js');
+      test('file', () => expect(file).toBe('PROJECT2'));
+    `,
+    'project2/jest.config.js': `
+      module.exports = {
+        rootDir: './',
+        transform: {'file\.js': './transformer.js'},
+      };`,
+    'project2/transformer.js': `
+      module.exports = {
+        process: () => 'module.exports = "PROJECT2";',
+        getCacheKey: () => 'PROJECT2_CACHE_KEY',
+      }
+    `,
+  });
+
+  const {stderr} = runJest(DIR, [
+    '--no-watchman',
+    '-i',
+    '--projects',
+    'project1',
+    'project2',
+  ]);
+
+  expect(stderr).toMatch('Ran all test suites in 2 projects.');
+  expect(stderr).toMatch('PASS project1/__tests__/project1.test.js');
+  expect(stderr).toMatch('PASS project2/__tests__/project2.test.js');
+});
diff --git a/packages/jest-runtime/src/__tests__/script_transformer.test.js b/packages/jest-runtime/src/__tests__/script_transformer.test.js
@@ -509,4 +509,25 @@ describe('ScriptTransformer', () => {
     expect(fs.readFileSync).not.toBeCalledWith(cachePath, 'utf8');
     expect(writeFileAtomic.sync).toBeCalled();
   });
+
+  it('does not reuse the in-memory cache between different projects', () => {
+    const scriptTransformer = new ScriptTransformer(
+      Object.assign({}, config, {
+        transform: [['^.+\\.js$', 'test_preprocessor']],
+      }),
+    );
+
+    scriptTransformer.transform('/fruits/banana.js', {});
+
+    const anotherScriptTransformer = new ScriptTransformer(
+      Object.assign({}, config, {
+        transform: [['^.+\\.js$', 'css-preprocessor']],
+      }),
+    );
+
+    anotherScriptTransformer.transform('/fruits/banana.js', {});
+
+    expect(fs.readFileSync.mock.calls.length).toBe(2);
+    expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8');
+  });
 });
diff --git a/packages/jest-runtime/src/script_transformer.js b/packages/jest-runtime/src/script_transformer.js
@@ -40,31 +40,47 @@ export type Options = {|
   isInternalModule?: boolean,
 |};
 
-const cache: Map<string, TransformResult> = new Map();
-const configToJsonMap = new Map();
-// Cache regular expressions to test whether the file needs to be preprocessed
-const ignoreCache: WeakMap<ProjectConfig, ?RegExp> = new WeakMap();
+type ProjectCache = {|
+  configString: string,
+  ignorePatternsRegExp: ?RegExp,
+  transformedFiles: Map<string, TransformResult>,
+|};
+
+// This data structure is used to avoid recalculating some data every time that
+// we need to transform a file. Since ScriptTransformer is instantiated for each
+// file we need to keep this object in the local scope of this module.
+const projectCaches: WeakMap<ProjectConfig, ProjectCache> = new WeakMap();
 
 // To reset the cache for specific changesets (rather than package version).
 const CACHE_VERSION = '1';
 
 export default class ScriptTransformer {
   static EVAL_RESULT_VARIABLE: string;
+  _cache: ProjectCache;
   _config: ProjectConfig;
   _transformCache: Map<Path, ?Transformer>;
 
   constructor(config: ProjectConfig) {
     this._config = config;
     this._transformCache = new Map();
+
+    let projectCache = projectCaches.get(config);
+
+    if (!projectCache) {
+      projectCache = {
+        configString: stableStringify(this._config),
+        ignorePatternsRegExp: calcIgnorePatternRegexp(this._config),
+        transformedFiles: new Map(),
+      };
+
+      projectCaches.set(config, projectCache);
+    }
+
+    this._cache = projectCache;
   }
 
   _getCacheKey(fileData: string, filename: Path, instrument: boolean): string {
-    if (!configToJsonMap.has(this._config)) {
-      // We only need this set of config options that can likely influence
-      // cached output instead of all config options.
-      configToJsonMap.set(this._config, stableStringify(this._config));
-    }
-    const configString = configToJsonMap.get(this._config) || '';
+    const configString = this._cache.configString;
     const transformer = this._getTransformer(filename);
 
     if (transformer && typeof transformer.getCacheKey === 'function') {
@@ -188,8 +204,7 @@ export default class ScriptTransformer {
     // Ignore cache if `config.cache` is set (--no-cache)
     let code = this._config.cache ? readCodeCacheFile(cacheFilePath) : null;
 
-    const shouldCallTransform =
-      transform && shouldTransform(filename, this._config);
+    const shouldCallTransform = transform && this._shouldTransform(filename);
 
     // That means that the transform has a custom instrumentation
     // logic and will handle it based on `config.collectCoverage` option
@@ -287,7 +302,7 @@ export default class ScriptTransformer {
     const willTransform =
       !isInternalModule &&
       !isCoreModule &&
-      (shouldTransform(filename, this._config) || instrument);
+      (this._shouldTransform(filename) || instrument);
 
     try {
       if (willTransform) {
@@ -341,7 +356,7 @@ export default class ScriptTransformer {
     if (!options.isCoreModule) {
       instrument = shouldInstrument(filename, options, this._config);
       scriptCacheKey = getScriptCacheKey(filename, instrument);
-      result = cache.get(scriptCacheKey);
+      result = this._cache.transformedFiles.get(scriptCacheKey);
     }
 
     if (result) {
@@ -356,11 +371,20 @@ export default class ScriptTransformer {
     );
 
     if (scriptCacheKey) {
-      cache.set(scriptCacheKey, result);
+      this._cache.transformedFiles.set(scriptCacheKey, result);
     }
 
     return result;
   }
+
+  _shouldTransform(filename: Path): boolean {
+    const ignoreRegexp = this._cache.ignorePatternsRegExp;
+    const isIgnored = ignoreRegexp ? ignoreRegexp.test(filename) : false;
+
+    return (
+      !!this._config.transform && !!this._config.transform.length && !isIgnored
+    );
+  }
 }
 
 const removeFile = (path: Path) => {
@@ -482,24 +506,15 @@ const getScriptCacheKey = (filename, instrument: boolean) => {
   return filename + '_' + mtime.getTime() + (instrument ? '_instrumented' : '');
 };
 
-const shouldTransform = (filename: Path, config: ProjectConfig): boolean => {
-  if (!ignoreCache.has(config)) {
-    if (
-      !config.transformIgnorePatterns ||
-      config.transformIgnorePatterns.length === 0
-    ) {
-      ignoreCache.set(config, null);
-    } else {
-      ignoreCache.set(
-        config,
-        new RegExp(config.transformIgnorePatterns.join('|')),
-      );
-    }
+const calcIgnorePatternRegexp = (config: ProjectConfig): ?RegExp => {
+  if (
+    !config.transformIgnorePatterns ||
+    config.transformIgnorePatterns.length === 0
+  ) {
+    return null;
   }
 
-  const ignoreRegexp = ignoreCache.get(config);
-  const isIgnored = ignoreRegexp ? ignoreRegexp.test(filename) : false;
-  return !!config.transform && !!config.transform.length && !isIgnored;
+  return new RegExp(config.transformIgnorePatterns.join('|'));
 };
 
 const wrap = content =>