Skip to content

feat: determine circular dependency#398

Closed
underfin wants to merge 3 commits into
12-26-feat_add_ambiguous_export_errorfrom
12-27-feat_determine_circular_dependency
Closed

feat: determine circular dependency#398
underfin wants to merge 3 commits into
12-26-feat_add_ambiguous_export_errorfrom
12-27-feat_determine_circular_dependency

Conversation

@underfin

@underfin underfin commented Dec 27, 2023

Copy link
Copy Markdown
Contributor

Description

Support warning for circular dependency, here is an example, esbuild is not doing it, it is done by rollup. The warning is important to let the user improve the code quality.

[CIRCULAR_DEPENDENCY] Warning: Circular dependency: main.js -> foo.js -> main.js.

The implement is port from https://github.com/rollup/rollup/blob/master/src/utils/executionOrder.ts#L31, it is easy to compat rollup code(the rollup implement look like is wired, it may have special behavior, so here using it), but the current sort_modules function can't add the logic, so I add single check function.

Test Plan

Added.


@underfin

underfin commented Dec 27, 2023

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @underfin and the rest of your teammates on Graphite Graphite

Comment thread crates/rolldown/src/bundler/stages/link_stage.rs Outdated
}

// Port from https://github.com/rollup/rollup/blob/master/src/utils/executionOrder.ts#L31
fn determine_circular_dependency(&mut self) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that we don't need an extra traversal just to checking if there are circles. This should be able to be done during other process.

@hyf0 hyf0 added the on-hold label Jan 22, 2024
@hyf0 hyf0 force-pushed the 12-26-feat_add_ambiguous_export_error branch 2 times, most recently from 45b47d5 to 12ca49a Compare January 22, 2024 16:41
@hyf0

hyf0 commented Apr 22, 2024

Copy link
Copy Markdown
Member

Closed by #830

@hyf0 hyf0 closed this Apr 22, 2024
@Brooooooklyn Brooooooklyn deleted the 12-27-feat_determine_circular_dependency branch January 17, 2025 05:04
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

Successfully merging this pull request may close these issues.

3 participants