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

Fix electron builds #2745

Merged
merged 13 commits into from
Dec 30, 2024
Merged

Fix electron builds #2745

merged 13 commits into from
Dec 30, 2024

Conversation

imolorhe
Copy link
Collaborator

@imolorhe imolorhe commented Dec 30, 2024

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)

  • Updated the workflow to configure npmrc with node-linker=hoisted for electron builds as specified in the guide
  • Generated a deploy (portable) package that should have all the dependency resolutions handled already
  • Use the portable package to build the electron apps instead of the regular package directory

Summary by Sourcery

Update Electron to v33 and configure electron-builder to work with pnpm.

Enhancements:

  • Updated Electron from v27 to v33.

Build:

  • Configure electron-builder to use a hoisted node_modules for Electron builds.
  • Generate a portable package with all dependencies resolved for Electron builds.
  • Use the portable package to build the Electron apps.

Chores:

  • Updated the project name to altair-repo.

Copy link

sourcery-ai bot commented Dec 30, 2024

Reviewer's Guide by Sourcery

This pull request updates the electron build process to use a portable pnpm package and configures npmrc with node-linker=hoisted to resolve runtime errors.

Sequence diagram for updated electron build process

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Updated workflow to configure npmrc and generate a portable package
  • Added a step to update .npmrc with node-linker=hoisted for electron builds.
  • Added a step to generate a portable package using pnpm deploy.
  • Updated the electron builder to use the portable package instead of the regular package directory.
.github/workflows/_publish-electron.yml
Updated electron dependency
  • Updated electron dependency to ^33.2.1
pnpm-lock.yaml
packages/altair-app/package.json
packages/altair-electron-interop/package.json
packages/altair-electron-settings/package.json
packages/altair-electron/package.json
@types/electron
Updated altair-electron postinstall command
  • Added electron-builder install-app-deps to postinstall command to install required electron dependencies after install.
packages/altair-electron/package.json
Renamed root package name
  • Renamed root package name to altair-repo
package.json
Updated ElectronApp proxy config type
  • Updated the type of proxyConfig from Electron.Config to Electron.ProxyConfig.
packages/altair-electron/src/app/index.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a 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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

github-actions bot commented Dec 30, 2024

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

@imolorhe imolorhe added this pull request to the merge queue Dec 30, 2024
Merged via the queue into master with commit 0b0d801 Dec 30, 2024
15 checks passed
@imolorhe imolorhe deleted the imolorhe/fix-electron-builds branch December 30, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant