metro-bundler: use buildID instead of entry path for reporting

Summary: This fixes a longstanding bug that happens when 2 bundles with the same entry path but different options (dev, minify, etc.) get mixed up in the reporting. To prevent that we just use a unique build ID for each bundle that the Server handles separately.

Reviewed By: cpojer

Differential Revision: D5147049

fbshipit-source-id: da5c9cfe8c6a5d888b5be737947800d213081d86

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

@@ -154,6 +154,7 @@ class Server {
   _reporter: Reporter;
   _symbolicateInWorker: Symbolicate;
   _platforms: Set<string>;
+  _nextBundleBuildID: number;
 
   constructor(options: Options) {
     this._opts = {
@@ -239,6 +240,7 @@ class Server {
     }, 50);
 
     this._symbolicateInWorker = symbolicate.createWorker();
+    this._nextBundleBuildID = 1;
   }
 
   end(): mixed {
@@ -491,29 +493,34 @@ class Server {
    * and for when we build for scratch.
    */
   _reportBundlePromise(
+    buildID: string,
     options: {entryFile: string},
     bundlePromise: Promise<Bundle>,
   ): Promise<Bundle> {
     this._reporter.update({
+      buildID,
       entryFilePath: options.entryFile,
       type: 'bundle_build_started',
     });
     return bundlePromise.then(bundle => {
       this._reporter.update({
-        entryFilePath: options.entryFile,
+        buildID,
         type: 'bundle_build_done',
       });
       return bundle;
     }, error => {
       this._reporter.update({
-        entryFilePath: options.entryFile,
+        buildID,
         type: 'bundle_build_failed',
       });
       return Promise.reject(error);
     });
   }
 
-  useCachedOrUpdateOrCreateBundle(options: BundleOptions): Promise<Bundle> {
+  useCachedOrUpdateOrCreateBundle(
+    buildID: string,
+    options: BundleOptions,
+  ): Promise<Bundle> {
     const optionsJson = this.optionsHash(options);
     const bundleFromScratch = () => {
       const building = this.buildBundle(options);
@@ -606,7 +613,7 @@ class Server {
               debug('Failed to update existing bundle, rebuilding...', e.stack || e.message);
               return bundleFromScratch();
             });
-          return this._reportBundlePromise(options, bundlePromise);
+          return this._reportBundlePromise(buildID, options, bundlePromise);
         } else {
           debug('Using cached bundle');
           return bundle;
@@ -614,7 +621,7 @@ class Server {
       });
     }
 
-    return this._reportBundlePromise(options, bundleFromScratch());
+    return this._reportBundlePromise(buildID, options, bundleFromScratch());
   }
 
   processRequest(
@@ -660,12 +667,13 @@ class Server {
         entry_point: options.entryFile,
       }));
 
+    const buildID = this.getNewBuildID();
     let reportProgress = emptyFunction;
     if (!this._opts.silent) {
       reportProgress = (transformedFileCount, totalFileCount) => {
         this._reporter.update({
+          buildID,
           type: 'bundle_transform_progressed',
-          entryFilePath: options.entryFile,
           transformedFileCount,
           totalFileCount,
         });
@@ -679,7 +687,7 @@ class Server {
     };
 
     debug('Getting bundle for request');
-    const building = this.useCachedOrUpdateOrCreateBundle(options);
+    const building = this.useCachedOrUpdateOrCreateBundle(buildID, options);
     building.then(
       p => {
         if (requestType === 'bundle') {
@@ -780,7 +788,12 @@ class Server {
 
   _sourceMapForURL(reqUrl: string): Promise<SourceMap> {
     const options = this._getOptionsFromUrl(reqUrl);
-    const building = this.useCachedOrUpdateOrCreateBundle(options);
+    // We're not properly reporting progress here. Reporting should be done
+    // from within that function.
+    const building = this.useCachedOrUpdateOrCreateBundle(
+      this.getNewBuildID(),
+      options,
+    );
     return building.then(p => p.getSourceMap({
       minify: options.minify,
       dev: options.dev,
@@ -898,6 +911,10 @@ class Server {
     return query[opt] === 'true' || query[opt] === '1';
   }
 
+  getNewBuildID(): string {
+    return (this._nextBundleBuildID++).toString(36);
+  }
+
   static DEFAULT_BUNDLE_OPTIONS;
 
 }

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

@@ -26,6 +26,7 @@ const GLOBAL_CACHE_DISABLED_MESSAGE_FORMAT =
   'The global cache is now disabled because %s';
 
 type BundleProgress = {
+  entryFilePath: string,
   transformedFileCount: number,
   totalFileCount: number,
   ratio: number,
@@ -43,8 +44,8 @@ function getProgressBar(ratio: number, length: number) {
 }
 
 export type TerminalReportableEvent = ReportableEvent | {
+  buildID: string,
   type: 'bundle_transform_progressed_throttled',
-  entryFilePath: string,
   transformedFileCount: number,
   totalFileCount: number,
 };
@@ -66,7 +67,7 @@ class TerminalReporter {
 
   _dependencyGraphHasLoaded: boolean;
   _scheduleUpdateBundleProgress: (data: {
-    entryFilePath: string,
+    buildID: string,
     transformedFileCount: number,
     totalFileCount: number,
   }) => void;
@@ -86,8 +87,7 @@ class TerminalReporter {
    *     Bunding `foo.js`  |####         | 34.2% (324/945)
    */
   _getBundleStatusMessage(
-    entryFilePath: string,
+    {entryFilePath, totalFileCount, transformedFileCount, ratio}: BundleProgress,
-    {totalFileCount, transformedFileCount, ratio}: BundleProgress,
     phase: BuildPhase,
   ): string {
     const localPath = path.relative('.', entryFilePath);
@@ -114,10 +114,10 @@ class TerminalReporter {
     }
   }
 
-  _logBundleBuildDone(entryFilePath: string) {
+  _logBundleBuildDone(buildID: string) {
-    const progress = this._activeBundles.get(entryFilePath);
+    const progress = this._activeBundles.get(buildID);
     if (progress != null) {
-      const msg = this._getBundleStatusMessage(entryFilePath, {
+      const msg = this._getBundleStatusMessage({
         ...progress,
         ratio: 1,
         transformedFileCount: progress.totalFileCount,
@@ -126,10 +126,10 @@ class TerminalReporter {
     }
   }
 
-  _logBundleBuildFailed(entryFilePath: string) {
+  _logBundleBuildFailed(buildID: string) {
-    const progress = this._activeBundles.get(entryFilePath);
+    const progress = this._activeBundles.get(buildID);
     if (progress != null) {
-      const msg = this._getBundleStatusMessage(entryFilePath, progress, 'failed');
+      const msg = this._getBundleStatusMessage(progress, 'failed');
       terminal.log(msg);
     }
   }
@@ -197,10 +197,10 @@ class TerminalReporter {
         this._logPackagerInitializingFailed(event.port, event.error);
         break;
       case 'bundle_build_done':
-        this._logBundleBuildDone(event.entryFilePath);
+        this._logBundleBuildDone(event.buildID);
         break;
       case 'bundle_build_failed':
-        this._logBundleBuildFailed(event.entryFilePath);
+        this._logBundleBuildFailed(event.buildID);
         break;
       case 'bundling_error':
         this._logBundlingError(event.error);
@@ -249,13 +249,13 @@ class TerminalReporter {
    * also prevent the ratio from going backwards.
    */
   _updateBundleProgress(
-    {entryFilePath, transformedFileCount, totalFileCount}: {
+    {buildID, transformedFileCount, totalFileCount}: {
-      entryFilePath: string,
+      buildID: string,
       transformedFileCount: number,
       totalFileCount: number,
     },
   ) {
-    const currentProgress = this._activeBundles.get(entryFilePath);
+    const currentProgress = this._activeBundles.get(buildID);
     if (currentProgress == null) {
       return;
     }
@@ -277,10 +277,11 @@ class TerminalReporter {
     switch (event.type) {
       case 'bundle_build_done':
       case 'bundle_build_failed':
-        this._activeBundles.delete(event.entryFilePath);
+        this._activeBundles.delete(event.buildID);
         break;
       case 'bundle_build_started':
-        this._activeBundles.set(event.entryFilePath, {
+        this._activeBundles.set(event.buildID, {
+          entryFilePath: event.entryFilePath,
           transformedFileCount: 0,
           totalFileCount: 1,
           ratio: 0,
@@ -322,8 +323,8 @@ class TerminalReporter {
     return [
       this._getDepGraphStatusMessage(),
     ].concat(Array.from(this._activeBundles.entries()).map(
-      ([entryFilePath, progress]) =>
+      ([_, progress]) =>
-        this._getBundleStatusMessage(entryFilePath, progress, 'in_progress'),
+        this._getBundleStatusMessage(progress, 'in_progress'),
     )).filter(str => str != null).join('\n');
   }
 

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

@@ -33,12 +33,13 @@ export type ReportableEvent = {
   port: number,
   error: Error,
 } | {
-  entryFilePath: string,
+  buildID: string,
   type: 'bundle_build_done',
 } | {
-  entryFilePath: string,
+  buildID: string,
   type: 'bundle_build_failed',
 } | {
+  buildID: string,
   entryFilePath: string,
   type: 'bundle_build_started',
 } | {
@@ -49,8 +50,8 @@ export type ReportableEvent = {
 } | {
   type: 'dep_graph_loaded',
 } | {
+  buildID: string,
   type: 'bundle_transform_progressed',
-  entryFilePath: string,
   transformedFileCount: number,
   totalFileCount: number,
 } | {