Start using the Graph object in the traverse dependencies logic

Reviewed By: davidaurelio

Differential Revision: D7180698

fbshipit-source-id: a7c39b73a7673f450e3a147e7feff4cfcc3061bf

packages/metro/src/DeltaBundler/DeltaCalculator.js

@@ -21,7 +21,7 @@ import type Bundler from '../Bundler';
 import type {Options as JSTransformerOptions} from '../JSTransformer/worker';
 import type DependencyGraph from '../node-haste/DependencyGraph';
 import type {BundleOptions} from '../shared/types.flow';
-import type {DependencyEdge, DependencyEdges} from './traverseDependencies';
+import type {DependencyEdge, Graph} from './traverseDependencies';
 
 export type DeltaResult = {|
   +modified: Map<string, DependencyEdge>,
@@ -29,10 +29,7 @@ export type DeltaResult = {|
   +reset: boolean,
 |};
 
-export type Graph = {|
+export type {Graph} from './traverseDependencies';
-  dependencies: DependencyEdges,
-  entryFile: string,
-|};
 
 /**
  * This class is in charge of calculating the delta of changed modules that
@@ -203,10 +200,12 @@ class DeltaCalculator extends EventEmitter {
       {dev: this._options.dev, platform: this._options.platform},
       async path => {
         const {added} = await initialTraverseDependencies(
-          path,
+          {
+            dependencies: new Map(),
+            entryFile: path,
+          },
           this._dependencyGraph,
           transformOptionsForBlacklist,
-          new Map(),
         );
 
         return Array.from(added.keys());
@@ -266,10 +265,9 @@ class DeltaCalculator extends EventEmitter {
 
     if (!this._graph.dependencies.size) {
       const {added} = await initialTraverseDependencies(
-        this._graph.entryFile,
+        this._graph,
         this._dependencyGraph,
         transformerOptions,
-        this._graph.dependencies,
         this._options.onProgress || undefined,
       );
 
@@ -304,7 +302,7 @@ class DeltaCalculator extends EventEmitter {
       modifiedDependencies,
       this._dependencyGraph,
       transformerOptions,
-      this._graph.dependencies,
+      this._graph,
       this._options.onProgress || undefined,
     );
 

packages/metro/src/DeltaBundler/__tests__/DeltaCalculator-test.js

@@ -91,7 +91,7 @@ describe('DeltaCalculator', () => {
     };
 
     initialTraverseDependencies.mockImplementationOnce(
-      async (path, dg, opt, edges) => {
+      async (graph, dg, opt) => {
         edgeModule = {
           ...entryModule,
           dependencies: new Map([
@@ -116,10 +116,10 @@ describe('DeltaCalculator', () => {
           inverseDependencies: ['/bundle'],
         };
 
-        edges.set('/bundle', edgeModule);
+        graph.dependencies.set('/bundle', edgeModule);
-        edges.set('/foo', edgeFoo);
+        graph.dependencies.set('/foo', edgeFoo);
-        edges.set('/bar', edgeBar);
+        graph.dependencies.set('/bar', edgeBar);
-        edges.set('/baz', edgeBaz);
+        graph.dependencies.set('/baz', edgeBaz);
 
         return {
           added: new Map([
@@ -274,8 +274,8 @@ describe('DeltaCalculator', () => {
 
     mockedDependencies.push(moduleQux);
 
-    traverseDependencies.mockImplementation(async (path, dg, opt, edges) => {
+    traverseDependencies.mockImplementation(async (path, dg, opt, graph) => {
-      edges.set('/qux', edgeQux);
+      graph.dependencies.set('/qux', edgeQux);
 
       return {
         added: new Map([['/foo', edgeFoo], ['/qux', edgeQux]]),

packages/metro/src/DeltaBundler/__tests__/__snapshots__/traverseDependencies-test.js.snap

@@ -1,61 +1,64 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`should do the initial traversal correctly 1`] = `
-Map {
+Object {
-  "/bundle" => Object {
+  "dependencies": Map {
-    "dependencies": Map {
+    "/bundle" => Object {
-      "foo" => "/foo",
+      "dependencies": Map {
+        "foo" => "/foo",
+      },
+      "inverseDependencies": Set {},
+      "output": Object {
+        "code": "// code",
+        "map": Array [],
+        "source": "// source",
+        "type": "module",
+      },
+      "path": "/bundle",
     },
-    "inverseDependencies": Set {},
+    "/foo" => Object {
-    "output": Object {
+      "dependencies": Map {
-      "code": "// code",
+        "bar" => "/bar",
-      "map": Array [],
+        "baz" => "/baz",
-      "source": "// source",
+      },
-      "type": "module",
+      "inverseDependencies": Set {
+        "/bundle",
+      },
+      "output": Object {
+        "code": "// code",
+        "map": Array [],
+        "source": "// source",
+        "type": "module",
+      },
+      "path": "/foo",
+    },
+    "/bar" => Object {
+      "dependencies": Map {},
+      "inverseDependencies": Set {
+        "/foo",
+      },
+      "output": Object {
+        "code": "// code",
+        "map": Array [],
+        "source": "// source",
+        "type": "module",
+      },
+      "path": "/bar",
+    },
+    "/baz" => Object {
+      "dependencies": Map {},
+      "inverseDependencies": Set {
+        "/foo",
+      },
+      "output": Object {
+        "code": "// code",
+        "map": Array [],
+        "source": "// source",
+        "type": "module",
+      },
+      "path": "/baz",
     },
-    "path": "/bundle",
-  },
-  "/foo" => Object {
-    "dependencies": Map {
-      "bar" => "/bar",
-      "baz" => "/baz",
-    },
-    "inverseDependencies": Set {
-      "/bundle",
-    },
-    "output": Object {
-      "code": "// code",
-      "map": Array [],
-      "source": "// source",
-      "type": "module",
-    },
-    "path": "/foo",
-  },
-  "/bar" => Object {
-    "dependencies": Map {},
-    "inverseDependencies": Set {
-      "/foo",
-    },
-    "output": Object {
-      "code": "// code",
-      "map": Array [],
-      "source": "// source",
-      "type": "module",
-    },
-    "path": "/bar",
-  },
-  "/baz" => Object {
-    "dependencies": Map {},
-    "inverseDependencies": Set {
-      "/foo",
-    },
-    "output": Object {
-      "code": "// code",
-      "map": Array [],
-      "source": "// source",
-      "type": "module",
-    },
-    "path": "/baz",
   },
+  "entryFile": "/bundle",
 }
 `;

packages/metro/src/DeltaBundler/__tests__/traverseDependencies-integration-test.js

@@ -60,17 +60,20 @@ describe('traverseDependencies', function() {
   ) {
     const dgraph = await dgraphPromise;
 
-    const edges = new Map();
+    const graph = {
+      dependencies: new Map(),
+      entryFile: entryPath,
+    };
+
     const {added} = await traverseDependencies.initialTraverseDependencies(
-      entryPath,
+      graph,
       dgraph,
       {...emptyTransformOptions, platform},
-      edges,
     );
 
     const dependencies = recursive
       ? [...added.values()].map(edge => edge.path)
-      : edges.get(entryPath).dependencies.values();
+      : graph.dependencies.get(entryPath).dependencies.values();
 
     return await Promise.all(
       [...dependencies].map(async path => {
@@ -5068,10 +5071,12 @@ describe('traverseDependencies', function() {
 
     function getDependencies() {
       return traverseDependencies.initialTraverseDependencies(
-        '/root/index.js',
+        {
+          dependencies: new Map(),
+          entryFile: '/root/index.js',
+        },
         dependencyGraph,
         emptyTransformOptions,
-        new Map(),
         onProgress,
       );
     }

packages/metro/src/DeltaBundler/__tests__/traverseDependencies-test.js

@@ -19,6 +19,7 @@ let dependencyGraph;
 let mockedDependencies;
 let mockedDependencyTree;
 let files = new Set();
+let graph;
 
 let entryModule;
 let moduleFoo;
@@ -168,32 +169,30 @@ beforeEach(async () => {
   Actions.addDependency('/foo', '/baz');
 
   files = new Set();
+
+  graph = {
+    dependencies: new Map(),
+    entryFile: '/bundle',
+  };
 });
 
 it('should do the initial traversal correctly', async () => {
-  const edges = new Map();
+  const result = await initialTraverseDependencies(graph, dependencyGraph, {});
-  const result = await initialTraverseDependencies(
-    '/bundle',
-    dependencyGraph,
-    {},
-    edges,
-  );
 
   expect(getPaths(result)).toEqual({
     added: new Set(['/bundle', '/foo', '/bar', '/baz']),
     deleted: new Set(),
   });
 
-  expect(edges).toMatchSnapshot();
+  expect(graph).toMatchSnapshot();
 });
 
 it('should return an empty result when there are no changes', async () => {
-  const edges = new Map();
+  await initialTraverseDependencies(graph, dependencyGraph, {});
-  await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
   expect(
     getPaths(
-      await traverseDependencies(['/bundle'], dependencyGraph, {}, edges),
+      await traverseDependencies(['/bundle'], dependencyGraph, {}, graph),
     ),
   ).toEqual({
     added: new Set(['/bundle']),
@@ -202,14 +201,13 @@ it('should return an empty result when there are no changes', async () => {
 });
 
 it('should return a removed dependency', async () => {
-  const edges = new Map();
+  await initialTraverseDependencies(graph, dependencyGraph, {});
-  await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
   Actions.removeDependency('/foo', '/bar');
 
   expect(
     getPaths(
-      await traverseDependencies([...files], dependencyGraph, {}, edges),
+      await traverseDependencies([...files], dependencyGraph, {}, graph),
     ),
   ).toEqual({
     added: new Set(['/foo']),
@@ -218,8 +216,7 @@ it('should return a removed dependency', async () => {
 });
 
 it('should return added/removed dependencies', async () => {
-  const edges = new Map();
+  await initialTraverseDependencies(graph, dependencyGraph, {});
-  await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
   Actions.addDependency('/foo', '/qux');
   Actions.removeDependency('/foo', '/bar');
@@ -227,7 +224,7 @@ it('should return added/removed dependencies', async () => {
 
   expect(
     getPaths(
-      await traverseDependencies([...files], dependencyGraph, {}, edges),
+      await traverseDependencies([...files], dependencyGraph, {}, graph),
     ),
   ).toEqual({
     added: new Set(['/foo', '/qux']),
@@ -236,8 +233,7 @@ it('should return added/removed dependencies', async () => {
 });
 
 it('should return added modules before the modified ones', async () => {
-  const edges = new Map();
+  await initialTraverseDependencies(graph, dependencyGraph, {});
-  await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
   Actions.addDependency('/foo', '/qux');
   Actions.modifyFile('/bar');
@@ -247,32 +243,30 @@ it('should return added modules before the modified ones', async () => {
   // it to an array.
   expect([
     ...getPaths(
-      await traverseDependencies([...files], dependencyGraph, {}, edges),
+      await traverseDependencies([...files], dependencyGraph, {}, graph),
     ).added,
   ]).toEqual(['/qux', '/foo', '/bar', '/baz']);
 });
 
 it('should retry to traverse the dependencies as it was after getting an error', async () => {
-  const edges = new Map();
+  await initialTraverseDependencies(graph, dependencyGraph, {});
-  await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
   Actions.deleteFile(moduleBar.path);
 
   await expect(
-    traverseDependencies(['/foo'], dependencyGraph, {}, edges),
+    traverseDependencies(['/foo'], dependencyGraph, {}, graph),
   ).rejects.toBeInstanceOf(Error);
 
   // Second time that the traversal of dependencies we still have to throw an
   // error (no matter if no file has been changed).
   await expect(
-    traverseDependencies(['/foo'], dependencyGraph, {}, edges),
+    traverseDependencies(['/foo'], dependencyGraph, {}, graph),
   ).rejects.toBeInstanceOf(Error);
 });
 
 describe('edge cases', () => {
   it('should handle renames correctly', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     Actions.removeDependency('/foo', '/baz');
     Actions.moveFile('/baz', '/qux');
@@ -280,7 +274,7 @@ describe('edge cases', () => {
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/foo', '/qux']),
@@ -289,8 +283,7 @@ describe('edge cases', () => {
   });
 
   it('should not try to remove wrong dependencies when renaming files', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     // Rename /foo to /foo-renamed, but keeping all its dependencies.
     Actions.addDependency('/bundle', '/foo-renamed');
@@ -302,7 +295,7 @@ describe('edge cases', () => {
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/bundle', '/foo-renamed']),
@@ -311,15 +304,14 @@ describe('edge cases', () => {
   });
 
   it('modify a file and delete it afterwards', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     Actions.modifyFile('/baz');
     Actions.removeDependency('/foo', '/baz');
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/foo']),
@@ -328,15 +320,14 @@ describe('edge cases', () => {
   });
 
   it('move a file to a different folder', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     Actions.addDependency('/foo', '/baz-moved');
     Actions.removeDependency('/foo', '/baz');
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/foo', '/baz-moved']),
@@ -345,21 +336,20 @@ describe('edge cases', () => {
   });
 
   it('maintain the order of module dependencies consistent', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     Actions.addDependency('/foo', '/qux', 0);
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/foo', '/qux']),
       deleted: new Set(),
     });
 
-    expect([...edges.get(moduleFoo.path).dependencies]).toEqual([
+    expect([...graph.dependencies.get(moduleFoo.path).dependencies]).toEqual([
       ['qux', '/qux'],
       ['bar', '/bar'],
       ['baz', '/baz'],
@@ -367,22 +357,21 @@ describe('edge cases', () => {
   });
 
   it('should create two entries when requiring the same file in different forms', async () => {
-    const edges = new Map();
+    await initialTraverseDependencies(graph, dependencyGraph, {});
-    await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges);
 
     // We're adding a new reference from bundle to foo.
     Actions.addDependency('/bundle', '/foo', 0, 'foo.js');
 
     expect(
       getPaths(
-        await traverseDependencies([...files], dependencyGraph, {}, edges),
+        await traverseDependencies([...files], dependencyGraph, {}, graph),
       ),
     ).toEqual({
       added: new Set(['/bundle']),
       deleted: new Set(),
     });
 
-    expect([...edges.get(entryModule.path).dependencies]).toEqual([
+    expect([...graph.dependencies.get(entryModule.path).dependencies]).toEqual([
       ['foo.js', '/foo'],
       ['foo', '/foo'],
     ]);
@@ -425,15 +414,15 @@ describe('edge cases', () => {
     }
 
     async function assertOrder() {
+      graph = {
+        dependencies: new Map(),
+        entryFile: '/bundle',
+      };
+
       expect(
         Array.from(
           getPaths(
-            await initialTraverseDependencies(
+            await initialTraverseDependencies(graph, dependencyGraph, {}),
-              '/bundle',
-              dependencyGraph,
-              {},
-              new Map(),
-            ),
           ).added,
         ),
       ).toEqual(['/bundle', '/foo', '/baz', '/bar']);
@@ -467,7 +456,6 @@ describe('edge cases', () => {
     jest.spyOn(moduleBar, 'read');
     jest.spyOn(moduleBaz, 'read');
 
-    const edges = new Map();
     const transformOptions = {
       inlineRequires: {
         blacklist: {
@@ -476,12 +464,7 @@ describe('edge cases', () => {
       },
     };
 
-    await initialTraverseDependencies(
+    await initialTraverseDependencies(graph, dependencyGraph, transformOptions);
-      '/bundle',
-      dependencyGraph,
-      transformOptions,
-      edges,
-    );
 
     expect(entryModule.read).toHaveBeenCalledWith({inlineRequires: true});
     expect(moduleFoo.read).toHaveBeenCalledWith({inlineRequires: true});
@@ -494,7 +477,7 @@ describe('edge cases', () => {
       ['/foo'],
       dependencyGraph,
       transformOptions,
-      edges,
+      graph,
     );
 
     expect(moduleFoo.read).toHaveBeenCalledWith({inlineRequires: true});

packages/metro/src/DeltaBundler/traverseDependencies.js

@@ -33,6 +33,11 @@ export type DependencyEdge = {|
 
 export type DependencyEdges = Map<string, DependencyEdge>;
 
+export type Graph = {|
+  dependencies: DependencyEdges,
+  entryFile: string,
+|};
+
 type Result = {added: Map<string, DependencyEdge>, deleted: Set<string>};
 
 /**
@@ -65,7 +70,7 @@ async function traverseDependencies(
   paths: Array<string>,
   dependencyGraph: DependencyGraph,
   transformOptions: JSTransformerOptions,
-  edges: DependencyEdges,
+  graph: Graph,
   onProgress?: (numProcessed: number, total: number) => mixed = () => {},
 ): Promise<Result> {
   const delta = {
@@ -76,7 +81,7 @@ async function traverseDependencies(
 
   await Promise.all(
     paths.map(async path => {
-      const edge = edges.get(path);
+      const edge = graph.dependencies.get(path);
 
       if (!edge) {
         return;
@@ -88,7 +93,7 @@ async function traverseDependencies(
         edge,
         dependencyGraph,
         transformOptions,
-        edges,
+        graph,
         delta,
         onProgress,
       );
@@ -129,13 +134,15 @@ async function traverseDependencies(
 }
 
 async function initialTraverseDependencies(
-  path: string,
+  graph: Graph,
   dependencyGraph: DependencyGraph,
   transformOptions: JSTransformerOptions,
-  edges: DependencyEdges,
   onProgress?: (numProcessed: number, total: number) => mixed = () => {},
 ): Promise<Result> {
-  const edge = createEdge(dependencyGraph.getModuleForPath(path), edges);
+  const edge = createEdge(
+    dependencyGraph.getModuleForPath(graph.entryFile),
+    graph,
+  );
 
   const delta = {
     added: new Map([[edge.path, edge]]),
@@ -147,7 +154,7 @@ async function initialTraverseDependencies(
     edge,
     dependencyGraph,
     transformOptions,
-    edges,
+    graph,
     delta,
     onProgress,
   );
@@ -162,7 +169,7 @@ async function traverseDependenciesForSingleFile(
   edge: DependencyEdge,
   dependencyGraph: DependencyGraph,
   transformOptions: JSTransformerOptions,
-  edges: DependencyEdges,
+  graph: Graph,
   delta: Delta,
   onProgress?: (numProcessed: number, total: number) => mixed = () => {},
 ): Promise<void> {
@@ -174,7 +181,7 @@ async function traverseDependenciesForSingleFile(
     edge,
     dependencyGraph,
     transformOptions,
-    edges,
+    graph,
     delta,
     () => {
       total++;
@@ -194,7 +201,7 @@ async function processEdge(
   edge: DependencyEdge,
   dependencyGraph: DependencyGraph,
   transformOptions: JSTransformerOptions,
-  edges: DependencyEdges,
+  graph: Graph,
   delta: Delta,
   onDependencyAdd: () => mixed,
   onDependencyAdded: () => mixed,
@@ -228,7 +235,7 @@ async function processEdge(
 
   for (const [relativePath, absolutePath] of previousDependencies) {
     if (!currentDependencies.has(relativePath)) {
-      removeDependency(edge, absolutePath, edges, delta);
+      removeDependency(edge, absolutePath, graph, delta);
     }
   }
 
@@ -247,7 +254,7 @@ async function processEdge(
           absolutePath,
           dependencyGraph,
           transformOptions,
-          edges,
+          graph,
           delta,
           onDependencyAdd,
           onDependencyAdded,
@@ -262,12 +269,12 @@ async function addDependency(
   path: string,
   dependencyGraph: DependencyGraph,
   transformOptions: JSTransformerOptions,
-  edges: DependencyEdges,
+  graph: Graph,
   delta: Delta,
   onDependencyAdd: () => mixed,
   onDependencyAdded: () => mixed,
 ): Promise<void> {
-  const existingEdge = edges.get(path);
+  const existingEdge = graph.dependencies.get(path);
 
   // The new dependency was already in the graph, we don't need to do anything.
   if (existingEdge) {
@@ -276,7 +283,7 @@ async function addDependency(
     return;
   }
 
-  const edge = createEdge(dependencyGraph.getModuleForPath(path), edges);
+  const edge = createEdge(dependencyGraph.getModuleForPath(path), graph);
 
   edge.inverseDependencies.add(parentEdge.path);
   delta.added.set(edge.path, edge);
@@ -287,7 +294,7 @@ async function addDependency(
     edge,
     dependencyGraph,
     transformOptions,
-    edges,
+    graph,
     delta,
     onDependencyAdd,
     onDependencyAdded,
@@ -299,10 +306,10 @@ async function addDependency(
 function removeDependency(
   parentEdge: DependencyEdge,
   absolutePath: string,
-  edges: DependencyEdges,
+  graph: Graph,
   delta: Delta,
 ): void {
-  const edge = edges.get(absolutePath);
+  const edge = graph.dependencies.get(absolutePath);
 
   if (!edge) {
     return;
@@ -322,14 +329,14 @@ function removeDependency(
   // clean up everything (we cannot read the module because it may have
   // been deleted).
   for (const depAbsolutePath of edge.dependencies.values()) {
-    removeDependency(edge, depAbsolutePath, edges, delta);
+    removeDependency(edge, depAbsolutePath, graph, delta);
   }
 
   // This module is not used anywhere else!! we can clear it from the bundle
-  destroyEdge(edge, edges);
+  destroyEdge(edge, graph);
 }
 
-function createEdge(module: Module, edges: DependencyEdges): DependencyEdge {
+function createEdge(module: Module, graph: Graph): DependencyEdge {
   const edge = {
     dependencies: new Map(),
     inverseDependencies: new Set(),
@@ -341,7 +348,7 @@ function createEdge(module: Module, edges: DependencyEdges): DependencyEdge {
       type: getType(module),
     },
   };
-  edges.set(module.path, edge);
+  graph.dependencies.set(module.path, edge);
 
   return edge;
 }
@@ -358,8 +365,8 @@ function getType(module: Module): DependencyType {
   return 'module';
 }
 
-function destroyEdge(edge: DependencyEdge, edges: DependencyEdges) {
+function destroyEdge(edge: DependencyEdge, graph: Graph) {
-  edges.delete(edge.path);
+  graph.dependencies.delete(edge.path);
 }
 
 function resolveDependencies(