Clean up Bundler

Summary:
The bundler class had duplicated code and parts that were hard to follow, because functions accepted heterogenous arguments, leading to a lot of conditionals.

This commit tries to remove duplication between build steps of different type of bundles, and by accepting less different types of arguments. These differences are now handled further up the call stack.

public

Reviewed By: sebmarkbage

Differential Revision: D2905807

fb-gh-sync-id: ef85ea0d461a9a06a4a64480e014a5324c4ef532

react-packager/src/Bundler/BundleBase.js

@@ -16,7 +16,7 @@ class BundleBase {
     this._finalized = false;
     this._modules = [];
     this._assets = [];
-    this._mainModuleId = this._mainModuleName = undefined;
+    this._mainModuleId = undefined;
   }
 
   isEmpty() {
@@ -31,14 +31,6 @@ class BundleBase {
     this._mainModuleId = moduleId;
   }
 
-  getMainModuleName() {
-    return this._mainModuleName;
-  }
-
-  setMainModuleName(moduleName) {
-    this._mainModuleName = moduleName;
-  }
-
   addModule(module) {
     if (!module instanceof ModuleTransport) {
       throw new Error('Expeceted a ModuleTransport object');
@@ -89,7 +81,6 @@ class BundleBase {
       modules: this._modules,
       assets: this._assets,
       mainModuleId: this.getMainModuleId(),
-      mainModuleName: this.getMainModuleName(),
     };
   }
 
@@ -97,7 +88,6 @@ class BundleBase {
     bundle._assets = json.assets;
     bundle._modules = json.modules;
     bundle.setMainModuleId(json.mainModuleId);
-    bundle.setMainModuleName(json.mainModuleName);
 
     Object.freeze(bundle._modules);
     Object.seal(bundle._modules);

react-packager/src/Bundler/__tests__/Bundle-test.js

@@ -312,28 +312,21 @@ describe('Bundle', () => {
     });
   });
 
-  describe('main module name and id:', function() {
+  describe('main module id:', function() {
-    it('keeps distinct module names and IDs', function() {
+    it('can save a main module ID', function() {
       const id = 'arbitrary module ID';
-      const name = 'arbitrary module name';
       bundle.setMainModuleId(id);
-      bundle.setMainModuleName(name);
-
       expect(bundle.getMainModuleId()).toEqual(id);
-      expect(bundle.getMainModuleName()).toEqual(name);
     });
 
-    it('can serialize and deserialize module ID and name', function() {
+    it('can serialize and deserialize the module ID', function() {
       const id = 'arbitrary module ID';
-      const name = 'arbitrary module name';
       bundle.setMainModuleId(id);
-      bundle.setMainModuleName(name);
       bundle.finalize({});
 
       const deserialized = Bundle.fromJSON(bundle.toJSON());
 
       expect(deserialized.getMainModuleId()).toEqual(id);
-      expect(deserialized.getMainModuleName()).toEqual(name);
     });
   });
 });

react-packager/src/Bundler/index.js

@@ -29,6 +29,8 @@ const version = require('../../../../package.json').version;
 const sizeOf = Promise.denodeify(imageSize);
 const readFile = Promise.denodeify(fs.readFile);
 
+const noop = () => {};
+
 const validateOpts = declareOpts({
   projectRoots: {
     type: 'array',
@@ -159,9 +161,12 @@ class Bundler {
   }
 
   bundle(options) {
+    const {dev, isUnbundle, platform} = options;
+    const moduleSystemDeps =
+      this._resolver.getModuleSystemDependencies({dev, isUnbundle, platform});
     return this._bundle({
       bundle: new Bundle(options.sourceMapUrl),
-      includeSystemDependencies: true,
+      moduleSystemDeps,
       ...options,
     });
   }
@@ -209,7 +214,7 @@ class Bundler {
     );
   }
 
-  bundleForHMR(options) {
+  hmrBundle(options) {
     return this._bundle({
       bundle: new HMRBundle({
         sourceURLFn: this._sourceHMRURL.bind(this, options.platform),
@@ -225,121 +230,50 @@ class Bundler {
 
   _bundle({
     bundle,
-    modules,
     entryFile,
     runModule: runMainModule,
     runBeforeMainModule,
     dev: isDev,
-    includeSystemDependencies,
     platform,
-    unbundle: isUnbundle,
+    moduleSystemDeps = [],
-    hot: hot,
+    hot,
     entryModuleOnly,
-    resolutionResponse,
+    resolutionResponse
   }) {
-    let transformEventId;
+    const onResolutionResponse = response => {
-    const moduleSystemDeps = includeSystemDependencies
+      bundle.setMainModuleId(response.mainModuleId);
-      ? this._resolver.getModuleSystemDependencies(
+      if (bundle.setNumPrependedModules) {
-        { dev: isDev, platform, isUnbundle }
+        bundle.setNumPrependedModules(
-      )
+          response.numPrependedDependencies + moduleSystemDeps.length
-      : [];
+        );
-
+      }
-    const findModules = () => {
-      const findEventId = Activity.startEvent('find dependencies');
-      return this.getDependencies(entryFile, isDev, platform).then(response => {
-        Activity.endEvent(findEventId);
-        bundle.setMainModuleId(response.mainModuleId);
-        bundle.setMainModuleName(response.mainModuleId);
-        if (!entryModuleOnly && bundle.setNumPrependedModules) {
-          bundle.setNumPrependedModules(
-            response.numPrependedDependencies + moduleSystemDeps.length
-          );
-        }
-
-        return {
-          response,
-          modulesToProcess: response.dependencies,
-        };
-      });
-    };
-
-    const useProvidedModules = () => {
-      const moduleId = this._resolver.getModuleForPath(entryFile);
-      bundle.setMainModuleId(moduleId);
-      bundle.setMainModuleName(moduleId);
-      return Promise.resolve({
-        response: resolutionResponse,
-        modulesToProcess: modules
-      });
-    };
-
-    return (
-      modules ? useProvidedModules() : findModules()
-    ).then(({response, modulesToProcess}) => {
-
-      transformEventId = Activity.startEvent('transform');
-
-      let dependencies;
       if (entryModuleOnly) {
-        dependencies = response.dependencies.filter(module =>
+        response.dependencies = response.dependencies.filter(module =>
           module.path.endsWith(entryFile)
         );
       } else {
-        const moduleSystemDeps = includeSystemDependencies
+        response.dependencies = moduleSystemDeps.concat(response.dependencies);
-          ? this._resolver.getModuleSystemDependencies(
-            { dev: isDev, platform, isUnbundle }
-          )
-          : [];
-
-        const modulesToProcess = modules || response.dependencies;
-        dependencies = moduleSystemDeps.concat(modulesToProcess);
       }
-
+    };
-      let bar;
+    const finalizeBundle = ({bundle, transformedModules, response}) =>
-      if (process.stdout.isTTY) {
+      Promise.all(
-        bar = new ProgressBar('transforming [:bar] :percent :current/:total', {
+        transformedModules.map(({module, transformed}) =>
-          complete: '=',
+          bundle.addModule(this._resolver, response, module, transformed)
-          incomplete: ' ',
-          width: 40,
-          total: dependencies.length,
-        });
-      }
-
-      return Promise.all(
-        dependencies.map(
-          module => {
-            return this._transformModule(
-              bundle,
-              module,
-              platform,
-              isDev,
-              hot,
-            ).then(transformed => {
-              if (bar) {
-                bar.tick();
-              }
-
-              return {
-                module,
-                transformed,
-              };
-            });
-          }
         )
-      ).then(transformedModules => Promise.all(
+      ).then(() => {
-        transformedModules.map(({module, transformed}) => {
+        bundle.finalize({runBeforeMainModule, runMainModule});
-          return bundle.addModule(
+        return bundle;
-            this._resolver,
+      });
-            response,
+
-            module,
+    return this._buildBundle({
-            transformed,
+      entryFile,
-          );
+      isDev,
-        })
+      platform,
-      ));
+      bundle,
-    }).then(() => {
+      hot,
-      Activity.endEvent(transformEventId);
+      resolutionResponse,
-      bundle.finalize({runBeforeMainModule, runMainModule});
+      onResolutionResponse,
-      return bundle;
+      finalizeBundle,
     });
   }
 
@@ -351,78 +285,86 @@ class Bundler {
     dev: isDev,
     platform,
   }) {
-    const bundle = new PrepackBundle(sourceMapUrl);
+    const onModuleTransformed = ({module, transformed, response, bundle}) => {
-    const findEventId = Activity.startEvent('find dependencies');
+      const deps = Object.create(null);
-    let transformEventId;
+      const pairs = response.getResolvedDependencyPairs(module);
-    let mainModuleId;
+      if (pairs) {
-
+        pairs.forEach(pair => {
-    return this.getDependencies(entryFile, isDev, platform).then((response) => {
+          deps[pair[0]] = pair[1].path;
-      Activity.endEvent(findEventId);
-      transformEventId = Activity.startEvent('transform');
-
-      let bar;
-      if (process.stdout.isTTY) {
-        bar = new ProgressBar('transforming [:bar] :percent :current/:total', {
-          complete: '=',
-          incomplete: ' ',
-          width: 40,
-          total: response.dependencies.length,
         });
       }
 
-      mainModuleId = response.mainModuleId;
+      return module.getName().then(name => {
-
+        bundle.addModule(name, transformed, deps, module.isPolyfill());
-      return Promise.all(
+      });
-        response.dependencies.map(
+    };
-          module => this._transformModule(
+    const finalizeBundle = ({bundle, response}) => {
-            bundle,
+      const {mainModuleId} = response;
-            module,
+      bundle.finalize({runBeforeMainModule, runMainModule, mainModuleId});
-            platform,
-            isDev,
-          ).then(transformed => {
-            if (bar) {
-              bar.tick();
-            }
-
-            var deps = Object.create(null);
-            var pairs = response.getResolvedDependencyPairs(module);
-            if (pairs) {
-              pairs.forEach(pair => {
-                deps[pair[0]] = pair[1].path;
-              });
-            }
-
-            return module.getName().then(name => {
-              bundle.addModule(name, transformed, deps, module.isPolyfill());
-            });
-          })
-        )
-      );
-    }).then(() => {
-      Activity.endEvent(transformEventId);
-      bundle.finalize({runBeforeMainModule, runMainModule, mainModuleId });
       return bundle;
+    };
+
+    return this._buildBundle({
+      entryFile,
+      isDev,
+      platform,
+      onModuleTransformed,
+      finalizeBundle,
+      bundle: new PrepackBundle(sourceMapUrl),
     });
   }
 
-  _transformModuleForHMR(module, platform) {
+  _buildBundle({
-    if (module.isAsset()) {
+    entryFile,
-      return this._generateAssetObjAndCode(module, platform).then(
+    isDev,
-        ({asset, code}) => {
+    platform,
-          return {
+    bundle,
-            code,
+    hot,
-          };
+    resolutionResponse,
-        }
+    onResolutionResponse = noop,
-      );
+    onModuleTransformed = noop,
-    } else {
+    finalizeBundle = noop,
-      // TODO(martinb): pass non null main (t9527509)
+  }) {
-      return this._getTransformOptions(
+    const findEventId = Activity.startEvent('find dependencies');
-        {main: null, dev: true, platform: 'ios'}, // TODO(martinb): avoid hard-coding platform
+    if (!resolutionResponse) {
-        {hot: true},
+      resolutionResponse = this.getDependencies(entryFile, isDev, platform);
-      ).then(options => {
-        return this._transformer.loadFileAndTransform(module.path, options);
-      });
     }
+
+    return Promise.resolve(resolutionResponse).then(response => {
+      Activity.endEvent(findEventId);
+      onResolutionResponse(response);
+
+      const transformEventId = Activity.startEvent('transform');
+      const bar = process.stdout.isTTY
+          ? new ProgressBar('transforming [:bar] :percent :current/:total', {
+              complete: '=',
+              incomplete: ' ',
+              width: 40,
+              total: response.dependencies.length,
+            })
+          : {tick() {}};
+      const transformPromises =
+        response.dependencies.map(module =>
+          this._transformModule({
+            mainModuleName: response.mainModuleId,
+            bundle,
+            module,
+            platform,
+            dev: isDev,
+            hot
+          }).then(transformed => {
+            bar.tick();
+            onModuleTransformed({module, transformed, response, bundle});
+            return {module, transformed};
+          })
+        );
+      return Promise.all(transformPromises).then(transformedModules => {
+        Activity.endEvent(transformEventId);
+        return Promise
+          .resolve(finalizeBundle({bundle, transformedModules, response}))
+          .then(() => bundle);
+      });
+    });
   }
 
   invalidateFile(filePath) {
@@ -489,7 +431,14 @@ class Bundler {
     );
   }
 
-  _transformModule(bundle, module, platform = null, dev = true, hot = false) {
+  _transformModule({
+    bundle,
+    module,
+    mainModuleName,
+    platform = null,
+    dev = true,
+    hot = false,
+  }) {
     if (module.isAsset_DEPRECATED()) {
       return this._generateAssetModule_DEPRECATED(bundle, module);
     } else if (module.isAsset()) {
@@ -499,12 +448,12 @@ class Bundler {
     } else {
       return this._getTransformOptions(
         {
-          bundleEntry: bundle.getMainModuleName(),
+          bundleEntry: mainModuleName,
           platform: platform,
           dev: dev,
           modulePath: module.path,
         },
-        {hot: hot},
+        {hot},
       ).then(options => {
         return this._transformer.loadFileAndTransform(
           path.resolve(module.path),

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

@@ -19,6 +19,21 @@ class ResolutionResponse {
     this._finalized = false;
   }
 
+  copy(properties) {
+    const {
+      dependencies = this.dependencies,
+      asyncDependencies = this.asyncDependencies,
+      mainModuleId = this.mainModuleId,
+      mocks = this.mocks,
+    } = properties;
+    return Object.assign(new this.constructor(), this, {
+      dependencies,
+      asyncDependencies,
+      mainModuleId,
+      mocks,
+    });
+  }
+
   _assertNotFinalized() {
     if (this._finalized) {
       throw new Error('Attempted to mutate finalized response.');

react-packager/src/Server/index.js

@@ -241,7 +241,7 @@ class Server {
   }
 
   buildBundleForHMR(modules) {
-    return this._bundler.bundleForHMR(modules);
+    return this._bundler.hmrBundle(modules);
   }
 
   getShallowDependencies(entryFile) {