Allow exclusion of certain dependency types from dependency graph (#244)

Blasz · lukebatchelor · commit e14ceda59e09 · 2019-10-02T11:05:19.000+10:00
* Add excludedTypes args to Package.getAllDependencies and Project.getDependen(cy|ts)Graph

Allows the dependency and dependents graphs to be filtered based on
dependency type.

* Change `Project.runPackageTasksGraphParallel` to exclude dev deps in dep graph

* Add --excludeFromGraph flag to exclude certain dependency types from `bolt workspaces` commands

* Add tests for runPackageTasks with excludeFromGraph option

* Add tests for excludeFromGraph

* Fix flow
diff --git a/src/Package.js b/src/Package.js
@@ -8,6 +8,7 @@ import * as logger from './utils/logger';
 import * as messages from './utils/messages';
 import sortObject from 'sort-object';
 import { BoltError } from './utils/errors';
+import type { configDependencyType } from './types';
 
 export default class Package {
   filePath: string;
@@ -46,10 +47,23 @@ export default class Package {
     return this.config.getWorkspaces() || [];
   }
 
-  getAllDependencies() {
+  getAllDependencies(excludedTypes?: configDependencyType[] = []) {
     let allDependencies = new Map();
+    if (excludedTypes.length > 0) {
+      let invalidTypes = excludedTypes.filter(
+        t => DEPENDENCY_TYPES.indexOf(t) === -1
+      );
+      if (invalidTypes.length > 0) {
+        throw new BoltError(
+          `Invalid dependency types to exclude: "${invalidTypes.join(',')}"`
+        );
+      }
+    }
+    let dependencyTypes = DEPENDENCY_TYPES.filter(
+      t => excludedTypes.indexOf(t) === -1
+    );
 
-    for (let type of DEPENDENCY_TYPES) {
+    for (let type of dependencyTypes) {
       let deps = this.config.getDeps(type);
       if (!deps) continue;
 
diff --git a/src/Project.js b/src/Project.js
@@ -4,7 +4,7 @@ import includes from 'array-includes';
 import semver from 'semver';
 import Package from './Package';
 import Config from './Config';
-import type { SpawnOpts, FilterOpts } from './types';
+import type { configDependencyType, SpawnOpts, FilterOpts } from './types';
 import * as fs from './utils/fs';
 import * as logger from './utils/logger';
 import {
@@ -86,7 +86,10 @@ export default class Project {
     return packages;
   }
 
-  async getDependencyGraph(packages: Array<Package>) {
+  async getDependencyGraph(
+    packages: Array<Package>,
+    excludedDependencyTypes?: Array<configDependencyType> | void
+  ) {
     let graph: Map<
       string,
       { pkg: Package, dependencies: Array<string> }
@@ -103,7 +106,7 @@ export default class Project {
     for (let pkg of queue) {
       let name = pkg.config.getName();
       let dependencies = [];
-      let allDependencies = pkg.getAllDependencies();
+      let allDependencies = pkg.getAllDependencies(excludedDependencyTypes);
 
       for (let [depName, depVersion] of allDependencies) {
         let match = packagesByName[depName];
@@ -135,10 +138,14 @@ export default class Project {
     return { graph, valid };
   }
 
-  async getDependentsGraph(packages: Array<Package>) {
+  async getDependentsGraph(
+    packages: Array<Package>,
+    excludedDepTypes?: Array<configDependencyType>
+  ) {
     let graph = new Map();
     let { valid, graph: dependencyGraph } = await this.getDependencyGraph(
-      packages
+      packages,
+      excludedDepTypes
     );
 
     let dependentsLookup: {
@@ -184,7 +191,11 @@ export default class Project {
     } else if (spawnOpts.orderMode === 'parallel-nodes') {
       results = await this.runPackageTasksParallelNodes(packages, wrappedTask);
     } else {
-      results = await this.runPackageTasksGraphParallel(packages, wrappedTask);
+      results = await this.runPackageTasksGraphParallel(
+        packages,
+        wrappedTask,
+        spawnOpts.excludeFromGraph
+      );
     }
 
     results.forEach(r => {
@@ -236,10 +247,12 @@ export default class Project {
 
   async runPackageTasksGraphParallel<T>(
     packages: Array<Package>,
-    task: GenericTask<T>
+    task: GenericTask<T>,
+    excludeDepTypesFromGraph: Array<configDependencyType> | void
   ): Promise<Array<T>> {
     let { graph: dependentsGraph, valid } = await this.getDependencyGraph(
-      packages
+      packages,
+      excludeDepTypesFromGraph
     );
 
     let graph = new Map();
diff --git a/src/__fixtures__/dev-dependent-workspaces-only/package.json b/src/__fixtures__/dev-dependent-workspaces-only/package.json
@@ -0,0 +1,11 @@
+{
+  "private": true,
+  "description": "Contains two workspaces that are only dependent as a devDependency.",
+  "name": "dev-dependent-workspaces-only",
+  "version": "1.0.0",
+  "bolt": {
+    "workspaces": [
+      "packages/*"
+    ]
+  }
+}
diff --git a/src/__fixtures__/dev-dependent-workspaces-only/packages/bar/package.json b/src/__fixtures__/dev-dependent-workspaces-only/packages/bar/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "bar",
+  "version": "1.0.0"
+}
diff --git a/src/__fixtures__/dev-dependent-workspaces-only/packages/foo/package.json b/src/__fixtures__/dev-dependent-workspaces-only/packages/foo/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "foo",
+  "version": "1.0.0",
+  "devDependencies": {
+    "bar": "^1.0.0"
+  }
+}
diff --git a/src/__fixtures__/simple-project-with-multiple-depTypes/package.json b/src/__fixtures__/simple-project-with-multiple-depTypes/package.json
@@ -2,10 +2,15 @@
   "private": true,
   "name": "simple-project-with-multiple-dep-types",
   "version": "1.0.0",
+  "dependencies": {
+    "depOnly": "^1.0.0"
+  },
   "devDependencies": {
+    "devDepOnly": "^1.0.0",
     "react": "^16.0.0"
   },
   "peerDependencies": {
+    "peerDepOnly": "^1.0.0",
     "react": "^16.0.0"
   },
   "bolt": {
diff --git a/src/__tests__/Package.test.js b/src/__tests__/Package.test.js
@@ -55,4 +55,37 @@ describe('Package', () => {
       expect(depTypes).toEqual([]);
     });
   });
+
+  describe('getAllDependencies', () => {
+    it('should return all dependencies of the package', async () => {
+      let filePath = path.join(
+        f.find('simple-project-with-multiple-depTypes'),
+        'package.json'
+      );
+      let pkg = await Package.init(filePath);
+      let dependencies = pkg.getAllDependencies();
+      let expected = new Map([
+        ['depOnly', '^1.0.0'],
+        ['devDepOnly', '^1.0.0'],
+        ['react', '^16.0.0'],
+        ['peerDepOnly', '^1.0.0']
+      ]);
+      expect(dependencies).toEqual(expected);
+    });
+
+    it('should filter out dependencies of a certain type when passed the excludedTypes arg', async () => {
+      let filePath = path.join(
+        f.find('simple-project-with-multiple-depTypes'),
+        'package.json'
+      );
+      let pkg = await Package.init(filePath);
+      let dependencies = pkg.getAllDependencies(['devDependencies']);
+      let expected = new Map([
+        ['depOnly', '^1.0.0'],
+        ['peerDepOnly', '^1.0.0'],
+        ['react', '^16.0.0']
+      ]);
+      expect(dependencies).toEqual(expected);
+    });
+  });
 });
diff --git a/src/__tests__/Project.test.js b/src/__tests__/Project.test.js
@@ -109,6 +109,46 @@ describe('Project', () => {
     });
   });
 
+  test('getDependencyGraph() without excluded dependency types', async () => {
+    let project = await Project.init(f.find('dev-dependent-workspaces-only'));
+    let packages = await project.getPackages();
+    let { valid, graph } = await project.getDependencyGraph(packages);
+    let expectedDependencies = {
+      'dev-dependent-workspaces-only': [],
+      bar: [],
+      foo: ['bar']
+    };
+
+    expect(valid).toEqual(true);
+    expect(graph).toBeInstanceOf(Map);
+    expect(graph.size).toBe(Object.keys(expectedDependencies).length);
+
+    Object.entries(expectedDependencies).forEach(([pkg, dependencies]) => {
+      assertDependencies(graph, pkg, dependencies);
+    });
+  });
+
+  test('getDependencyGraph() with excluded dependency types', async () => {
+    let project = await Project.init(f.find('dev-dependent-workspaces-only'));
+    let packages = await project.getPackages();
+    let { valid, graph } = await project.getDependencyGraph(packages, [
+      'devDependencies'
+    ]);
+    let expectedDependencies = {
+      'dev-dependent-workspaces-only': [],
+      bar: [],
+      foo: []
+    };
+
+    expect(valid).toEqual(true);
+    expect(graph).toBeInstanceOf(Map);
+    expect(graph.size).toBe(Object.keys(expectedDependencies).length);
+
+    Object.entries(expectedDependencies).forEach(([pkg, dependencies]) => {
+      assertDependencies(graph, pkg, dependencies);
+    });
+  });
+
   test('getDependentsGraph() with nested workspaces', async () => {
     let project = await Project.init(f.find('nested-workspaces'));
     let packages = await project.getPackages();
@@ -150,6 +190,44 @@ describe('Project', () => {
     });
   });
 
+  test('getDependentsGraph() without excluded dependency types', async () => {
+    let project = await Project.init(f.find('dev-dependent-workspaces-only'));
+    let packages = await project.getPackages();
+    let { valid, graph } = await project.getDependentsGraph(packages);
+    let expectedDependents = {
+      bar: ['foo'],
+      foo: []
+    };
+
+    expect(valid).toEqual(true);
+    expect(graph).toBeInstanceOf(Map);
+    expect(graph.size).toBe(Object.keys(expectedDependents).length);
+
+    Object.entries(expectedDependents).forEach(([pkg, dependents]) => {
+      assertDependents(graph, pkg, dependents);
+    });
+  });
+
+  test('getDependentsGraph() with excluded dependency types', async () => {
+    let project = await Project.init(f.find('dev-dependent-workspaces-only'));
+    let packages = await project.getPackages();
+    let { valid, graph } = await project.getDependentsGraph(packages, [
+      'devDependencies'
+    ]);
+    let expectedDependents = {
+      bar: [],
+      foo: []
+    };
+
+    expect(valid).toEqual(true);
+    expect(graph).toBeInstanceOf(Map);
+    expect(graph.size).toBe(Object.keys(expectedDependents).length);
+
+    Object.entries(expectedDependents).forEach(([pkg, dependents]) => {
+      assertDependents(graph, pkg, dependents);
+    });
+  });
+
   test('filterPackages() with no flags', async () => {
     let project = await Project.init(f.find('nested-workspaces'));
     let packages = await project.getPackages();
@@ -431,4 +509,42 @@ describe('Project', () => {
     }
     done();
   });
+
+  test('runPackageTasks() excludeFromGraph: devDependencies', async () => {
+    let project = await Project.init(f.find('dev-dependent-workspaces-only'));
+    let packages = await project.getPackages();
+    let ops = [];
+
+    await project.runPackageTasks(
+      packages,
+      { excludeFromGraph: ['devDependencies'] },
+      async pkg => {
+        ops.push('start:' + pkg.getName());
+        await Promise.resolve();
+        ops.push('end:' + pkg.getName());
+      }
+    );
+
+    expect(ops).toEqual(['start:bar', 'start:foo', 'end:bar', 'end:foo']);
+  });
+
+  test('runPackageTasks() excludeFromGraph: devDependencies cycle', async () => {
+    let project = await Project.init(f.find('dependent-workspaces-with-cycle'));
+    let packages = await project.getPackages();
+    let ops = [];
+
+    await project.runPackageTasks(
+      packages,
+      { excludeFromGraph: ['devDependencies'] },
+      async pkg => {
+        ops.push('start:' + pkg.getName());
+        await Promise.resolve();
+        ops.push('end:' + pkg.getName());
+      }
+    );
+
+    expect(ops).toEqual(['start:bar', 'end:bar', 'start:foo', 'end:foo']);
+    // There is not a cycle anymore now that we have excluded devDependencies
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
 });
diff --git a/src/functions/getDependencyGraph.js b/src/functions/getDependencyGraph.js
@@ -1,9 +1,11 @@
 // @flow
 import Project from '../Project';
 import * as yarn from '../utils/yarn';
+import type { configDependencyType } from '../types';
 
 type Options = {
-  cwd?: string
+  cwd?: string,
+  excludedTypes?: Array<configDependencyType>
 };
 
 type DependencyGraph = Map<string, Array<string>>;
@@ -18,7 +20,7 @@ export default async function getDependencyGraph(
   let {
     graph: dependencyGraph,
     valid: graphIsValid
-  } = await project.getDependencyGraph(packages);
+  } = await project.getDependencyGraph(packages, opts.excludedTypes);
 
   if (!graphIsValid) {
     throw new Error('Dependency graph is not valid');
diff --git a/src/functions/getDependentsGraph.js b/src/functions/getDependentsGraph.js
@@ -1,9 +1,11 @@
 // @flow
 import Project from '../Project';
 import * as yarn from '../utils/yarn';
+import type { configDependencyType } from '../types';
 
 type Options = {
-  cwd?: string
+  cwd?: string,
+  excludedTypes?: Array<configDependencyType>
 };
 
 type DependentsGraph = Map<string, Array<string>>;
@@ -18,7 +20,7 @@ export default async function getDependentsGraph(
   let {
     graph: dependentsGraph,
     valid: graphIsValid
-  } = await project.getDependentsGraph(packages);
+  } = await project.getDependentsGraph(packages, opts.excludedTypes);
 
   if (!graphIsValid) {
     throw new Error('Dependents graph is not valid');
diff --git a/src/types.js b/src/types.js
@@ -18,7 +18,10 @@ export type JSONValue =
 
 export type SpawnOpts = {
   orderMode?: 'serial' | 'parallel' | 'parallel-nodes',
-  bail?: boolean
+  bail?: boolean,
+  // Exclude certain dependency types from being included in the dependency graph used to
+  // execute package tasks in a certain order
+  excludeFromGraph?: Array<configDependencyType>
   // maxConcurrent?: number,
 };
 
diff --git a/src/utils/__tests__/options.test.js b/src/utils/__tests__/options.test.js
diff --git a/src/utils/options.js b/src/utils/options.js

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "name": "bar",
 +  "version": "1.0.0"
 +}