-
Notifications
You must be signed in to change notification settings - Fork 120
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
[multi] Reduce dependencies in main and preload layers #3509
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is useful to track dependencies in the project.
This isn't needed anymore due to being included natively in recent electron versions.
This removes the dependency to react in the main process.
This reworks some functions to remove the need to import lodash in the main process stack.
This removes the need to import the wallet pkg as a dependency in the main process code, which removes a lot of transitive dependencies. It also fixes the usage and version CLI args which were broken by previous commits.
This allows dropping the dependency to the ws (websocket) library.
We're not really using any fs-extra features, so the standard fs module is sufficient.
This makes imports of the "config.js" work when generating the dependency graphs.
bgptr
approved these changes
Jun 21, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
alexlyp
approved these changes
Jun 21, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces the dependencies to third party packages in the main electron and preload script layers of the application.
This improves the codebase by relying less on third party code, slightly reduces compilation time and improves security by reducing the attack surface of a possible supply chain attack.
The main changes introduced by this PR are:
node ./scripts/generateDepGraphs.js
wallet
by moving the single outstanding dependency directly into themain_dev
packagewebsocket' (
ws) library by using the
jsonrpc` api on dcrd in post mode (instead of websocket mode)fs-extra
package to the node standardfs
(we don't use any of the additional features offered byfs-extra
)yarn-deduplicate
The script added to generate graphs of the app's dependencies generates a link to npmgraphs.js.org which can be used to compare transitive dependencies as specified in the npm registry:
Before this PR
After this PR:
Note that the dependency to
electron
(andelectron-devtools-installer
in the main layer) are ignored in the graphs due to them being implicitly required, and their absence making the graphs slightly easier to read.After this PR, the main electron layer only has two top-level dependencies which themselves have a large number of transitive deps:
winston
andelectron-store
. Given those two are going to demand slightly more work to replace, I've left them to be refactored in future PRs.