Faster is acyclic test

__tests__/is-acyclic.test.js

@@ -0,0 +1,112 @@
+let { isAcyclic, depthFirstIterator } = require('../is-acyclic');
+
+
+function makeGraph(adjacencyList) {
+  const vertices = adjacencyList.map((v) => v[0]);
+  const arrow = (vertex) => {
+    for (const adj of adjacencyList)
+      if (adj[0] === vertex)
+        return adj[1];
+    throw new Error('invalid vertex: ' + vertex);
+  }
+  return {
+    vertices,
+    arrow,
+  }
+}
+
+
+describe('makeGraph', () => {
+  const graph = makeGraph([
+    ['u', ['v', 'x']],
+    ['v', ['y']],
+    ['w', ['y', 'z']],
+    ['x', ['v']],
+    ['y', ['x']],
+    ['z', ['z']],
+  ]);
+
+  it('constructs vertices', () => {
+    expect(graph.vertices).toEqual(['u', 'v', 'w', 'x', 'y', 'z'])
+  });
+
+  it('constructs arrow function', () => {
+    expect(graph.arrow('u')).toEqual(['v', 'x'])
+    expect(graph.arrow('v')).toEqual(['y'])
+    expect(graph.arrow('w')).toEqual(['y', 'z'])
+    expect(graph.arrow('x')).toEqual(['v'])
+    expect(graph.arrow('y')).toEqual(['x'])
+    expect(graph.arrow('z')).toEqual(['z'])
+  });
+})
+
+
+describe('depthFirstIterator', () => {
+
+  const graph = makeGraph([
+    ['u', ['v', 'x']],
+    ['v', ['y']],
+    ['w', ['y', 'z']],
+    ['x', ['v']],
+    ['y', ['x']],
+    ['z', ['z']],
+  ]);
+
+  it('traverses all vertices', () => {
+    vertices = new Set();
+    depthFirstIterator(
+      graph,
+      (vertex, adjacent) => vertices.add(vertex),
+      (u, v) => {},
+    )
+    expect(vertices).toEqual(new Set(graph.vertices));
+  });
+
+  it('follows this path', () => {
+    const path = [];
+    depthFirstIterator(
+      graph,
+      (vertex, adjacent) => path.push({ vertex, adjacent }),
+      (u, v) => path.push({ backEdge: [u, v] }),
+    )
+    expect(path).toEqual([
+      { vertex: 'u', adjacent: ['v', 'x'] },
+      { vertex: 'v', adjacent: ['y'] },
+      { vertex: 'y', adjacent: ['x'] },
+      { vertex: 'x', adjacent: ['v'] },
+      { backEdge: ['x', 'v'] },
+      { vertex: 'w', adjacent: ['y', 'z'] },
+      { vertex: 'z', adjacent: ['z'] },
+      { backEdge: ['z', 'z'] },
+    ]);
+  });
+});
+
+
+describe('isAcyclic', () => {
+
+  it('finds no cycle in tree', () => {
+    // note: adjacent vertices are alphabetically higher => tree graph
+    const graph = makeGraph([
+      ['u', ['v', 'x']],
+      ['v', ['y']],
+      ['w', ['y', 'z']],
+      ['x', []],
+      ['y', []],
+      ['z', []],
+    ]);
+    expect(isAcyclic(graph)).toBe(true);
+  });
+
+  it('finds cycle in complicated graph', () => {
+    const graph = makeGraph([
+      ['u', ['v', 'x']],
+      ['v', ['y']],
+      ['w', ['y', 'z']],
+      ['x', ['v']],
+      ['y', ['x']],
+      ['z', ['z']],
+    ]);
+    expect(isAcyclic(graph)).toBe(false);
+  });
+});

index.js

@@ -2,6 +2,7 @@ let path = require('path')
 let extend = require('util')._extend
 let BASE_ERROR = 'Circular dependency detected:\r\n'
 let PluginTitle = 'CircularDependencyPlugin'
+let { isAcyclic } = require('./is-acyclic');
 
 class CircularDependencyPlugin {
   constructor(options) {
@@ -24,111 +25,156 @@ class CircularDependencyPlugin {
         if (plugin.options.onStart) {
           plugin.options.onStart({ compilation });
         }
-        for (let module of modules) {
-          const shouldSkip = (
-            module.resource == null ||
-            plugin.options.exclude.test(module.resource) ||
-            !plugin.options.include.test(module.resource)
-          )
-          // skip the module if it matches the exclude pattern
-          if (shouldSkip) {
-            continue
-          }
 
-          let maybeCyclicalPathsList = this.isCyclic(module, module, {}, compilation)
-          if (maybeCyclicalPathsList) {
-            // allow consumers to override all behavior with onDetected
-            if (plugin.options.onDetected) {
-              try {
-                plugin.options.onDetected({
-                  module: module,
-                  paths: maybeCyclicalPathsList,
-                  compilation: compilation
-                })
-              } catch(err) {
-                compilation.errors.push(err)
-              }
-              continue
-            }
+        const graph = webpackDependencyGraph(compilation, modules, plugin, this.options);
 
+        cycleDetector(graph, (cycle) => {
+          // print modules as paths in error messages
+          const cyclicalPaths = cycle.map(
+            (module) => path.relative(cwd, module.resource));
+          // allow consumers to override all behavior with onDetected
+          if (plugin.options.onDetected) {
+            try {
+              plugin.options.onDetected({
+                module: module,
+                paths: cyclicalPaths,
+                compilation: compilation
+              })
+            } catch(err) {
+              compilation.errors.push(err)
+            }
+          } else {
             // mark warnings or errors on webpack compilation
-            let error = new Error(BASE_ERROR.concat(maybeCyclicalPathsList.join(' -> ')))
+            let error = new Error(BASE_ERROR.concat(cyclicalPaths.join(' -> ')))
             if (plugin.options.failOnError) {
               compilation.errors.push(error)
             } else {
               compilation.warnings.push(error)
             }
           }
-        }
+        });
+
         if (plugin.options.onEnd) {
           plugin.options.onEnd({ compilation });
         }
       })
     })
   }
+}
+
+
+/**
+ * Construct the dependency (directed) graph for the given plugin options
+ *
+ * Returns the graph as a object { vertices, arrow } where
+ * - vertices is an array containing all vertices, and
+ * - arrow is a function mapping vertices to the array of dependencies, that is, 
+ *   the head vertex for each graph edge whose tail is the given vertex.
+ */
+function webpackDependencyGraph(compilation, modules, plugin, options) {
+
+  // vertices of the dependency graph are the modules
+  const vertices = modules.filter((module) =>
+    module.resource != null &&
+      !plugin.options.exclude.test(module.resource) &&
+      plugin.options.include.test(module.resource)
+  );
+
+  // arrow function for the dependency graph
+  const arrow = (module) => module.dependencies
+        .filter((dependency) => {
+          // ignore CommonJsSelfReferenceDependency
+          if (dependency.constructor &&
+              dependency.constructor.name === 'CommonJsSelfReferenceDependency') {
+            return false;
+          }
+          // ignore dependencies that are resolved asynchronously
+          if (options.allowAsyncCycles && dependency.weak) { return false }
+          return true;
+        })
+        .map((dependency) => {
+          // map webpack dependency to module
+          if (compilation.moduleGraph) {
+            // handle getting a module for webpack 5
+            return compilation.moduleGraph.getModule(dependency)
+          } else {
+            // handle getting a module for webpack 4
+            return dependency.module
+          }
+        })
+        .filter((depModule) => {
+          if (!depModule) { return false }
+          // ignore dependencies that don't have an associated resource
+          if (!depModule.resource) { return false }
+          // the dependency was resolved to the current module due to how webpack internals
+          // setup dependencies like CommonJsSelfReferenceDependency and ModuleDecoratorDependency
+          if (module === depModule) {
+            return false
+          }
+          return true;
+        });
+
+  return { vertices, arrow };
+}
 
-  isCyclic(initialModule, currentModule, seenModules, compilation) {
-    let cwd = this.options.cwd
 
-    // Add the current module to the seen modules cache
-    seenModules[currentModule.debugId] = true
+/**
+ * Call the callback with all cycles in the directed graph defined by (vertices, arrow)
+ *
+ * The graph is acyclic iff the callback is not called.
+ */
+function cycleDetector(graph, cycleCallback) {
+  // checking that there are no cycles is much faster than actually listing cycles
+  if (isAcyclic(graph))
+    return;
 
-    // If the modules aren't associated to resources
-    // it's not possible to display how they are cyclical
-    if (!currentModule.resource || !initialModule.resource) {
-      return false
+  /**
+   * This is the old implementation which has performance issues. In the future, it might
+   * be replaced with a different implementation. Keep it for now so none of the unit tests change.
+   */
+  for (let module of graph.vertices) {
+    let cycle = findModuleCycleAt(module, module, {}, graph.arrow)
+    if (cycle) {
+      cycleCallback(cycle);
     }
+  }
+}
 
-    // Iterate over the current modules dependencies
-    for (let dependency of currentModule.dependencies) {
-      if (
-        dependency.constructor &&
-        dependency.constructor.name === 'CommonJsSelfReferenceDependency'
-      ) {
-        continue
-      }
 
-      let depModule = null
-      if (compilation.moduleGraph) {
-        // handle getting a module for webpack 5
-        depModule = compilation.moduleGraph.getModule(dependency)
-      } else {
-        // handle getting a module for webpack 4
-        depModule = dependency.module
-      }
+// part of the cycleDetector implementation
+function findModuleCycleAt(initialModule, currentModule, seenModules, arrow) {
 
-      if (!depModule) { continue }
-      // ignore dependencies that don't have an associated resource
-      if (!depModule.resource) { continue }
-      // ignore dependencies that are resolved asynchronously
-      if (this.options.allowAsyncCycles && dependency.weak) { continue }
-      // the dependency was resolved to the current module due to how webpack internals
-      // setup dependencies like CommonJsSelfReferenceDependency and ModuleDecoratorDependency
-      if (currentModule === depModule) {
-        continue
-      }
+  // Add the current module to the seen modules cache
+  seenModules[currentModule.debugId] = true
 
-      if (depModule.debugId in seenModules) {
-        if (depModule.debugId === initialModule.debugId) {
-          // Initial module has a circular dependency
-          return [
-            path.relative(cwd, currentModule.resource),
-            path.relative(cwd, depModule.resource)
-          ]
-        }
-        // Found a cycle, but not for this module
-        continue
-      }
+  // If the modules aren't associated to resources
+  // it's not possible to display how they are cyclical
+  if (!currentModule.resource || !initialModule.resource) {
+    return false
+  }
 
-      let maybeCyclicalPathsList = this.isCyclic(initialModule, depModule, seenModules, compilation)
-      if (maybeCyclicalPathsList) {
-        maybeCyclicalPathsList.unshift(path.relative(cwd, currentModule.resource))
-        return maybeCyclicalPathsList
+  // Iterate over the current modules dependencies
+  for (let depModule of arrow(currentModule)) {
+    if (depModule.debugId in seenModules) {
+      if (depModule.debugId === initialModule.debugId) {
+        // Initial module has a circular dependency
+        return [
+          currentModule,
+          depModule,
+        ]
       }
+      // Found a cycle, but not for this module
+      continue
     }
 
-    return false
+    let maybeCyclicalPathsList = findModuleCycleAt(initialModule, depModule, seenModules, arrow)
+    if (maybeCyclicalPathsList) {
+      maybeCyclicalPathsList.unshift(currentModule);
+      return maybeCyclicalPathsList;
+    }
   }
+  return false
 }
 
+
 module.exports = CircularDependencyPlugin