Support concurrent bundle requests on the Server

Reviewed By: mjesun

Differential Revision: D7320670

fbshipit-source-id: 02090f85bf1b5376e9af879d17898bf31c31ce3a

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

@@ -32,6 +32,7 @@ describe('processRequest', () => {
   let Bundler;
   let Server;
   let crypto;
+  let dependencies;
   let getAsset;
   let getPrependedScripts;
   let symbolicate;
@@ -91,35 +92,37 @@ describe('processRequest', () => {
   let requestHandler;
 
   beforeEach(() => {
+    dependencies = new Map([
+      [
+        '/root/mybundle.js',
+        {
+          path: '/root/mybundle.js',
+          dependencies: new Map([['foo', '/root/foo.js']]),
+          output: {
+            code: '__d(function() {entry();});',
+            map: [],
+            source: 'code-mybundle',
+          },
+        },
+      ],
+      [
+        '/root/foo.js',
+        {
+          path: '/root/foo.js',
+          dependencies: new Map(),
+          output: {
+            code: '__d(function() {foo();});',
+            map: [],
+            source: 'code-foo',
+          },
+        },
+      ],
+    ]);
+
     DeltaBundler.prototype.buildGraph.mockReturnValue(
       Promise.resolve({
         entryPoints: ['/root/mybundle.js'],
-        dependencies: new Map([
-          [
-            '/root/mybundle.js',
-            {
-              path: '/root/mybundle.js',
-              dependencies: new Map([['foo', '/root/foo.js']]),
-              output: {
-                code: '__d(function() {entry();});',
-                map: [],
-                source: 'code-mybundle',
-              },
-            },
-          ],
-          [
-            '/root/foo.js',
-            {
-              path: '/root/foo.js',
-              dependencies: new Map(),
-              output: {
-                code: '__d(function() {foo();});',
-                map: [],
-                source: 'code-foo',
-              },
-            },
-          ],
-        ]),
+        dependencies,
       }),
     );
 
@@ -138,6 +141,14 @@ 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()}),
@@ -221,14 +232,6 @@ describe('processRequest', () => {
     );
     const lastModified = response.headers['Last-Modified'];
 
-    DeltaBundler.prototype.getDelta.mockReturnValue(
-      Promise.resolve({
-        modified: new Map(),
-        deleted: new Set(),
-        reset: false,
-      }),
-    );
-
     global.Date = class {
       constructor() {
         return new NativeDate('2017-07-07T00:10:20.000Z');
@@ -382,6 +385,31 @@ describe('processRequest', () => {
     });
   });
 
+  it('does not rebuild the bundle when making concurrent requests', async () => {
+    let resolveBuildGraph;
+
+    // force the buildGraph
+    DeltaBundler.prototype.buildGraph.mockImplementation(async () => {
+      return new Promise(res => (resolveBuildGraph = res));
+    });
+
+    const promise1 = makeRequest(requestHandler, 'index.bundle');
+    const promise2 = makeRequest(requestHandler, 'index.bundle');
+
+    resolveBuildGraph({
+      entryPoints: ['/root/mybundle.js'],
+      dependencies,
+    });
+
+    const [result1, result2] = await Promise.all([promise1, promise2]);
+    expect(result1.body).toEqual(result2.body);
+    expect(result1.getHeader('X-Metro-Files-Changed-Count')).toEqual('3');
+    expect(result2.getHeader('X-Metro-Files-Changed-Count')).toEqual('0');
+
+    expect(DeltaBundler.prototype.buildGraph.mock.calls.length).toBe(1);
+    expect(DeltaBundler.prototype.getDelta.mock.calls.length).toBe(1);
+  });
+
   describe('Generate delta bundle endpoint', () => {
     it('should generate the initial delta correctly', async () => {
       const response = await makeRequest(
@@ -509,6 +537,36 @@ describe('processRequest', () => {
         },
       );
     });
+
+    it('does return the same initial delta when making concurrent requests', async () => {
+      let resolveBuildGraph;
+
+      // force the buildGraph
+      DeltaBundler.prototype.buildGraph.mockImplementation(async () => {
+        return new Promise(res => (resolveBuildGraph = res));
+      });
+
+      const promise1 = makeRequest(requestHandler, 'index.delta');
+      const promise2 = makeRequest(requestHandler, 'index.delta');
+
+      resolveBuildGraph({
+        entryPoints: ['/root/mybundle.js'],
+        dependencies,
+      });
+
+      const [result1, result2] = await Promise.all([promise1, promise2]);
+      const {id: id1, ...delta1} = JSON.parse(result1.body);
+      const {id: id2, ...delta2} = JSON.parse(result2.body);
+      expect(delta1).toEqual(delta2);
+      expect(id1).toEqual('XXXXX-0');
+      expect(id2).toEqual('XXXXX-1');
+
+      expect(DeltaBundler.prototype.buildGraph.mock.calls.length).toBe(1);
+      expect(DeltaBundler.prototype.getDelta.mock.calls.length).toBe(1);
+      expect(DeltaBundler.prototype.getDelta.mock.calls[0][1]).toEqual({
+        reset: true,
+      });
+    });
   });
 
   describe('/onchange endpoint', () => {

packages/metro/src/Server/index.js

@@ -131,8 +131,8 @@ class Server {
   _platforms: Set<string>;
   _nextBundleBuildID: number;
   _deltaBundler: DeltaBundler;
-  _graphs: Map<string, GraphInfo> = new Map();
-  _deltaGraphs: Map<string, GraphInfo> = new Map();
+  _graphs: Map<string, Promise<GraphInfo>> = new Map();
+  _deltaGraphs: Map<string, Promise<GraphInfo>> = new Map();
 
   constructor(options: Options) {
     const reporter =
@@ -380,19 +380,26 @@ class Server {
     {rebuild}: {rebuild: boolean},
   ): Promise<{...GraphInfo, numModifiedFiles: number}> {
     const id = this._optionsHash(options);
-    let graphInfo = this._graphs.get(id);
+    let graphPromise = this._graphs.get(id);
+    let graphInfo: GraphInfo;
     let numModifiedFiles = 0;
 
-    if (!graphInfo) {
-      graphInfo = await this._buildGraph(options);
-      this._graphs.set(id, graphInfo);
+    if (!graphPromise) {
+      graphPromise = this._buildGraph(options);
+      this._graphs.set(id, graphPromise);
+
+      graphInfo = await graphPromise;
       numModifiedFiles =
         graphInfo.prepend.length + graphInfo.graph.dependencies.size;
-    } else if (rebuild) {
-      const delta = await this._deltaBundler.getDelta(graphInfo.graph, {
-        reset: false,
-      });
-      numModifiedFiles = delta.modified.size;
+    } else {
+      graphInfo = await graphPromise;
+
+      if (rebuild) {
+        const delta = await this._deltaBundler.getDelta(graphInfo.graph, {
+          reset: false,
+        });
+        numModifiedFiles = delta.modified.size;
+      }
 
       if (numModifiedFiles > 0) {
         graphInfo.lastModified = new Date();
@@ -406,12 +413,15 @@ class Server {
     options: DeltaOptions,
   ): Promise<{...GraphInfo, delta: Delta}> {
     const id = this._optionsHash(options);
-    let graphInfo = this._deltaGraphs.get(id);
+    let graphPromise = this._deltaGraphs.get(id);
+    let graphInfo;
 
     let delta;
 
-    if (!graphInfo) {
-      graphInfo = await this._buildGraph(options);
+    if (!graphPromise) {
+      graphPromise = this._buildGraph(options);
+      this._deltaGraphs.set(id, graphPromise);
+      graphInfo = await graphPromise;
 
       delta = {
         modified: graphInfo.graph.dependencies,
@@ -419,6 +429,8 @@ class Server {
         reset: true,
       };
     } else {
+      graphInfo = await graphPromise;
+
       delta = await this._deltaBundler.getDelta(graphInfo.graph, {
         reset: graphInfo.sequenceId !== options.deltaBundleId,
       });
@@ -429,9 +441,9 @@ class Server {
         ...graphInfo,
         sequenceId: crypto.randomBytes(8).toString('hex'),
       };
-    }
 
-    this._deltaGraphs.set(id, graphInfo);
+      this._deltaGraphs.set(id, graphInfo);
+    }
 
     return {
       ...graphInfo,