Do not cache hash generation in the AssetServer

Reviewed By: davidaurelio

Differential Revision: D6436249

fbshipit-source-id: eebbd92ebfdf1c8f12dbbcf3f5a66166fb1bc828

packages/metro/src/AssetServer/__tests__/AssetServer-test.js

@@ -288,7 +288,7 @@ describe('AssetServer', () => {
       it('changes the hash when the passed-in file watcher emits an `all` event', () => {
         return server.getAssetData('/root/imgs/b.jpg').then(initialData => {
           mockFS.root.imgs['b@4x.jpg'] = 'updated data';
-          server.onFileChange('all', '/root/imgs/b@4x.jpg');
+
           return server
             .getAssetData('/root/imgs/b.jpg')
             .then(data => expect(data.hash).not.toEqual(initialData.hash));

packages/metro/src/AssetServer/index.js

@@ -14,7 +14,6 @@
 
 const AssetPaths = require('../node-haste/lib/AssetPaths');
 
-const crypto = require('crypto');
 const denodeify = require('denodeify');
 const fs = require('fs');
 const imageSize = require('image-size');
@@ -22,15 +21,13 @@ const path = require('path');
 const toLocalPath = require('../node-haste/lib/toLocalPath');
 
 const {isAssetTypeAnImage} = require('../Bundler/util');
-const {findRoot, getAbsoluteAssetRecord, hashFiles} = require('./util');
+const {
+  findRoot,
+  getAbsoluteAssetInfo,
+  getAbsoluteAssetRecord,
+} = require('./util');
 
-type AssetInfo = {|
-  files: Array<string>,
-  hash: string,
-  name: string,
-  scales: Array<number>,
-  type: string,
-|};
+import type {AssetInfo} from './util';
 
 export type AssetData = {|
   __packager_asset: boolean,
@@ -49,13 +46,9 @@ const readFile = denodeify(fs.readFile);
 
 class AssetServer {
   _roots: $ReadOnlyArray<string>;
-  _hashes: Map<?string, string>;
-  _files: Map<string, string>;
 
   constructor(options: {|+projectRoots: $ReadOnlyArray<string>|}) {
     this._roots = options.projectRoots;
-    this._hashes = new Map();
-    this._files = new Map();
   }
 
   get(assetPath: string, platform: ?string = null): Promise<Buffer> {
@@ -78,29 +71,11 @@ class AssetServer {
     assetPath: string,
     platform: ?string = null,
   ): Promise<AssetInfo> {
-    const nameData = AssetPaths.parse(
-      assetPath,
-      new Set(platform != null ? [platform] : []),
-    );
-    const {name, type} = nameData;
+    const dir = await findRoot(this._roots, path.dirname(assetPath), assetPath);
 
-    const {scales, files} = await this._getAssetRecord(assetPath, platform);
+    const assetAbsolutePath = path.join(dir, path.basename(assetPath));
 
-    const hash = this._hashes.get(assetPath);
-    if (hash != null) {
-      return {files, hash, name, scales, type};
-    }
-
-    const hasher = crypto.createHash('md5');
-
-    if (files.length > 0) {
-      await hashFiles(files.slice(), hasher);
-    }
-
-    const freshHash = hasher.digest('hex');
-    this._hashes.set(assetPath, freshHash);
-    files.forEach(f => this._files.set(f, assetPath));
-    return {files, hash: freshHash, name, scales, type};
+    return await getAbsoluteAssetInfo(assetAbsolutePath, platform);
   }
 
   async getAssetData(
@@ -116,7 +91,7 @@ class AssetServer {
     }
 
     const isImage = isAssetTypeAnImage(path.extname(assetPath).slice(1));
-    const assetData = await this._getAssetInfo(localPath, platform);
+    const assetData = await getAbsoluteAssetInfo(assetPath, platform);
     const dimensions = isImage ? imageSize(assetData.files[0]) : null;
     const scale = assetData.scales[0];
 
@@ -134,10 +109,6 @@ class AssetServer {
     };
   }
 
-  onFileChange(type: string, filePath: string) {
-    this._hashes.delete(this._files.get(filePath));
-  }
-
   /**
    * Given a request for an image by path. That could contain a resolution
    * postfix, we need to find that image (or the closest one to it's resolution)

packages/metro/src/AssetServer/util.js

@@ -14,6 +14,7 @@
 
 const AssetPaths = require('../node-haste/lib/AssetPaths');
 
+const crypto = require('crypto');
 const denodeify = require('denodeify');
 const fs = require('fs');
 const path = require('path');
@@ -23,6 +24,27 @@ const readDir = denodeify(fs.readdir);
 
 import type {AssetPath} from '../node-haste/lib/AssetPaths';
 
+export type AssetInfo = {|
+  files: Array<string>,
+  hash: string,
+  name: string,
+  scales: Array<number>,
+  type: string,
+|};
+
+const hashFiles = denodeify(function hashFilesCb(files, hash, callback) {
+  if (!files.length) {
+    callback(null);
+    return;
+  }
+
+  fs
+    .createReadStream(files.shift())
+    .on('data', data => hash.update(data))
+    .once('end', () => hashFilesCb(files, hash, callback))
+    .once('error', error => callback(error));
+});
+
 function buildAssetMap(
   dir: string,
   files: $ReadOnlyArray<string>,
@@ -82,19 +104,6 @@ function getAssetKey(assetName, platform) {
   }
 }
 
-function hashFiles(files, hash, callback) {
-  if (!files.length) {
-    callback(null);
-    return;
-  }
-
-  fs
-    .createReadStream(files.shift())
-    .on('data', data => hash.update(data))
-    .once('end', () => hashFiles(files, hash, callback))
-    .once('error', error => callback(error));
-}
-
 async function getAbsoluteAssetRecord(
   assetPath: string,
   platform: ?string = null,
@@ -171,8 +180,28 @@ async function findRoot(
   );
 }
 
+async function getAbsoluteAssetInfo(
+  assetPath: string,
+  platform: ?string = null,
+): Promise<AssetInfo> {
+  const nameData = AssetPaths.parse(
+    assetPath,
+    new Set(platform != null ? [platform] : []),
+  );
+  const {name, type} = nameData;
+
+  const {scales, files} = await getAbsoluteAssetRecord(assetPath, platform);
+  const hasher = crypto.createHash('md5');
+
+  if (files.length > 0) {
+    await hashFiles(files.slice(), hasher);
+  }
+
+  return {files, hash: hasher.digest('hex'), name, scales, type};
+}
+
 module.exports = {
   findRoot,
+  getAbsoluteAssetInfo,
   getAbsoluteAssetRecord,
-  hashFiles: denodeify(hashFiles),
 };

packages/metro/src/Server/index.js

@@ -276,8 +276,6 @@ class Server {
   }
 
   onFileChange(type: string, filePath: string) {
-    this._assetServer.onFileChange(type, filePath);
-
     Promise.all(
       this._fileChangeListeners.map(listener => listener(filePath)),
     ).then(