Skip to content
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

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

Open
wants to merge 93 commits into
base: master
Choose a base branch
from

Conversation

beyondkmp
Copy link
Collaborator

@beyondkmp beyondkmp commented Oct 8, 2024

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

  • npm
  • pnpm
  • pnpm with hosited
  • yarn1
  • 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.

test/src/globTest.ts Outdated Show resolved Hide resolved
@beyondkmp beyondkmp marked this pull request as ready for review October 10, 2024 08:10
@beyondkmp beyondkmp changed the title feat: switch app-builder-bin to node-module-collector feat: switch app-builder-bin to node-module-collector to get all production node modules Oct 10, 2024
@beyondkmp
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants