Chia-Network / chia-blockchain-gui

Chia blockchain GUI in electron/react
https://chia.net
Apache License 2.0
332 stars 246 forks source link

pnpm poc #2313

Closed wallentx closed 7 months ago

wallentx commented 7 months ago

Drafting monorepo management via pnmp - yarn v4 monorepo management PR coming shortly for comparison

socket-security[bot] commented 7 months ago

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/electron-playwright-helpers@1.7.1 filesystem Transitive: environment, shell +13 433 kB jjeff
npm/source-map-loader@4.0.2 None +3 567 kB evilebottnawi
npm/thread-loader@4.0.2 filesystem, shell, unsafe Transitive: environment, eval +13 2.21 MB evilebottnawi

🚮 Removed packages: npm/electron-playwright-helpers@1.6.0, npm/lerna-audit@1.3.3, npm/lerna@7.1.5, npm/nx@16.7.2, npm/source-map-loader@4.0.1

View full report↗︎

socket-security[bot] commented 7 months ago

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
AI warning npm/untun@0.1.3
  • Notes: The code appears to have risky practices such as downloading and executing binaries without validation and potential code execution via execSync. It does not contain obvious malware, but there is a risk associated with executing downloaded binaries and scripts without proper validation or integrity checks.
  • Confidence: 1.00
  • Severity: 0.60

View full report↗︎

Next steps

What is an AI detected anomaly?

AI has identified unusual behaviors that may pose a security risk.

An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/untun@0.1.3
wallentx commented 7 months ago

To provide a better description of what I'm exploring in this PR:

  1. In November 2023, node started shipping with corepack, as well as pnpm, and yarn (not enabled by default, but corepack enable enables them), and they also claim they have no intentions to replace npm as the default package manager. Both pnpm and yarn support management of monorepos by default, so you could ditch lerna and nx. Though, @seeden said he didn't want to shift away from npm at the moment.

  2. I was looking at build+install speed differences between using pnpm, yarn, and npm. I also saw that the webpack usage - the longest running task during gui install - might be able to leverage thread-loader for a speedup.

  3. I was always bothered by the install time of the GUI, so I added a build:local script to only generate locales that are on my system, and to also drop usage of cross-env for when building locally.

  4. It was Sunday and I wanted to learn a bit about new nodejs stuff, so I used this as an excuse, and also, my GitHub activity is full of crickets, so I felt anxious and needed to make some commits.

This PR can be ignored for the moment, since @seeden said he wants to stick with NPM, however, my findings from these changes when building on my phone did show a really good speedup. main: DNF. I got impatient and stopped the install at 27 minutes wallentx/pnpm: 19 min

So I might transform this PR to just the speedups if I can replicate them using npm. But at the moment, I'm actually having issues with the webpack config on x86_64 for some reason

seeden commented 7 months ago

@wallentx, thank you for your PR. I understand that pnpm is faster, but we are using monorepo with a flat structure. pnpm creates a hierarchical structure, which is the main reason why you see many webpack issues during the build. Because of this, you need to be very careful with all package versions. You can easily end up with two different versions in your final bundle, which brings "crazy" unexpected errors across the application. We will save 10 minutes, but the development effort will take you a few other hours of your life fixing problems with versions. In terms of Yarn we would like to stay with standard npm for now. NPM added all needed features related to workspaces and everybody knows it because it was in the node js from the beginning.

wallentx commented 7 months ago

Understood, and thanks for the explanation! I'll go ahead and close this, and if I can achieve any speedup just using thread-loader, I'll make a separate PR for that.