From c03a76b01a502b1513dd098a22b6fde4f636a55a Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 25 May 2017 04:23:19 -0700 Subject: [PATCH] packager: TransformCaching: make choice of cache explicit in the API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/metro-bundler/index.js | 3 + packages/metro-bundler/src/Bundler/index.js | 3 + packages/metro-bundler/src/Resolver/index.js | 4 +- packages/metro-bundler/src/Server/index.js | 4 ++ .../src/lib/GlobalTransformCache.js | 2 +- ...{TransformCache.js => TransformCaching.js} | 57 ++++++++++++++++++- ...{TransformCache.js => TransformCaching.js} | 2 +- ...Cache-test.js => TransformCaching-test.js} | 6 +- .../src/node-haste/DependencyGraph.js | 2 +- .../metro-bundler/src/node-haste/Module.js | 39 ++++--------- .../src/node-haste/ModuleCache.js | 7 ++- .../__tests__/DependencyGraph-test.js | 4 +- .../src/node-haste/__tests__/Module-test.js | 28 ++------- 13 files changed, 95 insertions(+), 66 deletions(-) rename packages/metro-bundler/src/lib/{TransformCache.js => TransformCaching.js} (87%) rename packages/metro-bundler/src/lib/__mocks__/{TransformCache.js => TransformCaching.js} (95%) rename packages/metro-bundler/src/lib/__tests__/{TransformCache-test.js => TransformCaching-test.js} (95%) diff --git a/packages/metro-bundler/index.js b/packages/metro-bundler/index.js index 7d7ab760..cfae75fd 100644 --- a/packages/metro-bundler/index.js +++ b/packages/metro-bundler/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, reporter?: Reporter, +sourceExts: ?Array, + +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'); diff --git a/packages/metro-bundler/src/Bundler/index.js b/packages/metro-bundler/src/Bundler/index.js index 945d1c4e..4068b027 100644 --- a/packages/metro-bundler/src/Bundler/index.js +++ b/packages/metro-bundler/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, + +transformCache: TransformCache, +transformModulePath: string, +transformTimeoutInterval: ?number, +watch: boolean, @@ -227,6 +229,7 @@ class Bundler { code, transformCodeOptions, ), + transformCache: opts.transformCache, watch: opts.watch, }); diff --git a/packages/metro-bundler/src/Resolver/index.js b/packages/metro-bundler/src/Resolver/index.js index 1713a42e..b855d336 100644 --- a/packages/metro-bundler/src/Resolver/index.js +++ b/packages/metro-bundler/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, + +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, diff --git a/packages/metro-bundler/src/Server/index.js b/packages/metro-bundler/src/Server/index.js index db2d10b8..7f20607d 100644 --- a/packages/metro-bundler/src/Server/index.js +++ b/packages/metro-bundler/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, + +transformCache: TransformCache, +transformModulePath: string, transformTimeoutInterval?: number, watch?: boolean, @@ -131,6 +133,7 @@ class Server { resetCache: boolean, silent: boolean, +sourceExts: Array, + +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, diff --git a/packages/metro-bundler/src/lib/GlobalTransformCache.js b/packages/metro-bundler/src/lib/GlobalTransformCache.js index 11ff4591..4f8f6834 100644 --- a/packages/metro-bundler/src/lib/GlobalTransformCache.js +++ b/packages/metro-bundler/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 diff --git a/packages/metro-bundler/src/lib/TransformCache.js b/packages/metro-bundler/src/lib/TransformCaching.js similarity index 87% rename from packages/metro-bundler/src/lib/TransformCache.js rename to packages/metro-bundler/src/lib/TransformCaching.js index 45b02d69..57eaaba5 100644 --- a/packages/metro-bundler/src/lib/TransformCache.js +++ b/packages/metro-bundler/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}; diff --git a/packages/metro-bundler/src/lib/__mocks__/TransformCache.js b/packages/metro-bundler/src/lib/__mocks__/TransformCaching.js similarity index 95% rename from packages/metro-bundler/src/lib/__mocks__/TransformCache.js rename to packages/metro-bundler/src/lib/__mocks__/TransformCaching.js index 288e330a..0fd89826 100644 --- a/packages/metro-bundler/src/lib/__mocks__/TransformCache.js +++ b/packages/metro-bundler/src/lib/__mocks__/TransformCaching.js @@ -44,4 +44,4 @@ class TransformCacheMock { } -module.exports = TransformCacheMock; +module.exports = {mocked: () => new TransformCacheMock()}; diff --git a/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js b/packages/metro-bundler/src/lib/__tests__/TransformCaching-test.js similarity index 95% rename from packages/metro-bundler/src/lib/__tests__/TransformCache-test.js rename to packages/metro-bundler/src/lib/__tests__/TransformCaching-test.js index 06c81592..d1604e85 100644 --- a/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js +++ b/packages/metro-bundler/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', () => { diff --git a/packages/metro-bundler/src/node-haste/DependencyGraph.js b/packages/metro-bundler/src/node-haste/DependencyGraph.js index 713649a0..3205187d 100644 --- a/packages/metro-bundler/src/node-haste/DependencyGraph.js +++ b/packages/metro-bundler/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'; diff --git a/packages/metro-bundler/src/node-haste/Module.js b/packages/metro-bundler/src/node-haste/Module.js index 26dbdb44..17269be8 100644 --- a/packages/metro-bundler/src/node-haste/Module.js +++ b/packages/metro-bundler/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 diff --git a/packages/metro-bundler/src/node-haste/ModuleCache.js b/packages/metro-bundler/src/node-haste/ModuleCache.js index bdc7c1b3..7d94509e 100644 --- a/packages/metro-bundler/src/node-haste/ModuleCache.js +++ b/packages/metro-bundler/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, }); } diff --git a/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js b/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js index d8a6d139..968f99a7 100644 --- a/packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js +++ b/packages/metro-bundler/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 => { diff --git a/packages/metro-bundler/src/node-haste/__tests__/Module-test.js b/packages/metro-bundler/src/node-haste/__tests__/Module-test.js index 111e86d3..1c92071d 100644 --- a/packages/metro-bundler/src/node-haste/__tests__/Module-test.js +++ b/packages/metro-bundler/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});