-
Notifications
You must be signed in to change notification settings - Fork 2
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
More compatible dependency discovery #11
Conversation
Cool! Quick question - does this new mechanism to retrieve deps just get "top-level" dependencies listed in package.json? Or does it grab peer/child dependencies as well? |
Neither, actually. It grabs every package that has been require'd (or imported) and discovers its package.json version. So it won't list downstream dependencies, and it might not list top-level either if they haven't been used. Subtle change from before, as before it listed all top level and global, regardless if they were used. But I think this is more relevant, actually. |
Cool cool, that does seem useful! |
Sorry, one last question - if an application installs TrackJS early on in the app startup process, maybe before they require a bunch of other modules, do we run any risk of the cached deps list being much shorter than once the app is working full swing? |
Hmm, that's an interesting question. If a user installed the agent, then went on to load more modules afterwards, they would not be in the module cache, and we wouldn't get them. I think we should move this to build the dependencies when the first error is recorded, rather than during install. This used to be an issue when discovering dependencies was async, but now that it's sync, it should be easy to move. Real good point! Will fix. |
I like that idea! Nice to defer the file system traversal until necessary too! |
Cool. I like the dir access stuff vs shelling out. Definitely seems like a better plan as far as compatibility goes. |
Fixes #9 by changing the approach to discovering dependencies. Instead of getting a listing from npm, which requires access to the shell, we can look for
package.json
files in the current directories that have been cached by the NodeJS module system.Upon the first (and only the first) installation of the agent, dependencies are discovered by pulling a list of cached modules and recursing up the folder structure to find all the package.json files. The logic limits recursion to 10 levels for sanity, and does not look at the same directory twice.
I don't believe we'll have permission problems, because the running process would already need access to these files in order to load them.
Testing this required a standalone test for loading up and caching modules, which appears to be intercepted in the normal Jest unit testing structure.
I also removed a test impacted by #10.