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

Rollup plugin Circular Dependency issue #99

Closed
dananderson opened this issue Mar 8, 2019 · 5 comments
Closed

Rollup plugin Circular Dependency issue #99

dananderson opened this issue Mar 8, 2019 · 5 comments

Comments

@dananderson
Copy link

command-line-args caused this error with rollup:

(!) Circular dependency: node_modules/command-line-args/lib/option.js -> node_modules/command-line-args/lib/option-flag.js -> node_modules/command-line-args/lib/option.js

A fix would be to move Option/OptionFlag.create to a new OptionFactory class file and update create callers to use the factory.

@75lb
Copy link
Owner

75lb commented Mar 13, 2019

Hi, could you post a reproduction case please - some code I can debug and fix. Thanks.

dananderson added a commit to dananderson/command-line-args-cicular-dependency that referenced this issue Mar 13, 2019
@dananderson
Copy link
Author

Thanks for looking into this.

https://github.com/dananderson/command-line-args-cicular-dependency

npm
npm run rollup
node one.js

My rollup target environment is node with es6 classes. rollup produces one.js with a circular dependency warning. one.js is created, but running one.js results in a runtime error due to the circular dependency.

@75lb
Copy link
Owner

75lb commented Mar 14, 2019

thanks for the reproduction case.

The option-flag module is lazy-loaded at run time therefore does not cause a compile-time circular reference. This is a rollup issue caused by a bug in rollup-plugin-commonjs.

Do you have a quick fix for this? If so, please drop me a PR. The long-term fix is to upgrade the command-line-args project to offer an ESM entry-point (as well as CJS). I'll do that when I next get chance.

@75lb 75lb changed the title Circular Dependency Rollup plugin Circular Dependency issue Mar 14, 2019
@dananderson
Copy link
Author

Thanks for digging into the rollup stuff.

The root cause seems to be how rollup translates inline requires. I suspect it is not worth fixing on the rollup side because user land expects the existing behavior.. and es modules are coming.

The long term fix would be an es module. For the short term, it would be nice to get command-line-args working with rollup. I'll propose a PR next week and we can discuss if it fits into your project.

@75lb
Copy link
Owner

75lb commented Mar 24, 2019

Fixed and released in v5.1.0.

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 a pull request may close this issue.

2 participants