packager: cleanup build status reporting

Reviewed By: cpojer

Differential Revision: D4521560

fbshipit-source-id: 8943e179192522da5145e4f14f0b648d3548cc36

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

@@ -544,6 +544,35 @@ class Server {
     return JSON.stringify(Object.assign({}, options, { onProgress: null }));
   }
 
+  /**
+   * Ensure we properly report the promise of a build that's happening,
+   * including failed builds. We use that separately for when we update a bundle
+   * and for when we build for scratch.
+   */
+  _reportBundlePromise(
+    options: {entryFile: string},
+    bundlePromise: Promise<Bundle>,
+  ): Promise<Bundle> {
+    this._reporter.update({
+      entryFilePath: options.entryFile,
+      type: 'bundle_build_started',
+    });
+    return bundlePromise.then(bundle => {
+      this._reporter.update({
+        entryFilePath: options.entryFile,
+        type: 'bundle_build_done',
+      });
+      return bundle;
+    }, error => {
+      this._reporter.update({
+        entryFilePath: options.entryFile,
+        error,
+        type: 'bundle_build_failed',
+      });
+      return Promise.reject(error);
+    });
+  }
+
   _useCachedOrUpdateOrCreateBundle(options: {
     entryFile: string,
     platform?: string,
@@ -567,11 +596,6 @@ class Server {
               action_name: 'Updating existing bundle',
               outdated_modules: outdated.size,
             }));
-          this._reporter.update({
-            type: 'bundle_update_existing',
-            entryFilePath: options.entryFile,
-            outdatedModuleCount: outdated.size,
-          });
 
           debug('Attempt to update existing bundle');
 
@@ -644,7 +668,7 @@ class Server {
             debug('Failed to update existing bundle, rebuilding...', e.stack || e.message);
             return bundleFromScratch();
           });
-          return bundlePromise;
+          return this._reportBundlePromise(options, bundlePromise);
         } else {
           debug('Using cached bundle');
           return bundle;
@@ -652,7 +676,7 @@ class Server {
       });
     }
 
-    return bundleFromScratch();
+    return this._reportBundlePromise(options, bundleFromScratch());
   }
 
   processRequest(
@@ -691,10 +715,6 @@ class Server {
     }
 
     const options = this._getOptionsFromUrl(req.url);
-    this._reporter.update({
-      type: 'bundle_requested',
-      entryFilePath: options.entryFile,
-    });
     const requestingBundleLogEntry =
       log(createActionStartEntry({
         action_name: 'Requesting bundle',
@@ -724,10 +744,6 @@ class Server {
     const building = this._useCachedOrUpdateOrCreateBundle(options);
     building.then(
       p => {
-        this._reporter.update({
-          type: 'bundle_built',
-          entryFilePath: options.entryFile,
-        });
         if (requestType === 'bundle') {
           debug('Generating source code');
           const bundleSource = p.getSource({

packages/metro-bundler/src/lib/TerminalReporter.js

@@ -27,7 +27,6 @@ type BundleProgress = {
   transformedFileCount: number,
   totalFileCount: number,
   ratio: number,
-  outdatedModuleCount: number,
 };
 
 const DARK_BLOCK_CHAR = '\u2593';
@@ -48,6 +47,8 @@ export type TerminalReportableEvent = ReportableEvent | {
   totalFileCount: number,
 };
 
+type BuildPhase = 'in_progress' | 'done' | 'failed';
+
 /**
  * We try to print useful information to the terminal for interactive builds.
  * This implements the `Reporter` interface from the './reporting' module.
@@ -77,49 +78,26 @@ class TerminalReporter {
   }
 
   /**
-   * Return a message looking like this:
+   * Construct a message that represents the progress of a
+   * single bundle build, for example:
    *
-   *     Transforming files  |####         | 34.2% (324/945)...
-   *
-   */
-  _getFileTransformMessage(
-    {totalFileCount, transformedFileCount, ratio, outdatedModuleCount}: BundleProgress,
-    build: 'in_progress' | 'done',
-  ): string {
-    if (outdatedModuleCount > 0) {
-      const plural = outdatedModuleCount > 1;
-      return `Updating ${outdatedModuleCount} ` +
-        `module${plural ? 's' : ''} in place` +
-        (build === 'done' ? ', done' : '...');
-    }
-    if (totalFileCount === 0) {
-      return build === 'done'
-        ? 'No module changed.'
-        : 'Analysing...';
-    }
-    return util.format(
-      'Transforming modules  %s%s% (%s/%s)%s',
-      build === 'done' ? '' : getProgressBar(ratio, 30) + '  ',
-      (100 * ratio).toFixed(1),
-      transformedFileCount,
-      totalFileCount,
-      build === 'done' ? ', done.' : '...',
-    );
-  }
-
-  /**
-   * Construct a message that represent the progress of a single bundle build.
+   *     Bunding `foo.js`  |####         | 34.2% (324/945)
    */
   _getBundleStatusMessage(
     entryFilePath: string,
-    progress: BundleProgress,
-    build: 'in_progress' | 'done',
+    {totalFileCount, transformedFileCount, ratio}: BundleProgress,
+    phase: BuildPhase,
   ): string {
     const localPath = path.relative('.', entryFilePath);
-    return [
-      `Bundling \`${localPath}\``,
-      '  ' + this._getFileTransformMessage(progress, build),
-    ].join('\n');
+    return util.format(
+      'Bundling `%s`  %s%s% (%s/%s)%s',
+      localPath,
+      phase === 'in_progress' ? getProgressBar(ratio, 16) + '  ' : '',
+      (100 * ratio).toFixed(1),
+      transformedFileCount,
+      totalFileCount,
+      phase === 'done' ? ', done.' : (phase === 'failed' ? ', failed.' : ''),
+    );
   }
 
   _logCacheDisabled(reason: GlobalCacheDisabledReason): void {
@@ -134,19 +112,38 @@ class TerminalReporter {
     }
   }
 
+  _logBundleBuildDone(entryFilePath: string) {
+    const progress = this._activeBundles.get(entryFilePath);
+    if (progress != null) {
+      const msg = this._getBundleStatusMessage(entryFilePath, {
+        ...progress,
+        ratio: 1,
+        transformedFileCount: progress.totalFileCount,
+      }, 'done');
+      terminal.log(msg);
+    }
+  }
+
+  _logBundleBuildFailed(entryFilePath: string, error: Error) {
+    reporting.logError(terminal, 'bundling: %s', error.stack);
+    const progress = this._activeBundles.get(entryFilePath);
+    if (progress != null) {
+      const msg = this._getBundleStatusMessage(entryFilePath, progress, 'failed');
+      terminal.log(msg);
+    }
+  }
+
   /**
    * This function is only concerned with logging and should not do state
    * or terminal status updates.
    */
   _log(event: TerminalReportableEvent): void {
     switch (event.type) {
-      case 'bundle_built':
-        const progress = this._activeBundles.get(event.entryFilePath);
-        if (progress != null) {
-          terminal.log(
-            this._getBundleStatusMessage(event.entryFilePath, progress, 'done'),
-          );
-        }
+      case 'bundle_build_done':
+        this._logBundleBuildDone(event.entryFilePath);
+        break;
+      case 'bundle_build_failed':
+        this._logBundleBuildFailed(event.entryFilePath, event.error);
         break;
       case 'dep_graph_loaded':
         terminal.log(`${DEP_GRAPH_MESSAGE}, done.`);
@@ -187,35 +184,24 @@ class TerminalReporter {
       ratio,
       transformedFileCount,
       totalFileCount,
-      outdatedModuleCount: 0,
     });
   }
 
-  _updateBundleOutdatedModuleCount(
-    {entryFilePath, outdatedModuleCount}: {
-      entryFilePath: string,
-      outdatedModuleCount: number,
-    },
-  ) {
-    const currentProgress = this._activeBundles.get(entryFilePath);
-    if (currentProgress == null) {
-      return;
-    }
-    currentProgress.outdatedModuleCount = outdatedModuleCount;
-  }
-
   /**
    * This function is exclusively concerned with updating the internal state.
    * No logging or status updates should be done at this point.
    */
   _updateState(event: TerminalReportableEvent): void {
     switch (event.type) {
-      case 'bundle_requested':
+      case 'bundle_build_done':
+      case 'bundle_build_failed':
+        this._activeBundles.delete(event.entryFilePath);
+        break;
+      case 'bundle_build_started':
         this._activeBundles.set(event.entryFilePath, {
           transformedFileCount: 0,
-          totalFileCount: 0,
+          totalFileCount: 1,
           ratio: 0,
-          outdatedModuleCount: 0,
         });
         break;
       case 'bundle_transform_progressed':
@@ -229,12 +215,6 @@ class TerminalReporter {
       case 'bundle_transform_progressed_throttled':
         this._updateBundleProgress(event);
         break;
-      case 'bundle_update_existing':
-        this._updateBundleOutdatedModuleCount(event);
-        break;
-      case 'bundle_built':
-        this._activeBundles.delete(event.entryFilePath);
-        break;
       case 'dep_graph_loading':
         this._dependencyGraphHasLoaded = false;
         break;

packages/metro-bundler/src/lib/reporting.js

@@ -23,24 +23,24 @@ export type GlobalCacheDisabledReason = 'too_many_errors' | 'too_many_misses';
  * report to the tool user.
  */
 export type ReportableEvent = {
+  entryFilePath: string,
+  type: 'bundle_build_done',
+} | {
+  entryFilePath: string,
+  error: Error,
+  type: 'bundle_build_failed',
+} | {
+  entryFilePath: string,
+  type: 'bundle_build_started',
+} | {
   type: 'dep_graph_loading',
 } | {
   type: 'dep_graph_loaded',
-} | {
-  type: 'bundle_requested',
-  entryFilePath: string,
 } | {
   type: 'bundle_transform_progressed',
   entryFilePath: string,
   transformedFileCount: number,
   totalFileCount: number,
-} | {
-  entryFilePath: string,
-  outdatedModuleCount: number,
-  type: 'bundle_update_existing',
-} | {
-  type: 'bundle_built',
-  entryFilePath: string,
 } | {
   type: 'global_cache_error',
   error: Error,