Make HMR faster (2)

Summary:
public

The HMR listener needs to be invoked on the non debounced callback to avoid loosing updates if 2 files are updated within the debounce configured time.

Also, improve the time it takes to send HMR updates by avoiding rebuilding the bundle when the listener is defined. Instead just invalidate the bundles cache so that if the user reloads or disables Hot Loading the packager rebuilds the requested bundle.

Reviewed By: davidaurelio

Differential Revision: D2863141

fb-gh-sync-id: 3ab500eacbd4a2e4b63619755e5eabf8afdd1db9

local-cli/server/util/attachHMRServer.js

@@ -109,10 +109,10 @@ function attachHMRServer({httpServer, path, packagerServer}) {
 
         packagerServer.setHMRFileChangeListener((filename, stat) => {
           if (!client) {
-            return Promise.resolve();
+            return;
           }
 
-          return stat.then(() => {
+          stat.then(() => {
             return packagerServer.getShallowDependencies(filename)
               .then(deps => {
                 if (!client) {

packager/react-packager/src/Server/__tests__/Server-test.js

@@ -194,18 +194,6 @@ describe('processRequest', () => {
     });
 
     it('rebuilds the bundles that contain a file when that file is changed', () => {
-      testChangingFileWith(() => new Server(options));
-    });
-
-    it('rebuilds the bundles that contain a file when that file is changed, even when hot loading is enabled', () => {
-      testChangingFileWith(() => {
-        const server = new Server(options);
-        server.setHMRFileChangeListener(() => Promise.resolve());
-        return server;
-      });
-    });
-
-    function testChangingFileWith(createServer) {
       const bundleFunc = jest.genMockFunction();
       bundleFunc
         .mockReturnValueOnce(
@@ -225,7 +213,7 @@ describe('processRequest', () => {
 
       Bundler.prototype.bundle = bundleFunc;
 
-      server = createServer();
+      server = new Server(options);
 
       requestHandler = server.processRequest.bind(server);
 
@@ -248,7 +236,55 @@ describe('processRequest', () => {
           expect(response.body).toEqual('this is the rebuilt source')
         );
       jest.runAllTicks();
-    }
+    });
+
+    it('rebuilds the bundles that contain a file when that file is changed, even when hot loading is enabled', () => {
+      const bundleFunc = jest.genMockFunction();
+      bundleFunc
+        .mockReturnValueOnce(
+          Promise.resolve({
+            getSource: () => 'this is the first source',
+            getSourceMap: () => {},
+            getEtag: () => () => 'this is an etag',
+          })
+        )
+        .mockReturnValue(
+          Promise.resolve({
+            getSource: () => 'this is the rebuilt source',
+            getSourceMap: () => {},
+            getEtag: () => () => 'this is an etag',
+          })
+        );
+
+      Bundler.prototype.bundle = bundleFunc;
+
+      const server = new Server(options);
+      server.setHMRFileChangeListener(() => {});
+
+      requestHandler = server.processRequest.bind(server);
+
+      makeRequest(requestHandler, 'mybundle.bundle?runModule=true')
+        .done(response => {
+          expect(response.body).toEqual('this is the first source');
+          expect(bundleFunc.mock.calls.length).toBe(1);
+        });
+
+      jest.runAllTicks();
+
+      triggerFileChange('all','path/file.js', options.projectRoots[0]);
+      jest.runAllTimers();
+      jest.runAllTicks();
+
+      expect(bundleFunc.mock.calls.length).toBe(1);
+      server.setHMRFileChangeListener(null);
+
+      makeRequest(requestHandler, 'mybundle.bundle?runModule=true')
+        .done(response => {
+          expect(response.body).toEqual('this is the rebuilt source');
+          expect(bundleFunc.mock.calls.length).toBe(2);
+        });
+      jest.runAllTicks();
+    });
   });
 
   describe('/onchange endpoint', () => {

packager/react-packager/src/Server/index.js

@@ -192,23 +192,8 @@ class Server {
     this._fileWatcher.on('all', this._onFileChange.bind(this));
 
     this._debouncedFileChangeHandler = _.debounce(filePath => {
-      const onFileChange = () => {
-        this._rebuildBundles(filePath);
-        this._informChangeWatchers();
-      };
-
-      // 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,
-          this._bundler.stat(filePath),
-        ).then(onFileChange).done();
-        return;
-      }
-
-      onFileChange();
+      this._rebuildBundles(filePath);
+      this._informChangeWatchers();
     }, 50);
   }
 
@@ -288,11 +273,26 @@ class Server {
   _onFileChange(type, filepath, root) {
     const absPath = path.join(root, filepath);
     this._bundler.invalidateFile(absPath);
+
+    // If Hot Loading is enabled avoid rebuilding bundles and sending live
+    // updates. Instead, send the HMR updates right away and clear the bundles
+    // cache so that if the user reloads we send them a fresh bundle
+    if (this._hmrFileChangeListener) {
+      // Clear cached bundles in case user reloads
+      this._clearBundles();
+      this._hmrFileChangeListener(absPath, this._bundler.stat(absPath));
+      return;
+    }
+
     // Make sure the file watcher event runs through the system before
     // we rebuild the bundles.
     this._debouncedFileChangeHandler(absPath);
   }
 
+  _clearBundles() {
+    this._bundles = Object.create(null);
+  }
+
   _rebuildBundles() {
     const buildBundle = this.buildBundle.bind(this);
     const bundles = this._bundles;