From 502f24a26605a1067124868c70261ee82d6b49e8 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Wed, 24 May 2017 07:42:28 -0700 Subject: [PATCH] packager: TransformCache: extract choice of root path outside of class Summary: This is a first step towards doing an experiment I was talking about, that is, using the local directory (the directory where the config file lives) instead of the tmp dir, that is fragile. Reviewed By: davidaurelio Differential Revision: D5112326 fbshipit-source-id: 819636209972115e41213867f3eb78fbdf6ed9df --- .../src/Bundler/__tests__/Bundler-test.js | 4 + .../metro-bundler/src/lib/TransformCache.js | 101 +++++++----------- .../src/lib/__tests__/TransformCache-test.js | 2 +- .../metro-bundler/src/node-haste/Module.js | 30 +++++- 4 files changed, 71 insertions(+), 66 deletions(-) diff --git a/packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js b/packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js index bfcfa787..f0dad065 100644 --- a/packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js +++ b/packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js @@ -34,6 +34,7 @@ var defaults = require('../../../defaults'); var sizeOf = require('image-size'); var fs = require('fs'); const os = require('os'); +const path = require('path'); const {any, objectContaining} = expect; @@ -85,6 +86,9 @@ describe('Bundler', function() { beforeEach(function() { os.cpus.mockReturnValue({length: 1}); + // local directory on purpose, because it should not actually write + // anything to the disk during a unit test! + os.tmpDir.mockReturnValue(path.join(__dirname)); getDependencies = jest.fn(); getModuleSystemDependencies = jest.fn(); diff --git a/packages/metro-bundler/src/lib/TransformCache.js b/packages/metro-bundler/src/lib/TransformCache.js index d32189b8..45b02d69 100644 --- a/packages/metro-bundler/src/lib/TransformCache.js +++ b/packages/metro-bundler/src/lib/TransformCache.js @@ -15,6 +15,7 @@ const crypto = require('crypto'); const debugRead = require('debug')('RNP:TransformCache:Read'); const fs = require('fs'); +const invariant = require('fbjs/lib/invariant'); const mkdirp = require('mkdirp'); const path = require('path'); const rimraf = require('rimraf'); @@ -29,7 +30,6 @@ import type {LocalPath} from '../node-haste/lib/toLocalPath'; type CacheFilePaths = {transformedCode: string, metadata: string}; export type GetTransformCacheKey = (options: {}) => string; -const CACHE_NAME = 'react-native-packager-cache'; const CACHE_SUB_DIR = 'cache'; export type CachedResult = { @@ -68,11 +68,25 @@ const CACHE_FILE_MAX_LAST_ACCESS_TIME = GARBAGE_COLLECTION_PERIOD * 4; class TransformCache { _cacheWasReset: boolean; - _dirPath: string; _lastCollected: ?number; + _rootPath: string; - constructor() { + /** + * The root path is where the data will be stored. It shouldn't contain + * other files other than the cache's own files, so it should start empty + * when the packager is first run. When doing a cache reset, it may be + * completely deleted. + */ + constructor(rootPath: string) { this._cacheWasReset = false; + invariant( + path.isAbsolute(rootPath), + 'root path of the transform cache must be absolute', + ); + require('debug')('RNP:TransformCache:Dir')( + `transform cache directory: ${rootPath}`, + ); + this._rootPath = rootPath; } /** @@ -90,16 +104,14 @@ class TransformCache { * close to each others, one of the workers is going to loose its results no * matter what. */ - writeSync( - props: { - filePath: string, - sourceCode: string, - getTransformCacheKey: GetTransformCacheKey, - transformOptions: WorkerOptions, - transformOptionsKey: string, - result: CachedResult, - }, - ): void { + writeSync(props: { + filePath: string, + sourceCode: string, + getTransformCacheKey: GetTransformCacheKey, + transformOptions: WorkerOptions, + transformOptionsKey: string, + result: CachedResult, + }): void { const cacheFilePath = this._getCacheFilePaths(props); mkdirp.sync(path.dirname(cacheFilePath.transformedCode)); const {result} = props; @@ -202,7 +214,7 @@ class TransformCache { } _resetCache(reporter: Reporter) { - rimraf.sync(this._getCacheDirPath()); + rimraf.sync(this._rootPath); reporter.update({type: 'transform_cache_reset'}); this._cacheWasReset = true; this._lastCollected = Date.now(); @@ -220,7 +232,7 @@ class TransformCache { terminal.log( 'Error: Cleaning up the cache folder failed. Continuing anyway.', ); - terminal.log('The cache folder is: %s', this._getCacheDirPath()); + terminal.log('The cache folder is: %s', this._rootPath); } this._lastCollected = Date.now(); } @@ -231,7 +243,7 @@ class TransformCache { * first. */ _collectCacheIfOldSync() { - const cacheDirPath = this._getCacheDirPath(); + const cacheDirPath = this._rootPath; mkdirp.sync(cacheDirPath); const cacheCollectionFilePath = path.join(cacheDirPath, 'last_collected'); const lastCollected = Number.parseInt( @@ -255,12 +267,10 @@ class TransformCache { * account because it would generate lots of file during development. (The * source hash is stored in the metadata instead). */ - _getCacheFilePaths( - props: { - filePath: string, - transformOptionsKey: string, - }, - ): CacheFilePaths { + _getCacheFilePaths(props: { + filePath: string, + transformOptionsKey: string, + }): CacheFilePaths { const hasher = crypto .createHash('sha1') .update(props.filePath) @@ -268,38 +278,9 @@ class TransformCache { const hash = hasher.digest('hex'); const prefix = hash.substr(0, 2); const fileName = `${hash.substr(2)}`; - const base = path.join( - this._getCacheDirPath(), - CACHE_SUB_DIR, - prefix, - fileName, - ); + const base = path.join(this._rootPath, CACHE_SUB_DIR, prefix, fileName); return {transformedCode: base, metadata: base + '.meta'}; } - - /** - * 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. - */ - _getCacheDirPath() { - if (this._dirPath != null) { - return this._dirPath; - } - const hash = crypto.createHash('sha1').update(__dirname); - if (process.getuid != null) { - hash.update(process.getuid().toString()); - } - this._dirPath = path.join( - require('os').tmpdir(), - CACHE_NAME + '-' + hash.digest('hex'), - ); - require('debug')('RNP:TransformCache:Dir')( - `transform cache directory: ${this._dirPath}`, - ); - return this._dirPath; - } } /** @@ -388,15 +369,13 @@ function tryParseJSON(str: string): any { } } -function hashSourceCode( - props: { - filePath: string, - sourceCode: string, - getTransformCacheKey: GetTransformCacheKey, - transformOptions: WorkerOptions, - transformOptionsKey: string, - }, -): string { +function hashSourceCode(props: { + filePath: string, + sourceCode: string, + getTransformCacheKey: GetTransformCacheKey, + transformOptions: WorkerOptions, + transformOptionsKey: string, +}): string { return crypto .createHash('sha1') .update(props.getTransformCacheKey(props.transformOptions)) diff --git a/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js b/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js index 6f4d19a3..06c81592 100644 --- a/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js +++ b/packages/metro-bundler/src/lib/__tests__/TransformCache-test.js @@ -55,7 +55,7 @@ describe('TransformCache', () => { beforeEach(() => { jest.resetModules(); mockFS.clear(); - transformCache = new (require('../TransformCache'))(); + transformCache = new (require('../TransformCache'))('/cache'); }); it('is caching different files and options separately', () => { diff --git a/packages/metro-bundler/src/node-haste/Module.js b/packages/metro-bundler/src/node-haste/Module.js index c406e80f..26dbdb44 100644 --- a/packages/metro-bundler/src/node-haste/Module.js +++ b/packages/metro-bundler/src/node-haste/Module.js @@ -20,6 +20,7 @@ const fs = require('fs'); const invariant = require('fbjs/lib/invariant'); const isAbsolutePath = require('absolute-path'); const jsonStableStringify = require('json-stable-stringify'); +const path = require('path'); const {join: joinPath, relative: relativePath, extname} = require('path'); @@ -85,8 +86,6 @@ export type ConstructorArgs = { type DocBlock = {+[key: string]: string}; -const TRANSFORM_CACHE = new TransformCache(); - class Module { localPath: LocalPath; path: string; @@ -107,6 +106,8 @@ class Module { _readResultsByOptionsKey: Map; + static _transformCache: TransformCache; + constructor({ depGraphHelpers, localPath, @@ -333,7 +334,7 @@ class Module { return; } invariant(result != null, 'missing result'); - TRANSFORM_CACHE.writeSync({...cacheProps, result}); + Module._getTransformCache().writeSync({...cacheProps, result}); callback(undefined, result); }); } @@ -380,7 +381,7 @@ class Module { transformOptions, transformOptionsKey, ); - const cachedResult = TRANSFORM_CACHE.readSync(cacheProps); + const cachedResult = Module._getTransformCache().readSync(cacheProps); if (cachedResult.result == null) { return { result: null, @@ -468,6 +469,27 @@ 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