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 dependencies in moment #4305

Closed
nicolashenry opened this issue Nov 14, 2017 · 2 comments
Closed

Circular dependencies in moment #4305

nicolashenry opened this issue Nov 14, 2017 · 2 comments

Comments

@nicolashenry
Copy link

I found some circular dependencies in moment using madge:

madge --circular src

Processed 399 files (9.1s) 

× Found 15 circular dependencies!

1) lib/create/check-overflow.js > lib/units/month.js > lib/create/utc.js > lib/create/from-anything.js
2) lib/create/from-anything.js > lib/create/from-array.js > lib/create/local.js
3) lib/units/month.js > lib/create/utc.js > lib/create/from-anything.js > lib/create/from-array.js > lib/units/week-calendar-utils.js > lib/units/year.js > lib/moment/get-set.js
4) lib/units/year.js > lib/moment/get-set.js
5) lib/create/check-overflow.js > lib/units/month.js > lib/create/utc.js > lib/create/from-anything.js > lib/create/from-string-and-array.js > lib/create/from-string-and-format.js
6) lib/create/from-string-and-format.js > lib/create/from-string.js
7) lib/create/utc.js > lib/create/from-anything.js > lib/create/from-string-and-array.js > lib/create/from-string-and-format.js > lib/create/from-string.js > lib/units/day-of-week.js
8) lib/units/month.js > lib/create/utc.js > lib/create/from-anything.js > lib/create/from-string-and-array.js > lib/create/from-string-and-format.js > lib/create/from-string.js
9) lib/create/utc.js > lib/create/from-anything.js > lib/create/from-string-and-array.js > lib/create/valid.js
10) lib/units/month.js > lib/create/utc.js > lib/create/from-anything.js > lib/locale/locales.js > lib/locale/base-config.js
11) lib/duration/constructor.js > lib/duration/valid.js
12) lib/duration/create.js > lib/duration/constructor.js > lib/duration/valid.js
13) lib/duration/create.js > lib/units/offset.js
14) lib/duration/create.js > lib/units/offset.js > lib/moment/add-subtract.js
15) test/helpers/common-locale.js > test/qunit.js

I don't have any particular problem with this, I found out by analyzing my own sources that moment is the only package I have which have circular dependencies and I think it could be better without these.

@icambron
Copy link
Member

I agree that the modules could be much better organized, mostly in the sense that they could simply be glued together into fewer modules. However, I don't think "here's a list of circular dependencies" is a good driver for refactoring; the circularities are more of a symptom than the problem itself, and at any rate some amount of circularity is probably inevitable. So thanks for the report but I'm going to close this.

@nicolashenry
Copy link
Author

@icambron Note that rollup is reporting circular dependencies as warnings since some versions :
(it's a bit annoying)

WARN Circular dependency: node_modules\moment\src\lib\create\from-anything.js -> node_modules\moment\src\lib\create\valid.js -> node_modules\moment\src\lib\create\utc.js -> node_modules\moment\src\lib\create\from-anything.js
WARN Circular dependency: node_modules\moment\src\lib\units\month.js -> node_modules\moment\src\lib\moment\get-set.js -> node_modules\moment\src\lib\units\month.js
WARN Circular dependency: node_modules\moment\src\lib\moment\get-set.js -> node_modules\moment\src\lib\units\year.js -> node_modules\moment\src\lib\moment\get-set.js
WARN Circular dependency: node_modules\moment\src\lib\create\local.js -> node_modules\moment\src\lib\create\from-anything.js -> node_modules\moment\src\lib\locale\locales.js -> node_modules\moment\src\lib\locale\base-config.js -> node_modules\moment\src\lib\units\week.js -> node_modules\moment\src\lib\create\local.js
WARN Circular dependency: node_modules\moment\src\lib\create\local.js -> node_modules\moment\src\lib\create\from-anything.js -> node_modules\moment\src\lib\locale\locales.js -> node_modules\moment\src\lib\locale\base-config.js -> node_modules\moment\src\lib\units\week.js -> node_modules\moment\src\lib\units\week-calendar-utils.js -> node_modules\moment\src\lib\create\local.js
WARN Circular dependency: node_modules\moment\src\lib\create\from-string-and-format.js -> node_modules\moment\src\lib\create\from-string.js -> node_modules\moment\src\lib\create\from-string-and-format.js
WARN Circular dependency: node_modules\moment\src\lib\create\local.js -> node_modules\moment\src\lib\create\from-anything.js -> node_modules\moment\src\lib\create\from-string-and-array.js -> node_modules\moment\src\lib\create\from-string-and-format.js -> node_modules\moment\src\lib\create\from-string.js -> node_modules\moment\src\lib\create\from-array.js -> node_modules\moment\src\lib\create\local.js
WARN Circular dependency: node_modules\moment\src\lib\duration\constructor.js -> node_modules\moment\src\lib\duration\valid.js -> node_modules\moment\src\lib\duration\constructor.js
WARN Circular dependency: node_modules\moment\src\lib\duration\create.js -> node_modules\moment\src\lib\duration\constructor.js -> node_modules\moment\src\lib\duration\valid.js -> node_modules\moment\src\lib\duration\create.js
WARN Circular dependency: node_modules\moment\src\lib\duration\create.js -> node_modules\moment\src\lib\units\offset.js -> node_modules\moment\src\lib\duration\create.js
WARN Circular dependency: node_modules\moment\src\lib\moment\add-subtract.js -> node_modules\moment\src\lib\duration\create.js -> node_modules\moment\src\lib\units\offset.js -> node_modules\moment\src\lib\moment\add-subtract.js

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

2 participants