Add circular dependency detection to CI

[mtrezza]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Circular dependency patterns can cause hard to pin down bugs and should be avoided. Parse Sever currently has no CI check to prevent these.

In fact, Parse Server currently does have 4 circular dependencies that should be addressed:

1) logger.js > Controllers/LoggerController.js > Controllers/AdaptableController.js > Config.js > Controllers/DatabaseController.js > Adapters/Storage/Mongo/MongoStorageAdapter.js > Adapters/Storage/Mongo/MongoTransform.js
2) logger.js > Controllers/LoggerController.js > Controllers/AdaptableController.js > Config.js > Controllers/DatabaseController.js > Adapters/Storage/Mongo/MongoStorageAdapter.js
3) logger.js > Controllers/LoggerController.js > Controllers/AdaptableController.js > Config.js > Controllers/DatabaseController.js
4) Auth.js > RestWrite.js

Related issue: (n/a, meta)

Approach

Adds circular dependency detection to CI via madge, which seems to be a well maintained and popular repo.

Alternative

We could add a custom rule to ESLINT to detect that, but I have no experience with the reliability of that rule; madge seems to be better maintained and is designed specifically for that purpose.

TODOs before merging

• Add entry to changelog

[codecov]

Codecov Report

Merging #7316 (f1aa3b6) into master (0becb0c) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head f1aa3b6 differs from pull request most recent head 52747d8. Consider uploading reports for the commit 52747d8 to get more accurate results

@@            Coverage Diff             @@
##           master    #7316      +/-   ##
==========================================
+ Coverage   93.88%   93.89%   +0.01%     
==========================================
  Files         181      181              
  Lines       13194    13194              
==========================================
+ Hits        12387    12389       +2     
+ Misses        807      805       -2     

Impacted Files	Coverage Δ
src/Auth.js	100.00% <ø> (ø)
src/Controllers/AdaptableController.js	95.65% <ø> (-0.35%)	⬇️
src/Controllers/UserController.js	97.67% <100.00%> (+0.03%)	⬆️
src/RestWrite.js	94.08% <100.00%> (+0.24%)	⬆️
src/Routers/SessionsRouter.js	91.42% <100.00%> (+0.25%)	⬆️
src/Routers/UsersRouter.js	93.78% <100.00%> (+0.03%)	⬆️
src/Adapters/Files/GridFSBucketAdapter.js	80.32% <0.00%> (+0.81%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0becb0c...52747d8. Read the comment docs.

[davimacedo]

That looks good to me, but I believe we should fix the current circular dependencies in this PR, right?

[mtrezza]

Well, ideally yes, but I'm not sure how complex that is. Alternatively we can merge this as a non-required check, knowing that build will fail until another PR solves this.

[dplewis]

I don't this CI check is entirely accurate. Example https://github.com/parse-community/parse-server/blob/master/src/Auth.js#L326

[mtrezza]

I don't this CI check is entirely accurate. Example https://github.com/parse-community/parse-server/blob/master/src/Auth.js#L326

You mean because Auth requires Rest only in method createSession but not on module scope?

parse-server/src/Auth.js

Lines 325 to 326 in 0becb0c

	// We need to import RestWrite at this point for the cyclic dependency it has to it
	const RestWrite = require('./RestWrite');

I am thinking whether this can be an issue regardless.

• Can we say for sure that there is not one scenario in which this can become an issue?
• Do we want to evaluate every circular dependency or categorically prohibit them?

I think whether a certain construction of circular dependency is an issue depends on the inner workings of the node module cache, which can change at any point. Maybe the safe way is to generally avoid them. For my taste, circular dependency is generally a suspicious pattern that often hints to a code quality issue.

[dplewis]

Nice! We should do the JS SDK next. It has a lot more.

[mtrezza]

I removed the circular dependencies. I think we should add this CI check and prohibit them for a more robust code base.

[parseplatformorg]

🎉 This change has been released in version 5.0.0-beta.1

[parseplatformorg]

🎉 This change has been released in version 5.0.0