-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
Run in child process instead of lazy loading every dependency #210
Comments
There are some downsides with this though:
What are the upsides? Slightly cleaner code? I also wonder if using a worker thread would be easier. We don't really need to separation a child process provides. We just want the checking to not impact the startup performance of the Node.js app/CLI. |
If I understand this correctly, the whole loading still happens in the main thread, so this does affect the loading, it's just that those tools don't pick it up. If, as is common, code is run asynchronously, then update-notifier eventually competes for the same resources as the main app (because the lazy import is scheduled together with the app's own async callbacks) The only advantage of the current situation is that update-notifier is mostly removed from the very first loop, but then it still runs right after, and probably not as efficiently either (assuming that async imports have some overhead over just parsing a native I get that it's best not to rock the boat, but… I also don't appreciate this package secretly slowing down every bin I ever run. |
Dependencies are no longer lazy loaded. |
I'm open to making everything done in a child process. |
I may be wrong, it seems to me that this change was made simply to hide the package from the very initial load but in practice every dependency is still loaded right after.
update-notifier
already uses a child process to do the checking, so I think it'd be best to do the whole thing there.Related:
The text was updated successfully, but these errors were encountered: