Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular Dependencies Listed Twice #48

Open
abirmingham opened this issue Sep 17, 2019 · 1 comment
Open

Circular Dependencies Listed Twice #48

abirmingham opened this issue Sep 17, 2019 · 1 comment

Comments

@abirmingham
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants