packager: TransformCaching: make choice of cache explicit in the API

Summary:
This changeset moves the creation of the transform cache at the top-level of the bundler so that:

* we can use alternative folders, such as the project path itself, that I think will be more robust especially for OSS;
* we can disable the cache completely, that is useful in some cases (for example, the script that fills the global cache).

The reasons I believe a local project path is more robust are:

* there are less likely conflicts between different users and different projects on a single machine;
* the cache is de facto cleaned up if you clone a fresh copy of a project, something I think is desirable;
* some people have been reporting that `tmpDir` just returns nothing;
* finally, it prevents another user from writing malicious transformed code in the cache into the shared temp dir—only people with write access to the project have write access to the cache, that is consistent.

Reviewed By: davidaurelio

Differential Revision: D5113121

fbshipit-source-id: 74392733a0be306a7119516d7905fc43cd8c778e

local-cli/bundle/buildBundle.js

@@ -14,6 +14,7 @@
 const log = require('../util/log').out('bundle');
 const Server = require('../../packager/src/Server');
 const TerminalReporter = require('../../packager/src/lib/TerminalReporter');
+const TransformCaching = require('../../packager/src/lib/TransformCaching');
 
 const outputBundle = require('../../packager/src/shared/output/bundle');
 const path = require('path');
@@ -91,6 +92,7 @@ function buildBundle(
       resetCache: args.resetCache,
       reporter: new TerminalReporter(),
       sourceExts: defaultSourceExts.concat(sourceExts),
+      transformCache: TransformCaching.useTempDir(),
       transformModulePath: transformModulePath,
       watch: false,
     };

local-cli/util/Config.js

@@ -133,12 +133,17 @@ const defaultConfig: ConfigT = {
  */
 const Config = {
   find(startDir: string): ConfigT {
+    return this.findWithPath(startDir).config;
+  },
+
+  findWithPath(startDir: string): {config: ConfigT, projectPath: string} {
     const configPath = findConfigPath(startDir);
     invariant(
       configPath,
       `Can't find "${RN_CLI_CONFIG}" file in any parent folder of "${startDir}"`,
     );
-    return this.loadFile(configPath, startDir);
+    const projectPath = path.dirname(configPath);
+    return {config: this.loadFile(configPath, startDir), projectPath};
   },
 
   findOptional(startDir: string): ConfigT {

packager/index.js

@@ -19,6 +19,7 @@ const invariant = require('fbjs/lib/invariant');
 import type {PostProcessModules, PostMinifyProcess} from './src/Bundler';
 import type Server from './src/Server';
 import type {GlobalTransformCache} from './src/lib/GlobalTransformCache';
+import type {TransformCache} from './src/lib/TransformCaching';
 import type {Reporter} from './src/lib/reporting';
 import type {HasteImpl} from './src/node-haste/Module';
 
@@ -34,6 +35,7 @@ type Options = {
   projectRoots: $ReadOnlyArray<string>,
   reporter?: Reporter,
   +sourceExts: ?Array<string>,
+  +transformCache: TransformCache,
   +transformModulePath: string,
   watch?: boolean,
 };
@@ -118,6 +120,7 @@ function createServer(options: StrictOptions): Server {
 
   // Some callsites may not be Flowified yet.
   invariant(options.reporter != null, 'createServer() requires reporter');
+  invariant(options.transformCache != null, 'createServer() requires transformCache');
   const serverOptions = Object.assign({}, options);
   delete serverOptions.verbose;
   const ServerClass = require('./src/Server');

packager/src/Bundler/index.js

@@ -46,6 +46,7 @@ import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionRes
 import type {MappingsMap} from '../lib/SourceMap';
 import type {Options as JSTransformerOptions} from '../JSTransformer/worker/worker';
 import type {Reporter} from '../lib/reporting';
+import type {TransformCache} from '../lib/TransformCaching';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 
 export type BundlingOptions = {|
@@ -132,6 +133,7 @@ type Options = {|
   +reporter: Reporter,
   +resetCache: boolean,
   +sourceExts: Array<string>,
+  +transformCache: TransformCache,
   +transformModulePath: string,
   +transformTimeoutInterval: ?number,
   +watch: boolean,
@@ -227,6 +229,7 @@ class Bundler {
           code,
           transformCodeOptions,
         ),
+      transformCache: opts.transformCache,
       watch: opts.watch,
     });
 

packager/src/Resolver/index.js

@@ -22,7 +22,7 @@ import type {MappingsMap} from '../lib/SourceMap';
 import type {PostMinifyProcess} from '../Bundler';
 import type {Options as JSTransformerOptions} from '../JSTransformer/worker/worker';
 import type {Reporter} from '../lib/reporting';
-import type {GetTransformCacheKey} from '../lib/TransformCache';
+import type {TransformCache, GetTransformCacheKey} from '../lib/TransformCaching';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 
 type MinifyCode = (filePath: string, code: string, map: MappingsMap) =>
@@ -47,6 +47,7 @@ type Options = {|
   +reporter: Reporter,
   +resetCache: boolean,
   +sourceExts: Array<string>,
+  +transformCache: TransformCache,
   +transformCode: TransformCode,
   +watch: boolean,
 |};
@@ -76,6 +77,7 @@ class Resolver {
       moduleOptions: {
         hasteImpl: opts.hasteImpl,
         resetCache: opts.resetCache,
+        transformCache: opts.transformCache,
       },
       preferNativePlatform: true,
       roots: opts.projectRoots,

packager/src/Server/index.js

@@ -33,6 +33,7 @@ import type Bundle from '../Bundler/Bundle';
 import type HMRBundle from '../Bundler/HMRBundle';
 import type {Reporter} from '../lib/reporting';
 import type {GetTransformOptions, PostProcessModules, PostMinifyProcess} from '../Bundler';
+import type {TransformCache} from '../lib/TransformCaching';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 import type {SourceMap, Symbolicate} from './symbolicate';
 
@@ -75,6 +76,7 @@ type Options = {
   resetCache?: boolean,
   silent?: boolean,
   +sourceExts: ?Array<string>,
+  +transformCache: TransformCache,
   +transformModulePath: string,
   transformTimeoutInterval?: number,
   watch?: boolean,
@@ -131,6 +133,7 @@ class Server {
     resetCache: boolean,
     silent: boolean,
     +sourceExts: Array<string>,
+    +transformCache: TransformCache,
     +transformModulePath: string,
     transformTimeoutInterval: ?number,
     watch: boolean,
@@ -170,6 +173,7 @@ class Server {
       resetCache: options.resetCache || false,
       silent: options.silent || false,
       sourceExts: options.sourceExts || defaults.sourceExts,
+      transformCache: options.transformCache,
       transformModulePath: options.transformModulePath,
       transformTimeoutInterval: options.transformTimeoutInterval,
       watch: options.watch || false,

packager/src/lib/GlobalTransformCache.js

@@ -27,7 +27,7 @@ import type {
   TransformOptionsStrict,
 } from '../JSTransformer/worker/worker';
 import type {LocalPath} from '../node-haste/lib/toLocalPath';
-import type {CachedResult, GetTransformCacheKey} from './TransformCache';
+import type {CachedResult, GetTransformCacheKey} from './TransformCaching';
 
 /**
  * The API that a global transform cache must comply with. To implement a

packager/src/lib/TransformCaching.js

@@ -59,6 +59,23 @@ export type ReadTransformProps = {
   cacheOptions: CacheOptions,
 };
 
+type WriteTransformProps = {
+  filePath: string,
+  sourceCode: string,
+  getTransformCacheKey: GetTransformCacheKey,
+  transformOptions: WorkerOptions,
+  transformOptionsKey: string,
+  result: CachedResult,
+};
+
+/**
+ * The API that should be exposed for a transform cache.
+ */
+export type TransformCache = {
+  writeSync(props: WriteTransformProps): void,
+  readSync(props: ReadTransformProps): TransformCacheResult,
+};
+
 const EMPTY_ARRAY = [];
 
 /* 1 day */
@@ -66,7 +83,7 @@ const GARBAGE_COLLECTION_PERIOD = 24 * 60 * 60 * 1000;
 /* 4 days */
 const CACHE_FILE_MAX_LAST_ACCESS_TIME = GARBAGE_COLLECTION_PERIOD * 4;
 
-class TransformCache {
+class FileBasedCache {
   _cacheWasReset: boolean;
   _lastCollected: ?number;
   _rootPath: string;
@@ -398,4 +415,40 @@ function unlinkIfExistsSync(filePath: string) {
   }
 }
 
-module.exports = TransformCache;
+/**
+ * In some context we want to build from scratch, that is what this cache
+ * implementation allows.
+ */
+function none(): TransformCache {
+  return {
+    writeSync: () => {},
+    readSync: () => ({
+      result: null,
+      outdatedDependencies: [],
+    }),
+  };
+}
+
+/**
+ * If packager is running for two different directories, we don't want the
+ * caches to conflict with each other. `__dirname` carries that because
+ * packager will be, for example, installed in a different `node_modules/`
+ * folder for different projects.
+ */
+function useTempDir(): TransformCache {
+  const hash = crypto.createHash('sha1').update(__dirname);
+  if (process.getuid != null) {
+    hash.update(process.getuid().toString());
+  }
+  const tmpDir = require('os').tmpdir();
+  const cacheName = 'react-native-packager-cache';
+  const rootPath = path.join(tmpDir, cacheName + '-' + hash.digest('hex'));
+  return new FileBasedCache(rootPath);
+}
+
+function useProjectDir(projectPath: string): TransformCache {
+  invariant(path.isAbsolute(projectPath), 'project path must be absolute');
+  return new FileBasedCache(path.join(projectPath, '.metro-bundler'));
+}
+
+module.exports = {FileBasedCache, none, useTempDir, useProjectDir};

packager/src/lib/__mocks__/TransformCaching.js

@@ -44,4 +44,4 @@ class TransformCacheMock {
 
 }
 
-module.exports = TransformCacheMock;
+module.exports = {mocked: () => new TransformCacheMock()};

packager/src/lib/__tests__/TransformCaching-test.js

@@ -11,7 +11,7 @@
 
 jest
   .dontMock('json-stable-stringify')
-  .dontMock('../TransformCache')
+  .dontMock('../TransformCaching')
   .dontMock('left-pad')
   .dontMock('lodash/throttle')
   .dontMock('crypto');
@@ -48,14 +48,14 @@ function cartesianProductOf(a1, a2) {
   return product;
 }
 
-describe('TransformCache', () => {
+describe('TransformCaching.FileBasedCache', () => {
 
   let transformCache;
 
   beforeEach(() => {
     jest.resetModules();
     mockFS.clear();
-    transformCache = new (require('../TransformCache'))('/cache');
+    transformCache = new (require('../TransformCaching').FileBasedCache)('/cache');
   });
 
   it('is caching different files and options separately', () => {

packager/src/node-haste/DependencyGraph.js

@@ -39,7 +39,7 @@ import type {
   Options as JSTransformerOptions,
 } from '../JSTransformer/worker/worker';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
-import type {GetTransformCacheKey} from '../lib/TransformCache';
+import type {GetTransformCacheKey} from '../lib/TransformCaching';
 import type {Reporter} from '../lib/reporting';
 import type {ModuleMap} from './DependencyGraph/ResolutionRequest';
 import type {Options as ModuleOptions, TransformCode} from './Module';

packager/src/node-haste/Module.js

@@ -12,8 +12,6 @@
 
 'use strict';
 
-const TransformCache = require('../lib/TransformCache');
-
 const crypto = require('crypto');
 const docblock = require('./DependencyGraph/docblock');
 const fs = require('fs');
@@ -30,8 +28,11 @@ import type {
 } from '../JSTransformer/worker/worker';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 import type {MappingsMap} from '../lib/SourceMap';
-import type {GetTransformCacheKey} from '../lib/TransformCache';
-import type {ReadTransformProps} from '../lib/TransformCache';
+import type {
+  TransformCache,
+  GetTransformCacheKey,
+  ReadTransformProps,
+} from '../lib/TransformCaching';
 import type {Reporter} from '../lib/reporting';
 import type DependencyGraphHelpers
   from './DependencyGraph/DependencyGraphHelpers';
@@ -70,17 +71,18 @@ export type HasteImpl = {
 export type Options = {
   hasteImpl?: HasteImpl,
   resetCache?: boolean,
+  transformCache: TransformCache,
 };
 
 export type ConstructorArgs = {
   depGraphHelpers: DependencyGraphHelpers,
-  globalTransformCache: ?GlobalTransformCache,
   file: string,
+  getTransformCacheKey: GetTransformCacheKey,
+  globalTransformCache: ?GlobalTransformCache,
   localPath: LocalPath,
   moduleCache: ModuleCache,
   options: Options,
   reporter: Reporter,
-  getTransformCacheKey: GetTransformCacheKey,
   transformCode: ?TransformCode,
 };
 
@@ -334,7 +336,7 @@ class Module {
         return;
       }
       invariant(result != null, 'missing result');
-      Module._getTransformCache().writeSync({...cacheProps, result});
+      this._options.transformCache.writeSync({...cacheProps, result});
       callback(undefined, result);
     });
   }
@@ -381,7 +383,7 @@ class Module {
       transformOptions,
       transformOptionsKey,
     );
-    const cachedResult = Module._getTransformCache().readSync(cacheProps);
+    const cachedResult = this._options.transformCache.readSync(cacheProps);
     if (cachedResult.result == null) {
       return {
         result: null,
@@ -469,27 +471,6 @@ class Module {
   isPolyfill() {
     return false;
   }
-
-  /**
-   * If packager is running for two different directories, we don't want the
-   * caches to conflict with each other. `__dirname` carries that because
-   * packager will be, for example, installed in a different `node_modules/`
-   * folder for different projects.
-   */
-  static _getTransformCache(): TransformCache {
-    if (this._transformCache != null) {
-      return this._transformCache;
-    }
-    const hash = crypto.createHash('sha1').update(__dirname);
-    if (process.getuid != null) {
-      hash.update(process.getuid().toString());
-    }
-    const tmpDir = require('os').tmpdir();
-    const cacheName = 'react-native-packager-cache';
-    const rootPath = path.join(tmpDir, cacheName + '-' + hash.digest('hex'));
-    this._transformCache = new TransformCache(rootPath);
-    return this._transformCache;
-  }
 }
 
 // use weak map to speed up hash creation of known objects

packager/src/node-haste/ModuleCache.js

@@ -20,7 +20,7 @@ const Polyfill = require('./Polyfill');
 const toLocalPath = require('./lib/toLocalPath');
 
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
-import type {GetTransformCacheKey} from '../lib/TransformCache';
+import type {GetTransformCacheKey} from '../lib/TransformCaching';
 import type {Reporter} from '../lib/reporting';
 import type DependencyGraphHelpers
   from './DependencyGraph/DependencyGraphHelpers';
@@ -112,8 +112,8 @@ class ModuleCache {
        */
       this._moduleCache[filePath] = new AssetModule(
         {
-          dependencies: this._assetDependencies,
           depGraphHelpers: this._depGraphHelpers,
+          dependencies: this._assetDependencies,
           file: filePath,
           getTransformCacheKey: this._getTransformCacheKey,
           globalTransformCache: null,
@@ -161,11 +161,12 @@ class ModuleCache {
   createPolyfill({file}: {file: string}) {
     /* $FlowFixMe: there are missing arguments. */
     return new Polyfill({
-      file,
       depGraphHelpers: this._depGraphHelpers,
+      file,
       getTransformCacheKey: this._getTransformCacheKey,
       localPath: toLocalPath(this._roots, file),
       moduleCache: this,
+      options: this._moduleOptions,
       transformCode: this._transformCode,
     });
   }

packager/src/node-haste/__tests__/DependencyGraph-test.js

@@ -15,7 +15,7 @@ jest.useRealTimers();
 jest
   .mock('fs')
   .mock('../../Logger')
-  .mock('../../lib/TransformCache')
+  .mock('../../lib/TransformCaching')
   // It's noticeably faster to prevent running watchman from FileWatcher.
   .mock('child_process', () => ({}));
 
@@ -96,7 +96,7 @@ describe('DependencyGraph', function() {
       useWatchman: false,
       ignoreFilePath: () => false,
       maxWorkerCount: 1,
-      moduleOptions: {},
+      moduleOptions: {transformCache: require('TransformCaching').mocked()},
       resetCache: true,
       transformCode: (module, sourceCode, transformOptions) => {
         return new Promise(resolve => {

packager/src/node-haste/__tests__/Module-test.js

@@ -21,7 +21,7 @@ jest
 const Module = require('../Module');
 const ModuleCache = require('../ModuleCache');
 const DependencyGraphHelpers = require('../DependencyGraph/DependencyGraphHelpers');
-const TransformCache = require('../../lib/TransformCache');
+const TransformCaching = require('../../lib/TransformCaching');
 const fs = require('graceful-fs');
 
 const packageJson =
@@ -31,8 +31,6 @@ const packageJson =
     description: "A require('foo') story",
   });
 
-const TRANSFORM_CACHE = new TransformCache();
-
 function mockFS(rootChildren) {
   fs.__setMockFilesystem({root: rootChildren});
 }
@@ -49,6 +47,7 @@ describe('Module', () => {
   const fileName = '/root/index.js';
 
   let cache;
+  const transformCache = TransformCaching.mocked();
 
   const createCache = () => ({
     get: jest.genMockFn().mockImplementation(
@@ -61,7 +60,7 @@ describe('Module', () => {
   let transformCacheKey;
   const createModule = options =>
     new Module({
-      options: {},
+      options: {transformCache},
       transformCode: (module, sourceCode, transformOptions) => {
         return Promise.resolve({code: sourceCode});
       },
@@ -80,7 +79,7 @@ describe('Module', () => {
     process.platform = 'linux';
     cache = createCache();
     transformCacheKey = 'abcdef';
-    TRANSFORM_CACHE.mock.reset();
+    transformCache.mock.reset();
   });
 
   describe('Module ID', () => {
@@ -183,7 +182,7 @@ describe('Module', () => {
       transformResult = {code: ''};
       transformCode = jest.genMockFn()
         .mockImplementation((module, sourceCode, options) => {
-          TRANSFORM_CACHE.writeSync({
+          transformCache.writeSync({
             filePath: module.path,
             sourceCode,
             transformOptions: options,
@@ -257,23 +256,6 @@ describe('Module', () => {
       });
     });
 
-    it('stores all things if options is undefined', () => {
-      transformResult = {
-        code: exampleCode,
-        arbitrary: 'arbitrary',
-        dependencies: ['foo', 'bar'],
-        dependencyOffsets: [12, 764],
-        id: null,
-        map: {version: 3},
-        subObject: {foo: 'bar'},
-      };
-      const module = createModule({transformCode, options: undefined});
-
-      return module.read().then(result => {
-        expect(result).toEqual({...transformResult, source: 'arbitrary(code);'});
-      });
-    });
-
     it('exposes the transformed code rather than the raw file contents', () => {
       transformResult = {code: exampleCode};
       const module = createModule({transformCode});