Make HMR server send full list modules that changed

Summary:
public

Before this diff we were only accepting the module that was modified but the user. This works fine as long as the user doesn't modify the dependencies a module has but once he starts doing so the HMR runtime may fail when updating modules' code because they might might a few dependencies. For instance, if the user changes the `src` a `Image` has to reference an image (using the new asset system) that wasn't on the original bundle the user will get a red box. This diff addresses this by diffing the modules the app currently has with the new ones it should have and including all of them on the HMR update. Note this diffing is only done when the we realize the module that was modified changed it's dependencies so there's no additional overhead on this change.

Reviewed By: vjeux

Differential Revision: D2796325

fb-gh-sync-id: cac95f2e995310634c221bbbb09d9f3e7bc03e8d

react-packager/src/Bundler/index.js

@@ -284,31 +284,29 @@ class Bundler {
     });
   }
 
-  bundleForHMR({
-    entryFile,
-    platform,
-  }) {
-    return this.getDependencies(entryFile, /*isDev*/true, platform).then(response => {
-      const module = response.dependencies.filter(module => module.path === entryFile)[0];
-
-      return Promise.all([
-        module.getName(),
-        this._transformer.loadFileAndTransform(
-          path.resolve(entryFile),
-          // TODO(martinb): pass non null main (t9527509)
-          this._getTransformOptions({main: null}, {hot: true}),
-        ),
-      ]).then(([moduleName, transformedSource]) => {
-        return (`
-          __accept(
-            '${moduleName}',
-            function(global, require, module, exports) {
-              ${transformedSource.code}
-            }
-          );
-        `);
-      });
-    });
+  bundleForHMR(modules) {
+    return Promise.all(
+      modules.map(module => {
+        return Promise.all([
+          module.getName(),
+          this._transformer.loadFileAndTransform(
+            module.path,
+            // TODO(martinb): pass non null main (t9527509)
+            this._getTransformOptions({main: null}, {hot: true}),
+          ),
+        ]).then(([moduleName, transformedSource]) => {
+          return (`
+            __accept(
+              '${moduleName}',
+              function(global, require, module, exports) {
+                ${transformedSource.code}
+              }
+            );
+          `);
+        });
+      })
+    )
+    .then(code => code.join('\n'));
   }
 
   invalidateFile(filePath) {
@@ -319,6 +317,10 @@ class Bundler {
     return this._resolver.getShallowDependencies(entryFile);
   }
 
+  getModuleForPath(entryFile) {
+    return this._resolver.getModuleForPath(entryFile);
+  }
+
   getDependencies(main, isDev, platform) {
     return this._resolver.getDependencies(main, { dev: isDev, platform });
   }

react-packager/src/DependencyResolver/DependencyGraph/index.js

@@ -143,6 +143,13 @@ class DependencyGraph {
     return this._moduleCache.getModule(entryPath).getDependencies();
   }
 
+  /**
+   * Returns the module object for the given path.
+   */
+  getModuleForPath(entryFile) {
+    return this._moduleCache.getModule(entryFile);
+  }
+
   getDependencies(entryPath, platform) {
     return this.load().then(() => {
       platform = this._getRequestPlatform(entryPath, platform);

react-packager/src/Resolver/index.js

@@ -104,6 +104,10 @@ class Resolver {
     return this._depGraph.getShallowDependencies(entryFile);
   }
 
+  getModuleForPath(entryFile) {
+    return this._depGraph.getModuleForPath(entryFile);
+  }
+
   getDependencies(main, options) {
     const opts = getDependenciesValidateOpts(options);
 

react-packager/src/Resolver/polyfills/require.js

@@ -99,12 +99,8 @@
       var mod = modules[id];
 
       if (!mod) {
-        console.warn(
-          'Cannot accept unknown module `' + id + '`. Make sure you\'re not ' +
-          'trying to modify something else other than a module ' +
-          '(i.e.: a polyfill).'
-        );
-        return;
+        define(id, factory);
+        return; // new modules don't need to be accepted
       }
 
       if (!mod.module.hot) {

react-packager/src/Server/index.js

@@ -120,17 +120,6 @@ const bundleOpts = declareOpts({
   },
 });
 
-const hmrBundleOpts = declareOpts({
-  entryFile: {
-    type: 'string',
-    required: true,
-  },
-  platform: {
-    type: 'string',
-    required: true,
-  },
-});
-
 const dependencyOpts = declareOpts({
   platform: {
     type: 'string',
@@ -199,13 +188,10 @@ class Server {
       // 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();
+        this._hmrFileChangeListener(filePath);
         return;
       }
 
-      this._fileChangeListeners.forEach(listener => listener(filePath));
       this._rebuildBundles(filePath);
       this._informChangeWatchers();
     }, 50);
@@ -218,10 +204,6 @@ class Server {
     ]);
   }
 
-  addFileChangeListener(listener) {
-    this._fileChangeListeners.push(listener);
-  }
-
   setHMRFileChangeListener(listener) {
     this._hmrFileChangeListener = listener;
   }
@@ -253,21 +235,18 @@ class Server {
     return this.buildBundle(options);
   }
 
-  buildBundleForHMR(options) {
-    return Promise.resolve().then(() => {
-      if (!options.platform) {
-        options.platform = getPlatformExtension(options.entryFile);
-      }
-
-      const opts = hmrBundleOpts(options);
-      return this._bundler.bundleForHMR(opts);
-    });
+  buildBundleForHMR(modules) {
+    return this._bundler.bundleForHMR(modules);
   }
 
   getShallowDependencies(entryFile) {
     return this._bundler.getShallowDependencies(entryFile);
   }
 
+  getModuleForPath(entryFile) {
+    return this._bundler.getModuleForPath(entryFile);
+  }
+
   getDependencies(options) {
     return Promise.resolve().then(() => {
       if (!options.platform) {