electron-builder icon indicating copy to clipboard operation
electron-builder copied to clipboard

feat: switch app-builder-bin to node-module-collector to get all production node modules

Open beyondkmp opened this issue 1 year ago • 4 comments

change app-builder-bin to node-module-collector(typescript)

Design

  1. Get node modules tree from "npm list"(npm & yarn) or "pnpm list"
  2. Transform the tree to dependency graph
  3. Transform the graph to hoisted tree
  4. Hoisted tree to node modules array

Support

  • [x] npm
  • [x] pnpm
  • [x] pnpm with hosited
  • [x] yarn1
  • [x] yarn berry(with node-modules)

Performance vs app-builder-bin

This is an IO-intensive task, primarily involving reading files and traversing the entire dependency tree. Node.js is capable of handling such IO-intensive tasks without issues.

Based on my personal testing, there's essentially no difference in performance

Advantages

  1. Perfectly supports pnpm
  2. Optimize packages in the node_modules within the asar file unlike the previous app-builder-bin which only searched for all node_modules without optimizing. For example, if a module has one version in the dev node_modules dependencies, another in the root node_modules, and yet another version with multiple dependencies in the production node_modules, it results in multiple duplicate modules in the production node_modules. Now, hoisting is applied in such situations, reducing these duplicate packages.

beyondkmp avatar Oct 08 '24 15:10 beyondkmp

⚠️ No Changeset found

Latest commit: 1995c187eddcf9588d79442e5e48d058ef510fbf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Oct 08 '24 15:10 changeset-bot[bot]

Deploy Preview for car-park-attendant-cleat-11576 failed.

Name Link
Latest commit d02d442fc96133151cb8ce6530291fbfc7221c59
Latest deploy log https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/67218d5652a4a10008d82240

netlify[bot] avatar Oct 08 '24 15:10 netlify[bot]

ready for review. @mmaietta Please help review when you have time.

beyondkmp avatar Oct 10 '24 16:10 beyondkmp

@mmaietta Please help review again.

beyondkmp avatar Oct 16 '24 09:10 beyondkmp

Note: There's some util functions already existing in builder-util/src/util and builder-util/src/fs that we could leverage to reduce duplicated code and improve debug logging

Excellent suggestions, all changes have been implemented.

beyondkmp avatar Oct 24 '24 23:10 beyondkmp