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 / cyclic dependency between glob.js and sync.js #365

Closed
paulocoghi opened this issue Mar 22, 2018 · 8 comments
Closed

Circular / cyclic dependency between glob.js and sync.js #365

paulocoghi opened this issue Mar 22, 2018 · 8 comments

Comments

@paulocoghi
Copy link

When using glob in a project processed by Rollup, it identifies a circular dependency between glob.js and sync.js.

This issue is just a note, because we can use glob as it is and it is possible to work around this problem. But I'd like to notify you of that.

@isaacs
Copy link
Owner

isaacs commented Mar 22, 2018

This is not a problem. Why does rollup think it is?

@masone
Copy link

masone commented Dec 10, 2021

since #374 is not moving forward - what's the workaround @paulocoghi?

@paulocoghi
Copy link
Author

paulocoghi commented Dec 11, 2021

@masone , unfortunately I don't remember exactly but I believe the solution is to mark "node-glob" as an external dependency on Rollup config, as well as leaving it on the "dependencies" of your project, instead of the "dev-dependencies", since with this approach it will not be included in the final bundle generated by Rollup.

@paulocoghi
Copy link
Author

paulocoghi commented Dec 11, 2021

This is sad because one positive consequence of using Rollup is to dedup and bundle all deps in a single independent JS file with zero runtime deps, providing a better experience for the users of your library (lower lib size, lower install time, etc).

Such workarounds, like informing to do not bundle a specific library, undoes part of this benefit.

I don't know if Rollup should/could be able to solve such issues (circular dep) by itself.

@kuraga
Copy link

kuraga commented Apr 24, 2022

Rollup workarounded it in rollup/plugins#1038 (@rollup/plugin-commonjs 22.0.0).

It's a shame to tell this is not a problem in a PR which removes needless code and let us compile it.

eventualbuddha added a commit to eventualbuddha/node-glob that referenced this issue May 23, 2022
Another pass at isaacs#374, fixing issues with certain bundlers like Rollup (see isaacs#365).
@Stanzilla
Copy link

This is not a problem. Why does rollup think it is?

Even though it isn't, would still be nice to get it fixed since it is holding back large parts of the ecosystem that depend on node-glob.

@michaelfaith
Copy link

Yeah, that's a pretty poor response. An upstream component to so many things, and to just go ¯_(ツ)_/¯

@leumasme
Copy link

I ended up just switching to fast-glob. It has a proper project structure&tests, supports promises and is written in typescript. Seems to be actively maintained.

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

7 participants