Skip to content

fix: circularDependencyRspackPlugin should check all cycle#10163

Merged
LingyuCoder merged 2 commits intomainfrom
fix-9889
Apr 24, 2025
Merged

fix: circularDependencyRspackPlugin should check all cycle#10163
LingyuCoder merged 2 commits intomainfrom
fix-9889

Conversation

@fireairforce
Copy link
Copy Markdown
Contributor

@fireairforce fireairforce commented Apr 24, 2025

Summary

In CircularDependencyRspackPlugin, we should check all cycle, for example, if we have follow case:

// a.js
import './b.js';

// b.js
import './c.js';

// c.js
import './d.js';
import './e.js';

// d.js
import './a.js';

// e.js
import './a.js';

In that case, we shoud output two cycle check diagnostic messages:

a.js -> b.js -> c.js -> d.js -> a.js

a.js -> b.js -> c.js -> e.js -> a.js

In the algorithm of CircularDependencyRspackPlugin's fn recurse_dependencies() we just use a visited(seen_set) hashmap to filter the duplicate module id, like c.js will be recurse twice, so it will not be outputed twice, the output now will be:

a.js -> b.js -> c.js -> d.js -> a.js

The behavior is not right as the user's feedback, so we need to move the visited hashmap here in the algorithm, just use the simple algorithm here like the answer of https://leetcode.cn/problems/binary-tree-paths/description/.

closes: #9889

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 24, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit e178d9d
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/680a208201747e00094be6cf

@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Apr 24, 2025
@fireairforce fireairforce force-pushed the fix-9889 branch 2 times, most recently from ee79641 to d4867b8 Compare April 24, 2025 09:39
@fireairforce fireairforce marked this pull request as ready for review April 24, 2025 09:44
LingyuCoder
LingyuCoder previously approved these changes Apr 24, 2025
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 24, 2025

CodSpeed Performance Report

Merging #10163 will not alter performance

Comparing fix-9889 (e178d9d) with main (42a8a01)

Summary

✅ 11 untouched benchmarks

@LingyuCoder LingyuCoder merged commit f9ee1c9 into main Apr 24, 2025
30 checks passed
@LingyuCoder LingyuCoder deleted the fix-9889 branch April 24, 2025 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: circular dependency detection is non-deterministic

2 participants