Replace the ETag header by the Last-Modified to improve performance

Reviewed By: mjesun

Differential Revision: D5823545

fbshipit-source-id: 57eac5548e626eeed05f9b454e3f54b114193eb0

packages/metro-bundler/src/DeltaBundler/DeltaPatcher.js

@@ -29,6 +29,7 @@ class DeltaPatcher {
   };
   _initialized = false;
   _lastNumModifiedFiles = 0;
+  _lastModifiedDate = new Date();
 
   /**
    * Applies a Delta Bundle to the current bundle.
@@ -55,6 +56,10 @@ class DeltaPatcher {
     this._lastNumModifiedFiles =
       deltaBundle.pre.size + deltaBundle.post.size + deltaBundle.delta.size;
 
+    if (this._lastNumModifiedFiles > 0) {
+      this._lastModifiedDate = new Date();
+    }
+
     this._patchMap(this._lastBundle.pre, deltaBundle.pre);
     this._patchMap(this._lastBundle.post, deltaBundle.post);
     this._patchMap(this._lastBundle.modules, deltaBundle.delta);
@@ -72,6 +77,10 @@ class DeltaPatcher {
     return this._lastNumModifiedFiles;
   }
 
+  getLastModifiedDate(): Date {
+    return this._lastModifiedDate;
+  }
+
   /**
    * Converts the current delta bundle to a standard string bundle, ready to
    * be interpreted by any JS VM.

packages/metro-bundler/src/DeltaBundler/__tests__/DeltaBundler-test.js

@@ -21,6 +21,7 @@ const DeltaTransformer = require('../DeltaTransformer');
 const DeltaBundler = require('../');
 
 describe('DeltaBundler', () => {
+  const OriginalDate = global.Date;
   let deltaBundler;
   let bundler;
   const initialTransformerResponse = {
@@ -31,6 +32,10 @@ describe('DeltaBundler', () => {
     reset: true,
   };
 
+  function setCurrentTime(time: number) {
+    global.Date = jest.fn(() => new OriginalDate(time));
+  }
+
   beforeEach(() => {
     DeltaTransformer.prototype.getDelta = jest
       .fn()
@@ -42,6 +47,8 @@ describe('DeltaBundler', () => {
 
     bundler = new Bundler();
     deltaBundler = new DeltaBundler(bundler, {});
+
+    setCurrentTime(1482363367000);
   });
 
   it('should create a new transformer to build the initial bundle', async () => {

packages/metro-bundler/src/DeltaBundler/__tests__/DeltaPatcher-test.js

@@ -14,11 +14,20 @@
 
 const DeltaPatcher = require('../DeltaPatcher');
 
+const INITIAL_TIME = 1482363367000;
+
 describe('DeltaPatcher', () => {
+  const OriginalDate = global.Date;
   let deltaPatcher;
 
+  function setCurrentTime(time: number) {
+    global.Date = jest.fn(() => new OriginalDate(time));
+  }
+
   beforeEach(() => {
     deltaPatcher = new DeltaPatcher();
+
+    setCurrentTime(INITIAL_TIME);
   });
 
   it('should throw if received a non-reset delta as the initial one', () => {
@@ -117,4 +126,44 @@ describe('DeltaPatcher', () => {
     // A deleted module counts as a modified file.
     expect(deltaPatcher.getLastNumModifiedFiles()).toEqual(2);
   });
+
+  it('should return the time it was last modified', () => {
+    deltaPatcher
+      .applyDelta({
+        reset: 1,
+        pre: new Map([[1, {code: 'pre'}]]),
+        post: new Map([[2, {code: 'post'}]]),
+        delta: new Map([[3, {code: 'middle'}]]),
+      })
+      .stringifyCode();
+
+    expect(deltaPatcher.getLastModifiedDate().getTime()).toEqual(INITIAL_TIME);
+    setCurrentTime(INITIAL_TIME + 1000);
+
+    // Apply empty delta
+    deltaPatcher
+      .applyDelta({
+        reset: 1,
+        pre: new Map(),
+        post: new Map(),
+        delta: new Map(),
+      })
+      .stringifyCode();
+
+    expect(deltaPatcher.getLastModifiedDate().getTime()).toEqual(INITIAL_TIME);
+    setCurrentTime(INITIAL_TIME + 2000);
+
+    deltaPatcher
+      .applyDelta({
+        reset: 1,
+        pre: new Map(),
+        post: new Map([[2, {code: 'newpost'}]]),
+        delta: new Map(),
+      })
+      .stringifyCode();
+
+    expect(deltaPatcher.getLastModifiedDate().getTime()).toEqual(
+      INITIAL_TIME + 2000,
+    );
+  });
 });

packages/metro-bundler/src/DeltaBundler/__tests__/__snapshots__/DeltaBundler-test.js.snap

@@ -6,6 +6,7 @@ Object {
 ;module3
 ;another
 ;post",
+  "lastModified": 2016-12-21T23:36:07.000Z,
   "numModifiedFiles": 4,
 }
 `;
@@ -18,6 +19,7 @@ Object {
 ;post
 ;bananas
 ;apples",
+  "lastModified": 2016-12-21T23:36:07.000Z,
   "numModifiedFiles": 5,
 }
 `;

packages/metro-bundler/src/DeltaBundler/index.js

@@ -113,7 +113,7 @@ class DeltaBundler {
 
   async buildFullBundle(
     options: FullBuildOptions,
-  ): Promise<{bundle: string, numModifiedFiles: number}> {
+  ): Promise<{bundle: string, numModifiedFiles: number, lastModified: Date}> {
     const deltaPatcher = await this._getDeltaPatcher(options);
     let bundle = deltaPatcher.stringifyCode();
 
@@ -123,6 +123,7 @@ class DeltaBundler {
 
     return {
       bundle,
+      lastModified: deltaPatcher.getLastModifiedDate(),
       numModifiedFiles: deltaPatcher.getLastNumModifiedFiles(),
     };
   }

packages/metro-bundler/src/Server/index.js

@@ -923,14 +923,13 @@ class Server {
       }),
     );
 
-    let bundle;
-    let numModifiedFiles;
+    let result;
 
     try {
-      ({bundle, numModifiedFiles} = await this._deltaBundler.buildFullBundle({
+      result = await this._deltaBundler.buildFullBundle({
         ...options,
         deltaBundleId: this.optionsHash(options),
-      }));
+      });
     } catch (error) {
       this._handleError(res, this.optionsHash(options), error);
 
@@ -942,18 +941,24 @@ class Server {
       return;
     }
 
-    const etag = crypto.createHash('md5').update(bundle).digest('hex');
-
-    if (req.headers['if-none-match'] === etag) {
+    if (
+      // We avoid parsing the dates since the client should never send a more
+      // recent date than the one returned by the Delta Bundler (if that's the
+      // case it's fine to return the whole bundle).
+      req.headers['if-modified-since'] === result.lastModified.toUTCString()
+    ) {
       debug('Responding with 304');
       res.writeHead(304);
       res.end();
     } else {
-      res.setHeader(FILES_CHANGED_COUNT_HEADER, String(numModifiedFiles));
+      res.setHeader(
+        FILES_CHANGED_COUNT_HEADER,
+        String(result.numModifiedFiles),
+      );
       res.setHeader('Content-Type', 'application/javascript');
-      res.setHeader('ETag', etag);
-      res.setHeader('Content-Length', String(Buffer.byteLength(bundle)));
-      res.end(bundle);
+      res.setHeader('Last-Modified', result.lastModified.toUTCString());
+      res.setHeader('Content-Length', String(Buffer.byteLength(result.bundle)));
+      res.end(result.bundle);
     }
 
     this._reporter.update({
@@ -964,7 +969,7 @@ class Server {
     debug('Finished response');
     log({
       ...createActionEndEntry(requestingBundleLogEntry),
-      outdated_modules: numModifiedFiles,
+      outdated_modules: result.numModifiedFiles,
       bundler: 'delta',
     });
   }