-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Comments
This is not a problem. Why does rollup think it is? |
since #374 is not moving forward - what's the workaround @paulocoghi? |
@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. |
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. |
Rollup workarounded it in rollup/plugins#1038 ( It's a shame to tell |
Another pass at isaacs#374, fixing issues with certain bundlers like Rollup (see isaacs#365).
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. |
Yeah, that's a pretty poor response. An upstream component to so many things, and to just go ¯_(ツ)_/¯ |
I ended up just switching to |
When using glob in a project processed by Rollup, it identifies a circular dependency between
glob.js
andsync.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.
The text was updated successfully, but these errors were encountered: