Replace cycle detection algorithm with Tarjan's strongly connected components algorithm

__tests__/index.test.js

@@ -57,11 +57,8 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let msg0 = getWarningMessage(stats, 0)
-      let msg1 = getWarningMessage(stats, 1)
       expect(msg0).toContain('__tests__/deps/b.js -> __tests__/deps/c.js -> __tests__/deps/b.js')
       expect(msg0).toMatch(/Circular/)
-      expect(msg1).toMatch(/b\.js/)
-      expect(msg1).toMatch(/c\.js/)
     })
 
     it('detects circular dependencies from d -> e -> f -> g -> e', async () => {
@@ -78,12 +75,8 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let msg0 = getWarningMessage(stats, 0)
-      let msg1 = getWarningMessage(stats, 1)
       expect(msg0).toContain('__tests__/deps/e.js -> __tests__/deps/f.js -> __tests__/deps/g.js -> __tests__/deps/e.js')
       expect(msg0).toMatch(/Circular/)
-      expect(msg1).toMatch(/e\.js/)
-      expect(msg1).toMatch(/f\.js/)
-      expect(msg1).toMatch(/g\.js/)
     })
 
     it('uses errors instead of warnings with failOnError', async () => {
@@ -102,12 +95,8 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let err0 = getErrorsMessage(stats, 0)
-      let err1 = getErrorsMessage(stats, 1)
       expect(err0).toContain('__tests__/deps/e.js -> __tests__/deps/f.js -> __tests__/deps/g.js -> __tests__/deps/e.js')
       expect(err0).toMatch(/Circular/)
-      expect(err1).toMatch(/e\.js/)
-      expect(err1).toMatch(/f\.js/)
-      expect(err1).toMatch(/g\.js/)
     })
 
     it('can exclude cyclical deps from being output', async () => {
@@ -128,13 +117,10 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let msg0 = getWarningMessage(stats, 0)
-      let msg1 = getWarningMessage(stats, 1)
       expect(msg0).toMatch(/Circular/)
-      expect(msg1).toMatch(/e\.js/)
-      expect(msg1).toMatch(/g\.js/)
     })
 
-    it('can include only specific cyclical deps in the output', async () => {
+    it('includes all cyclical deps in the output even if some are excluded', async () => {
       let fs = new MemoryFS()
       let compiler = webpack({
         mode: 'development',
@@ -150,11 +136,28 @@ for (let version of versions) {
 
       let runAsync = wrapRun(compiler.run.bind(compiler))
       let stats = await runAsync()
-      stats.warnings.forEach(warning => {
-        let msg = typeof warning == 'string' ? warning : warning.message
-        const firstFile = msg.match(/\w+\.js/)[0]
-        expect(firstFile).toMatch(/f\.js/)
+      let msg0 = getWarningMessage(stats, 0)
+      expect(msg0).toMatch(/Circular dependency detected:\s*__tests__\/deps\/e.js -> __tests__\/deps\/f.js -> __tests__\/deps\/g.js -> __tests__\/deps\/e.js/)
+    })
+
+    it('does not report errors for cycles where all files are excluded', async () => {
+      let fs = new MemoryFS()
+      let compiler = webpack({
+        mode: 'development',
+        entry: path.join(__dirname, 'deps/d.js'),
+        output: { path: __dirname },
+        plugins: [
+          new CircularDependencyPlugin({
+            exclude: /(e|f|g)\.js/
+          })
+        ]
       })
+      compiler.outputFileSystem = fs
+
+      let runAsync = wrapRun(compiler.run.bind(compiler))
+      let stats = await runAsync()
+      let msg0 = getWarningMessage(stats, 0)
+      expect(msg0).toEqual(null)
     })
 
     it(`can handle context modules that have an undefined resource h -> i -> a -> i`, async () => {
@@ -227,10 +230,10 @@ for (let version of versions) {
       let runAsync = wrapRun(compiler.run.bind(compiler))
       await runAsync()
       expect(cyclesPaths).toEqual([
-        '__tests__/deps/g.js',
         '__tests__/deps/e.js',
         '__tests__/deps/f.js',
-        '__tests__/deps/g.js'
+        '__tests__/deps/g.js',
+        '__tests__/deps/e.js',
       ])
     })
 
@@ -254,11 +257,7 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let msg0 = getWarningMessage(stats, 0)
-      let msg1 = getWarningMessage(stats, 1)
       expect(msg0).toContain('__tests__/deps/e.js -> __tests__/deps/f.js -> __tests__/deps/g.js -> __tests__/deps/e.js')
-      expect(msg1).toMatch(/e\.js/)
-      expect(msg1).toMatch(/f\.js/)
-      expect(msg1).toMatch(/g\.js/)
     })
 
     it('can detect circular dependencies when the ModuleConcatenationPlugin is used', async () => {
@@ -278,9 +277,7 @@ for (let version of versions) {
       let stats = await runAsync()
 
       let msg0 = getWarningMessage(stats, 0)
-      let msg1 = getWarningMessage(stats, 1)
       expect(msg0).toContain('__tests__/deps/module-concat-plugin-compat/a.js -> __tests__/deps/module-concat-plugin-compat/b.js -> __tests__/deps/module-concat-plugin-compat/a.js')
-      expect(msg1).toContain('__tests__/deps/module-concat-plugin-compat/b.js -> __tests__/deps/module-concat-plugin-compat/a.js -> __tests__/deps/module-concat-plugin-compat/b.js')
     })
   })
 }

index.js

@@ -1,5 +1,6 @@
 let path = require('path')
 let extend = require('util')._extend
+let Graph = require('tarjan-graph')
 let BASE_ERROR = 'Circular dependency detected:\r\n'
 let PluginTitle = 'CircularDependencyPlugin'
 
@@ -24,98 +25,82 @@ class CircularDependencyPlugin {
         if (plugin.options.onStart) {
           plugin.options.onStart({ compilation });
         }
+        const dependencyGraph = new Graph()
+        //console.log('LENGTH', modules.length);
         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
-            }
 
-            // mark warnings or errors on webpack compilation
-            let error = new Error(BASE_ERROR.concat(maybeCyclicalPathsList.join(' -> ')))
-            if (plugin.options.failOnError) {
-              compilation.errors.push(error)
+          // Iterate over the current modules dependencies
+          const dependedModuleIds = [];
+          for (let dependency of module.dependencies) {
+            let depModule = null
+            if (compilation.moduleGraph) {
+              // handle getting a module for webpack 5
+              depModule = compilation.moduleGraph.getModule(dependency)
             } else {
-              compilation.warnings.push(error)
+              // handle getting a module for webpack 4
+              depModule = dependency.module
             }
+
+            if (!depModule) { continue }
+
+            // ignore dependencies that don't have an associated resource
+            if (!depModule.resource) { continue }
+
+            // optionally ignore dependencies that are resolved asynchronously
+            if (this.options.allowAsyncCycles && dependency.weak) { continue }
+
+            dependedModuleIds.push(depModule.identifier());
           }
+          dependencyGraph.add(module.identifier(), dependedModuleIds)
         }
-        if (plugin.options.onEnd) {
-          plugin.options.onEnd({ compilation });
-        }
-      })
-    })
-  }
 
-  isCyclic(initialModule, currentModule, seenModules, compilation) {
-    let cwd = this.options.cwd
+        const cycles = dependencyGraph.getCycles();
 
-    // Add the current module to the seen modules cache
-    seenModules[currentModule.debugId] = true
+        cycles.forEach((vertices) => {
+          // Convert the array of vertices into an array of module paths
+          const cyclicAbsolutePaths = vertices
+            .slice()
+            .reverse()
+            .map((vertex) => compilation.findModule(vertex.name).resource);
 
-    // 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
-    }
+          if (cyclicAbsolutePaths.every((resource) => (
+            resource == null ||
+            plugin.options.exclude.test(resource) ||
+            !plugin.options.include.test(resource)
+          ))) {
+            // If all modules in the cycle are excluded by the config, don't report an error
+            return;
+          }
 
-    // Iterate over the current modules dependencies
-    for (let dependency of currentModule.dependencies) {
-      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
-      }
+          const cyclicPaths = cyclicAbsolutePaths.map((resource) => path.relative(cwd, resource));
 
-      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 }
+          // allow consumers to override all behavior with onDetected
+          if (plugin.options.onDetected) {
+            try {
+              plugin.options.onDetected({
+                module: module,
+                paths: cyclicPaths.concat([cyclicPaths[0]]),
+                compilation: compilation
+              })
+            } catch(err) {
+              compilation.errors.push(err)
+            }
+            return
+          }
 
-      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)
-          ]
+          // mark warnings or errors on webpack compilation
+          let error = new Error(BASE_ERROR.concat(cyclicPaths.concat([cyclicPaths[0]]).join(' -> ')))
+          if (plugin.options.failOnError) {
+            compilation.errors.push(error)
+          } else {
+            compilation.warnings.push(error)
+          }
+        });
+        if (plugin.options.onEnd) {
+          plugin.options.onEnd({ compilation });
         }
-        // Found a cycle, but not for this module
-        continue
-      }
-
-      let maybeCyclicalPathsList = this.isCyclic(initialModule, depModule, seenModules, compilation)
-      if (maybeCyclicalPathsList) {
-        maybeCyclicalPathsList.unshift(path.relative(cwd, currentModule.resource))
-        return maybeCyclicalPathsList
-      }
-    }
-
-    return false
+      })
+    })
   }
 }
 

package.json

@@ -35,5 +35,8 @@
     "testMatch": [
       "**/?(*.)(spec|test).js?(x)"
     ]
+  },
+  "dependencies": {
+    "tarjan-graph": "^2.0.0"
   }
 }