refactor: untangling lib/index.js part 1, monitor #443
Conversation
| @@ -25,6 +25,7 @@ | |||
| "build": "tsc", | |||
| "build-watch": "tsc -w", | |||
| "eslint": "eslint -c .eslintrc src", | |||
| "find-circular": "npm run build && madge --circular ./dist", | |||
There was a problem hiding this comment.
Registry uses madge --extensions js,ts --circular ./src ./test, not sure if there's a benefit of doing that, just sharing :-)
There was a problem hiding this comment.
Yeah, I've copied this from the registry. It looks like madge is not 100% comfy with Typepscript, so the result was that the cycles were found twice - once in src, once in dist.
src/lib/index.js
Outdated
| // The properties represent other modules - so, it's some kind of "world object" | ||
| // that is used to indirectly import modules. | ||
| // The function modifies the object and returns it. | ||
| // This also indtroduces some circular imports. |
There was a problem hiding this comment.
typo, introduces (sorry for nitpicking)
src/lib/monitor.ts
Outdated
| @@ -1,4 +1,4 @@ | |||
| module.exports = monitor; | |||
| export = monitor; | |||
There was a problem hiding this comment.
Do we want to keep this or shall we export function monitor on line 40?
There was a problem hiding this comment.
Hmm. While we're at it, we can patch all the clients, I guess...
kyegupov
force-pushed
the
refactor/snyk-monitor-direct-import
branch
from
2968831 to
b1b514f
Compare
|
🎉 This PR is included in version 1.150.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
lib/monitor.tsthroughsnyk(which islib/index.js) with direct importssee more in Background context
Where should the reviewer start?
lib/index.js
How should this be manually tested?
All the functionality of Snyk CLI should remain intact.
We have a highly problematic module, "lib/index.js" aka "snyk".
It's stateful, it's a proxy for other modules, it introduces a lot of import circles.
Background context
The import cycles we have at the moment are:
This PR is a first baby step towards untangling index.js. It replaces "snyk.monitor" references with direct imports of "monitor", thus expanding the cycle 4 to:
lib/index.js > lib/capture.js > lib/monitor.jsRemoving other cyclic imports will follow.