From e75e861116b99de0d1815ecf5eb025e125177e06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Bigio?= <martinb@fb.com>
Date: Tue, 29 Dec 2015 18:24:10 -0800
Subject: [PATCH] Make Hot Loading faster

Summary:
public

Before this this when a file was changed besides sending the HMR update we rebuild every single bundle that the packager had build (to serve it faster when the user hit cmd+r). Since when hot loading is enabled we don't do cmd+r all this work was pointless (except for when you're developing multiple apps using the same packager instance at the same time, which we can assume is very uncommon). As a consequence, the HMR update was competing with the rebundling job making HMR quite slow (i.e.: on one huge internal app it took up to 6s for the HMR changes to get applied).

So, this diff tweaks the file change listener so that we don't rebundle nor invoke the fileWatchers (use for live reload which is also useless when hot load is enabled) when hot loading is enabled. Also, it makes the HMR listener more high pri than the other listeners so that the HMR dev experience is as good as it can get.

Reviewed By: vjeux

Differential Revision: D2793827

fb-gh-sync-id: 724930db9f44974c15ad3f562910b0885e44efde
---
 local-cli/server/util/attachHMRServer.js    | 70 ++++++++++-----------
 packager/react-packager/src/Server/index.js | 14 +++++
 2 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/local-cli/server/util/attachHMRServer.js b/local-cli/server/util/attachHMRServer.js
index 572d3792a..e039edef5 100644
--- a/local-cli/server/util/attachHMRServer.js
+++ b/local-cli/server/util/attachHMRServer.js
@@ -20,6 +20,7 @@ function attachHMRServer({httpServer, path, packagerServer}) {
 
   function disconnect() {
     client = null;
+    packagerServer.setHMRFileChangeListener(null);
   }
 
   // Returns a promise with the full list of dependencies and the shallow
@@ -59,41 +60,6 @@ function attachHMRServer({httpServer, path, packagerServer}) {
     });
   }
 
-  packagerServer.addFileChangeListener(filename => {
-    if (!client) {
-      return;
-    }
-
-    packagerServer.getShallowDependencies(filename)
-      .then(deps => {
-        // if the file dependencies have change we need to invalidate the
-        // dependencies caches because the list of files we need to send to the
-        // client may have changed
-        if (arrayEquals(deps, client.shallowDependencies[filename])) {
-          return Promise.resolve();
-        }
-        return getDependencies(client.platform, client.bundleEntry)
-          .then(({dependenciesCache, shallowDependencies}) => {
-            // invalidate caches
-            client.dependenciesCache = dependenciesCache;
-            client.shallowDependencies = shallowDependencies;
-          });
-      })
-      .then(() => {
-        // make sure the file was modified is part of the bundle
-        if (!client.shallowDependencies[filename]) {
-          return;
-        }
-
-        return packagerServer.buildBundleForHMR({
-          platform: client.platform,
-          entryFile: filename,
-        })
-        .then(bundle => client.ws.send(bundle));
-      })
-      .done();
-  });
-
   const WebSocketServer = require('ws').Server;
   const wss = new WebSocketServer({
     server: httpServer,
@@ -115,6 +81,40 @@ function attachHMRServer({httpServer, path, packagerServer}) {
           shallowDependencies,
         };
 
+        packagerServer.setHMRFileChangeListener(filename => {
+          if (!client) {
+            return Promise.resolve();
+          }
+
+          return packagerServer.getShallowDependencies(filename)
+            .then(deps => {
+              // if the file dependencies have change we need to invalidate the
+              // dependencies caches because the list of files we need to send
+              // to the client may have changed
+              if (arrayEquals(deps, client.shallowDependencies[filename])) {
+                return Promise.resolve();
+              }
+              return getDependencies(client.platform, client.bundleEntry)
+                .then(({dependenciesCache, shallowDependencies}) => {
+                  // invalidate caches
+                  client.dependenciesCache = dependenciesCache;
+                  client.shallowDependencies = shallowDependencies;
+                });
+            })
+            .then(() => {
+              // make sure the file was modified is part of the bundle
+              if (!client.shallowDependencies[filename]) {
+                return;
+              }
+
+              return packagerServer.buildBundleForHMR({
+                platform: client.platform,
+                entryFile: filename,
+              })
+              .then(bundle => client.ws.send(bundle));
+            });
+        });
+
         client.ws.on('error', e => {
           console.error('[Hot Module Replacement] Unexpected error', e);
           disconnect();
diff --git a/packager/react-packager/src/Server/index.js b/packager/react-packager/src/Server/index.js
index a6bb6b344..717ef9e17 100644
--- a/packager/react-packager/src/Server/index.js
+++ b/packager/react-packager/src/Server/index.js
@@ -191,6 +191,16 @@ class Server {
     this._fileWatcher.on('all', this._onFileChange.bind(this));
 
     this._debouncedFileChangeHandler = _.debounce(filePath => {
+      // if Hot Loading is enabled avoid rebuilding bundles and sending live
+      // updates. Instead, send the HMR updates right away and once that
+      // finishes, invoke any other file change listener.
+      if (this._hmrFileChangeListener) {
+        this._hmrFileChangeListener(filePath).then(() => {
+          this._fileChangeListeners.forEach(listener => listener(filePath));
+        }).done();
+        return;
+      }
+
       this._fileChangeListeners.forEach(listener => listener(filePath));
       this._rebuildBundles(filePath);
       this._informChangeWatchers();
@@ -208,6 +218,10 @@ class Server {
     this._fileChangeListeners.push(listener);
   }
 
+  setHMRFileChangeListener(listener) {
+    this._hmrFileChangeListener = listener;
+  }
+
   buildBundle(options) {
     return Promise.resolve().then(() => {
       if (!options.platform) {