diff --git a/__tests__/is-acyclic.test.js b/__tests__/is-acyclic.test.js new file mode 100644 index 0000000..8122f8b --- /dev/null +++ b/__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); + }); +}); diff --git a/index.js b/index.js index 32c7be9..a2c8470 100644 --- a/index.js +++ b/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 diff --git a/is-acyclic.js b/is-acyclic.js new file mode 100644 index 0000000..00a0127 --- /dev/null +++ b/is-acyclic.js @@ -0,0 +1,69 @@ + + +/** + * Test whether a directed graph is acyclic + * + * The graph is represented as a object { vertices, arrow } where + * - vertices is an array containing all vertices, and + * - arrow is a function mapping a tail vertex to the array of head vertices, that is, + * the vertices at the head of each graph edge whose tail is the given vertex. + * + * For example: + * + * x <- y <- z + * + * vertices == [x, y, z] (order does not matter) + * arrow(x) == [] + * arrow(y) == [x] + * arrow(z) == [y] + * + * See https://stackoverflow.com/questions/261573/best-algorithm-for-detecting-cycles-in-a-directed-graph + */ +function isAcyclic(graph) { + let isAcyclic = true; + depthFirstIterator( + graph, + () => {}, + (backEdge) => isAcyclic = false); + return isAcyclic; +} + + +/** + * Depth-first traversal of the graph + * + * The visitor function is called with vertex, adjacent vertices + * The backEdge function is called with head, tail of a back edge + */ +function depthFirstIterator(graph, visitorFn, backEdgeFn) { + const discovered = new Set(); + const finished = new Set(); + for (const vertex of graph.vertices) { + if (!(discovered.has(vertex) || finished.has(vertex))) + depthFirstVisitor(vertex, discovered, finished, graph, visitorFn, backEdgeFn) + } +} + + +function depthFirstVisitor(vertex, discovered, finished, graph, visitorFn, backEdgeFn) { + discovered.add(vertex) + const adjacent = graph.arrow(vertex); // the adjacent vertices in the direction of the edges + visitorFn(vertex, adjacent); + + for (const v of adjacent) { + if (discovered.has(v)) { + backEdgeFn(vertex, v); + } else { + if (!finished.has(v)) + depthFirstVisitor(v, discovered, finished, graph, visitorFn, backEdgeFn) + } + } + discovered.delete(vertex) + finished.add(vertex) +} + + +module.exports = { + isAcyclic, + depthFirstIterator, +}