-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Fix electron builds #2745
Fix electron builds #2745
Conversation
Reviewer's Guide by SourceryThis pull request updates the electron build process to use a portable pnpm package and configures npmrc with Sequence diagram for updated electron build processsequenceDiagram
participant CI as CI/CD Pipeline
participant PNPM as PNPM
participant Builder as Electron Builder
participant Dist as Distribution
CI->>PNPM: Configure node-linker=hoisted
Note over PNPM: Update .npmrc
CI->>PNPM: Install dependencies
CI->>PNPM: Build project
CI->>PNPM: Create deploy package
Note over PNPM: Generate portable package
PNPM-->>Builder: Pass portable package
Builder->>Dist: Build electron apps
Builder->>Dist: Generate distributions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @imolorhe - I've reviewed your changes - here's some feedback:
Overall Comments:
- The PR includes a major Electron version bump (27 -> 33) - please document the rationale for this significant version change and any potential breaking changes that need to be considered.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Visit the preview URL for this PR (updated for commit 9dc98ca): https://altair-gql--pr2745-imolorhe-fix-electro-fnctx49y.web.app (expires Mon, 06 Jan 2025 11:45:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 02d6323d75a99e532a38922862e269d63351a6cf |
Switching to pnpm for package management causes some runtime errors for the electron apps since electron-builder doesn't work with pnpm by default. Instead additional steps are required.
pnpm/pnpm#3415
https://github.com/orgs/nodejs/discussions/37509#discussion-3240334
electron-userland/electron-builder#6289 (comment)
node-linker=hoisted
for electron builds as specified in the guideSummary by Sourcery
Update Electron to v33 and configure electron-builder to work with pnpm.
Enhancements:
Build:
Chores: