Do not read file on the main process when using SHA-1 + experimental caches

Reviewed By: jeanlauliac

Differential Revision: D7443476

fbshipit-source-id: 4363b35ee5cbf988758b249c9a9c3d5ca606f317

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

@@ -30,6 +30,7 @@ var fs = require('fs');
 const os = require('os');
 const path = require('path');
 const mkdirp = require('mkdirp');
+const Module = require('../../node-haste/Module');
 
 var commonOptions = {
   allowBundleUpdates: false,
@@ -117,4 +118,59 @@ describe('Bundler', function() {
     expect(result.code).toEqual(minifiedCode);
     expect(result.map).toEqual([]);
   });
+
+  it('uses new cache layers when transforming if requested to do so', async () => {
+    const get = jest.fn();
+    const set = jest.fn();
+
+    const bundlerInstance = new Bundler({
+      ...commonOptions,
+      cacheStores: [{get, set}],
+      projectRoots,
+    });
+
+    const depGraph = {
+      getSha1: jest.fn(() => '0123456789012345678901234567890123456789'),
+    };
+
+    jest.spyOn(bundlerInstance, 'getDependencyGraph').mockImplementation(() => {
+      return new Promise(resolve => {
+        resolve(depGraph);
+      });
+    });
+
+    const module = new Module({
+      file: '/root/foo.js',
+      localPath: 'foo.js',
+      experimentalCaches: true,
+    });
+
+    require('../../JSTransformer').prototype.transform.mockReturnValue({
+      sha1: 'abcdefabcdefabcdefabcdefabcdefabcdefabcd',
+      result: {},
+    });
+
+    await bundlerInstance._cachedTransformCode(module, null, {});
+
+    // We got the SHA-1 of the file from the dependency graph.
+    expect(depGraph.getSha1).toBeCalledWith('/root/foo.js');
+
+    // Only one get, with the original SHA-1.
+    expect(get).toHaveBeenCalledTimes(1);
+    expect(get.mock.calls[0][0].toString('hex')).toMatch(
+      '0123456789012345678901234567890123456789',
+    );
+
+    // Only one set, with the *modified* SHA-1. This happens when the file gets
+    // modified between querying the caches and saving.
+    expect(set).toHaveBeenCalledTimes(1);
+    expect(set.mock.calls[0][0].toString('hex')).toMatch(
+      'abcdefabcdefabcdefabcdefabcdefabcdefabcd',
+    );
+
+    // But, the common part of the key remains the same.
+    expect(get.mock.calls[0][0].toString('hex').substr(0, 32)).toBe(
+      set.mock.calls[0][0].toString('hex').substr(0, 32),
+    );
+  });
 });

packages/metro/src/Bundler/index.js

@@ -257,12 +257,14 @@ class Bundler {
 
   async _cachedTransformCode(
     module: Module,
-    code: string,
+    code: ?string,
     transformCodeOptions: WorkerOptions,
   ): Promise<TransformedCode> {
     const cache = this._cache;
-    let result;
-    let key;
+    let data = null;
+    let partialKey;
+    let fullKey;
+    let sha1;
 
     // First, try getting the result from the cache if enabled.
     if (cache) {
@@ -287,13 +289,12 @@ class Bundler {
         }
       }
 
-      key = stableHash([
+      partialKey = stableHash([
         // This is the hash related to the global Bundler config.
         this._baseHash,
 
         // Path and code hash.
         module.localPath,
-        (await this.getDependencyGraph()).getSha1(module.path),
 
         // We cannot include "transformCodeOptions" because of "projectRoot".
         assetDataPlugins,
@@ -306,12 +307,25 @@ class Bundler {
         platform,
       ]);
 
-      result = await cache.get(key);
+      sha1 = (await this.getDependencyGraph()).getSha1(module.path);
+      fullKey = Buffer.concat([partialKey, Buffer.from(sha1, 'hex')]);
+
+      const result = await cache.get(fullKey);
+
+      if (result) {
+        data = {result, sha1};
+      }
+    }
+
+    if (!cache && code == null) {
+      throw new Error(
+        'When not using experimental caches, code should always be provided',
+      );
     }
 
     // Second, if there was no result, compute it ourselves.
-    if (!result) {
-      result = await this._transformer.transform(
+    if (!data) {
+      data = await this._transformer.transform(
         module.path,
         module.localPath,
         code,
@@ -323,11 +337,17 @@ class Bundler {
     }
 
     // Third, propagate the result to all cache layers.
-    if (key && cache) {
-      cache.set(key, result);
+    if (fullKey && partialKey && sha1 && cache) {
+      // It could be that the SHA-1 we had back when we sent to the transformer
+      // was outdated; if so, recompute it.
+      if (sha1 !== data.sha1) {
+        fullKey = Buffer.concat([partialKey, Buffer.from(data.sha1, 'hex')]);
+      }
+
+      cache.set(fullKey, data.result);
     }
 
-    return result;
+    return data.result;
   }
 }
 

packages/metro/src/JSTransformer/__tests__/Transformer-test.js

@@ -55,6 +55,7 @@ describe('Transformer', function() {
       api.transform.mockImplementation(() => {
         return {
           result: 'transformed(code)',
+          sha1: '4ea962697c876e2674d107f0fec6798414f5bf45',
           transformFileStartLogEntry: {},
           transformFileEndLogEntry: {},
         };

packages/metro/src/JSTransformer/index.js

@@ -35,6 +35,11 @@ type Reporters = {
   +stderrChunk: (chunk: string) => mixed,
 };
 
+type TransformerResult = {
+  result: TransformedCode,
+  sha1: string,
+};
+
 module.exports = class Transformer {
   _worker: WorkerInterface;
   _transformModulePath: string;
@@ -101,12 +106,12 @@ module.exports = class Transformer {
   async transform(
     filename: string,
     localPath: LocalPath,
-    code: string,
+    code: ?string,
     isScript: boolean,
     options: Options,
     assetExts: $ReadOnlyArray<string>,
     assetRegistryPath: string,
-  ): Promise<TransformedCode> {
+  ): Promise<TransformerResult> {
     try {
       debug('Started transforming file', filename);
 
@@ -128,7 +133,10 @@ module.exports = class Transformer {
       Logger.log(data.transformFileStartLogEntry);
       Logger.log(data.transformFileEndLogEntry);
 
-      return data.result;
+      return {
+        result: data.result,
+        sha1: Buffer.from(data.sha1, 'hex'),
+      };
     } catch (err) {
       debug('Failed transform file', filename);
 

packages/metro/src/JSTransformer/worker/index.js

@@ -15,6 +15,8 @@ const JsFileWrapping = require('../../ModuleGraph/worker/JsFileWrapping');
 const assetTransformer = require('../../assetTransformer');
 const collectDependencies = require('../../ModuleGraph/worker/collectDependencies');
 const constantFoldingPlugin = require('./constant-folding-plugin');
+const crypto = require('crypto');
+const fs = require('fs');
 const getMinifier = require('../../lib/getMinifier');
 const inlinePlugin = require('./inline-plugin');
 const optimizeDependencies = require('../../ModuleGraph/worker/optimizeDependencies');
@@ -90,6 +92,7 @@ export type Options = TransformOptionsStrict;
 
 export type Data = {
   result: TransformedCode,
+  sha1: string,
   transformFileStartLogEntry: LogEntry,
   transformFileEndLogEntry: LogEntry,
 };
@@ -115,7 +118,7 @@ function getDynamicDepsBehavior(
 async function transformCode(
   filename: string,
   localPath: LocalPath,
-  sourceCode: string,
+  sourceCode: ?string,
   transformerPath: string,
   isScript: boolean,
   options: Options,
@@ -124,6 +127,10 @@ async function transformCode(
   asyncRequireModulePath: string,
   dynamicDepsInPackages: DynamicRequiresBehavior,
 ): Promise<Data> {
+  if (sourceCode == null) {
+    sourceCode = fs.readFileSync(filename, 'utf8');
+  }
+
   const transformFileStartLogEntry = {
     action_name: 'Transforming file',
     action_phase: 'start',
@@ -132,6 +139,11 @@ async function transformCode(
     start_timestamp: process.hrtime(),
   };
 
+  const sha1 = crypto
+    .createHash('sha1')
+    .update(sourceCode)
+    .digest('hex');
+
   if (filename.endsWith('.json')) {
     const code = JsFileWrapping.wrapJson(sourceCode);
 
@@ -142,6 +154,7 @@ async function transformCode(
 
     return {
       result: {dependencies: [], code, map: []},
+      sha1,
       transformFileStartLogEntry,
       transformFileEndLogEntry,
     };
@@ -241,6 +254,7 @@ async function transformCode(
 
   return {
     result: {dependencies, code: result.code, map},
+    sha1,
     transformFileStartLogEntry,
     transformFileEndLogEntry,
   };

packages/metro/src/node-haste/Module.js

@@ -45,7 +45,7 @@ export type CachedReadResult = ?ReadResult;
 
 export type TransformCode = (
   module: Module,
-  sourceCode: string,
+  sourceCode: ?string,
   transformOptions: WorkerOptions,
 ) => Promise<TransformedCode>;
 
@@ -304,12 +304,18 @@ class Module {
     // TODO: T26134860 Cache layer lives inside the transformer now; just call
     // the transform method.
     if (this._experimentalCaches) {
-      const sourceCode = this._readSourceCode();
-
-      return {
-        ...(await this._transformCode(this, sourceCode, transformOptions)),
-        sourceCode,
+      // Source code is read on the worker.
+      const data = {
+        ...(await this._transformCode(this, null, transformOptions)),
       };
+
+      // eslint-disable-next-line lint/flow-no-fixme
+      // $FlowFixMe: Flow wants "value" here, where the get is for AVOIDING it.
+      Object.defineProperty(data, 'sourceCode', {
+        get: () => this._readSourceCode.bind(this),
+      });
+
+      return data;
     }
 
     const cached = this.readCached(transformOptions);

packages/metro/src/node-haste/__tests__/Module-test.js

@@ -120,8 +120,9 @@ describe('Module', () => {
       expect(res2.dependencies).toEqual(['dep1', 'dep2']);
       expect(transformCode).toHaveBeenCalledTimes(2);
 
-      // Code was only read once, though.
-      expect(fs.readFileSync).toHaveBeenCalledTimes(1);
+      // Code was never read, though, because experimental caches read on the
+      // worker, to speed up local cache!
+      expect(fs.readFileSync).not.toHaveBeenCalled();
     });
   });