Fix incremental builds when minify=true

Reviewed By: jeanlauliac

Differential Revision: D7615597

fbshipit-source-id: ab0985ed1954d8916e2f9c89c88465be196edd5f

packages/metro/src/Server.js

@@ -263,7 +263,11 @@ class Server {
   }
 
   async build(options: BundleOptions): Promise<{code: string, map: string}> {
-    const {prepend, graph} = await this._buildGraph(options);
+    let graphInfo = await this._buildGraph(options);
+
+    if (options.minify) {
+      graphInfo = await this._minifyGraph(graphInfo);
+    }
 
     const entryPoint = getAbsolutePath(
       options.entryFile,
@@ -271,7 +275,7 @@ class Server {
     );
 
     return {
-      code: plainJSBundle(entryPoint, prepend, graph, {
+      code: plainJSBundle(entryPoint, graphInfo.prepend, graphInfo.graph, {
         createModuleId: this._opts.createModuleId,
         getRunModuleStatement: this._opts.getRunModuleStatement,
         dev: options.dev,
@@ -279,7 +283,7 @@ class Server {
         runModule: options.runModule,
         sourceMapUrl: options.sourceMapUrl,
       }),
-      map: sourceMapString(prepend, graph, {
+      map: sourceMapString(graphInfo.prepend, graphInfo.graph, {
         excludeSource: options.excludeSource,
       }),
     };
@@ -302,31 +306,37 @@ class Server {
   }
 
   async getRamBundleInfo(options: BundleOptions): Promise<RamBundleInfo> {
-    const {prepend, graph} = await this._buildGraph(options);
+    let graphInfo = await this._buildGraph(options);
+
+    if (options.minify) {
+      graphInfo = await this._minifyGraph(graphInfo);
+    }
 
     const entryPoint = getAbsolutePath(
       options.entryFile,
       this._opts.projectRoots,
     );
 
-    return await getRamBundleInfo(entryPoint, prepend, graph, {
-      createModuleId: this._opts.createModuleId,
-      dev: options.dev,
-      excludeSource: options.excludeSource,
-      getRunModuleStatement: this._opts.getRunModuleStatement,
-      getTransformOptions: this._opts.getTransformOptions,
-      platform: options.platform,
-      runBeforeMainModule: options.runBeforeMainModule,
-      runModule: options.runModule,
-      sourceMapUrl: options.sourceMapUrl,
-    });
+    return await getRamBundleInfo(
+      entryPoint,
+      graphInfo.prepend,
+      graphInfo.graph,
+      {
+        createModuleId: this._opts.createModuleId,
+        dev: options.dev,
+        excludeSource: options.excludeSource,
+        getRunModuleStatement: this._opts.getRunModuleStatement,
+        getTransformOptions: this._opts.getTransformOptions,
+        platform: options.platform,
+        runBeforeMainModule: options.runBeforeMainModule,
+        runModule: options.runModule,
+        sourceMapUrl: options.sourceMapUrl,
+      },
+    );
   }
 
   async getAssets(options: BundleOptions): Promise<$ReadOnlyArray<AssetData>> {
-    const {graph} = await this._buildGraph({
-      ...options,
-      minify: false, // minification does not affect the assets.
-    });
+    const {graph} = await this._buildGraph(options);
 
     return await getAssets(graph, {
       assetPlugins: options.assetPlugins,
@@ -344,7 +354,6 @@ class Server {
     options = {
       ...Server.DEFAULT_BUNDLE_OPTIONS,
       ...options,
-      minify: false, // minification does not affect the dependencies.
       bundleType: 'bundle',
     };
 
@@ -375,21 +384,13 @@ class Server {
       type: 'module',
     };
 
-    let graph = await this._deltaBundler.buildGraph(crawlingOptions);
-    let prepend = await getPrependedScripts(
+    const graph = await this._deltaBundler.buildGraph(crawlingOptions);
+    const prepend = await getPrependedScripts(
       this._opts,
       crawlingOptions,
       this._deltaBundler,
     );
 
-    if (options.minify) {
-      prepend = await Promise.all(
-        prepend.map(script => this._minifyModule(script)),
-      );
-
-      graph = await mapGraph(graph, module => this._minifyModule(module));
-    }
-
     return {
       prepend,
       graph,
@@ -429,6 +430,10 @@ class Server {
       }
     }
 
+    if (options.minify) {
+      graphInfo = await this._minifyGraph(graphInfo);
+    }
+
     return {...graphInfo, numModifiedFiles};
   }
 
@@ -468,12 +473,39 @@ class Server {
       this._deltaGraphs.set(id, graphInfo);
     }
 
+    if (options.minify) {
+      // $FlowIssue #16581373 spread of an exact object should be exact
+      delta = {
+        ...delta,
+        modified: new Map(
+          await Promise.all(
+            Array.from(delta.modified).map(async ([path, module]) => [
+              path,
+              await this._minifyModule(module),
+            ]),
+          ),
+        ),
+      };
+    }
+
     return {
       ...graphInfo,
       delta,
     };
   }
 
+  async _minifyGraph(graphInfo: GraphInfo): Promise<GraphInfo> {
+    const prepend = await Promise.all(
+      graphInfo.prepend.map(script => this._minifyModule(script)),
+    );
+    const graph = await mapGraph(graphInfo.graph, module =>
+      this._minifyModule(module),
+    );
+
+    // $FlowIssue #16581373 spread of an exact object should be exact
+    return {...graphInfo, prepend, graph};
+  }
+
   async _minifyModule(module: DependencyEdge): Promise<DependencyEdge> {
     const {code, map} = await this._bundler.minifyModule(
       module.path,

packages/metro/src/Server/__tests__/Server-test.js

@@ -120,11 +120,28 @@ describe('processRequest', () => {
       ],
     ]);
 
-    DeltaBundler.prototype.buildGraph.mockReturnValue(
-      Promise.resolve({
+    const currentGraphs = new Set();
+    DeltaBundler.prototype.buildGraph.mockImplementation(async () => {
+      const graph = {
         entryPoints: ['/root/mybundle.js'],
         dependencies,
-      }),
+      };
+      currentGraphs.add(graph);
+
+      return graph;
+    });
+    DeltaBundler.prototype.getDelta.mockImplementation(
+      async (graph, {reset}) => {
+        if (!currentGraphs.has(graph)) {
+          throw new Error('Graph not found');
+        }
+
+        return {
+          modified: reset ? dependencies : new Map(),
+          deleted: new Set(),
+          reset,
+        };
+      },
     );
 
     getPrependedScripts.mockReturnValue(
@@ -142,14 +159,6 @@ describe('processRequest', () => {
       ]),
     );
 
-    DeltaBundler.prototype.getDelta.mockImplementation((options, {reset}) =>
-      Promise.resolve({
-        modified: reset ? dependencies : new Map(),
-        deleted: new Set(),
-        reset,
-      }),
-    );
-
     Bundler.prototype.getDependencyGraph = jest.fn().mockReturnValue(
       Promise.resolve({
         getHasteMap: jest.fn().mockReturnValue({on: jest.fn()}),
@@ -157,6 +166,13 @@ describe('processRequest', () => {
       }),
     );
 
+    Bundler.prototype.minifyModule = jest.fn().mockReturnValue(
+      Promise.resolve({
+        code: '__d(function(){minified();});',
+        map: [],
+      }),
+    );
+
     server = new Server(options);
     requestHandler = server.processRequest.bind(server);
 
@@ -283,6 +299,28 @@ describe('processRequest', () => {
     });
   });
 
+  it('calculates an incremental minified bundle', async () => {
+    await makeRequest(
+      requestHandler,
+      'mybundle.bundle?runModule=true&minify=true',
+    );
+
+    const response = await makeRequest(
+      requestHandler,
+      'mybundle.bundle?runModule=true&minify=true',
+    );
+    expect(response.statusCode).toEqual(200);
+    expect(response.body).toEqual(
+      [
+        '__d(function(){minified();});',
+        '__d(function(){minified();},0,[1],"mybundle.js");',
+        '__d(function(){minified();},1,[],"foo.js");',
+        'require(0);',
+        '//# sourceMappingURL=http://localhost:8081/mybundle.map?runModule=true&minify=true',
+      ].join('\n'),
+    );
+  });
+
   it('returns sourcemap on request of *.map', async () => {
     const response = await makeRequest(requestHandler, 'mybundle.map');
 
@@ -389,10 +427,15 @@ describe('processRequest', () => {
   it('does not rebuild the bundle when making concurrent requests', async () => {
     let resolveBuildGraph;
 
-    // force the buildGraph
+    // Delay the response of the buildGraph method.
     DeltaBundler.prototype.buildGraph.mockImplementation(async () => {
       return new Promise(res => (resolveBuildGraph = res));
     });
+    DeltaBundler.prototype.getDelta.mockReturnValue({
+      modified: new Map(),
+      deleted: new Set(),
+      reset: false,
+    });
 
     const promise1 = makeRequest(requestHandler, 'index.bundle');
     const promise2 = makeRequest(requestHandler, 'index.bundle');
@@ -550,6 +593,13 @@ describe('processRequest', () => {
       DeltaBundler.prototype.buildGraph.mockImplementation(async () => {
         return new Promise(res => (resolveBuildGraph = res));
       });
+      DeltaBundler.prototype.getDelta.mockImplementation(
+        async (graph, {reset}) => ({
+          modified: reset ? dependencies : new Map(),
+          deleted: new Set(),
+          reset,
+        }),
+      );
 
       const promise1 = makeRequest(requestHandler, 'index.delta');
       const promise2 = makeRequest(requestHandler, 'index.delta');
@@ -572,6 +622,29 @@ describe('processRequest', () => {
         reset: true,
       });
     });
+
+    it('should generate a minified delta correctly', async () => {
+      const response = await makeRequest(
+        requestHandler,
+        'index.delta?platform=ios&minify=true',
+      );
+
+      expect(JSON.parse(response.body)).toEqual({
+        id: 'XXXXX-0',
+        pre: [[-1, 'function () {require();}']],
+        delta: [
+          [0, '__d(function(){minified();},0,[1],"mybundle.js");'],
+          [1, '__d(function(){minified();},1,[],"foo.js");'],
+        ],
+        post: [
+          [
+            2,
+            '//# sourceMappingURL=http://localhost:8081/index.map?platform=ios&minify=true',
+          ],
+        ],
+        reset: true,
+      });
+    });
   });
 
   describe('/onchange endpoint', () => {