circular-dependency-plugin icon indicating copy to clipboard operation
circular-dependency-plugin copied to clipboard

Circular Dependencies Listed Twice

Open abirmingham opened this issue 5 years ago • 1 comments

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}`,
                );
            });
    },
};

abirmingham avatar Sep 17 '19 23:09 abirmingham

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

kennyhyun avatar Aug 24 '23 02:08 kennyhyun