diff --git a/__tests__/index.test.js b/__tests__/index.test.js index d5f1c2e..6837f60 100644 --- a/__tests__/index.test.js +++ b/__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') }) }) } diff --git a/index.js b/index.js index 7fd9411..51cff34 100644 --- a/index.js +++ b/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 + }) + }) } } diff --git a/package.json b/package.json index cf8aad0..4c20053 100644 --- a/package.json +++ b/package.json @@ -35,5 +35,8 @@ "testMatch": [ "**/?(*.)(spec|test).js?(x)" ] + }, + "dependencies": { + "tarjan-graph": "^2.0.0" } }