aackerman / circular-dependency-plugin

Detect circular dependencies in modules compiled with Webpack
https://www.npmjs.com/package/circular-dependency-plugin
ISC License
921 stars 47 forks source link

Circular Dependencies Listed Twice #48

Open abirmingham opened 5 years ago

abirmingham commented 5 years ago

Hello!

I'm seeing detections which I would consider to be duplicates.

Imagine that I have a.ts and b.ts and they import from one another, creating a circular dependency. In this case I would expect a single detection like a.ts -> b.ts -> a.ts, but instead I see a.ts -> b.ts -> a.ts and b.ts -> a.ts -> b.ts.

This is easy enough to work around with custom configuration, and I will include mine at the bottom of this message, but I'm wondering if the tool itself should dedupe these? Happy to create an MR if I'm pointed in the right direction. Thanks!

My Dedupe Config:

let circularDependencies = [];

module.exports = {
    exclude: /node_modules/,
    onStart() {
        circularDependencies = [];
    },
    onDetected({ paths }) {
        circularDependencies.push(paths);
    },
    onEnd({ compilation }) {
        // De-dupe paths if they they are identical when wrapped
        // e.g. 'a -> b -> a' and 'b -> a -> b' are identical
        circularDependencies
            .reduce(
                (memo, paths) => {
                    const combinations = [];
                    let currentCombination = [...paths];
                    for (let i = 0; i < paths.length - 1; i++) {
                        const msg = currentCombination.join(' > ');
                        if (memo.asSet.has(msg)) return memo;
                        combinations.push(msg);
                        currentCombination = [...paths.slice(1), paths[1]];
                    }
                    combinations.forEach(msg => memo.asSet.add(msg));
                    memo.asListOfMessages.push(combinations[0]);
                    return memo;
                },
                { asSet: new Set(), asListOfMessages: [] },
            )
            .asListOfMessages.forEach(msg => {
                compilation.warnings.push(
                    `Circular dependency detected:\n${msg}`,
                );
            });
    },
};
kennyhyun commented 1 year ago

Thanks for sharing the nice code. I am happy with the result

I had a.js -> b.js -> a.js and b.js -> a.js -> b.js

and now I have

b.js -> a.js -> b.js

only for example.

But had some curiosity.

I think [...paths.slice(1), paths[1]] this is same in the for loop.

so I tried

for (let i = 0; i < 2; i++) {

and had the same result.

I think you I meant

[...paths.slice(i + 1), ...paths.slice(1, i + 1), paths[i + 1]]

it has same result for my case though.

BTW, still curious why this package had to return the repeating item. from the first place. not sure if it could have added the first item when it generates the error message