Update Electron 29 (EOL) to 42, Forge 7.11, pnpm 10 toolchain - #146
Open
simonechecchia wants to merge 1 commit into
Open
Update Electron 29 (EOL) to 42, Forge 7.11, pnpm 10 toolchain#146simonechecchia wants to merge 1 commit into
simonechecchia wants to merge 1 commit into
Conversation
- electron ^42.4.0 (29 has been EOL since 2024) and @types/node ^22 - migrate the moodlemobile:// interceptor from the removed protocol.registerHttpProtocol to protocol.handle (removed in Electron 36) - electron-forge ^7.11.2, which with pnpm requires node-linker=hoisted (.npmrc) - pin packageManager pnpm@10.27.0, let pnpm/action-setup read it, CI on node 22 (Electron 42 requires node >= 22.12) - pnpm-workspace.yaml allows the electron postinstall build script (pnpm 10 skips build scripts by default) Validated locally on Linux: lint, type-check, webpack build, app boot on Electron 42. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the Electron login flow to use the newer custom-protocol handler API and refreshes the project’s Node/pnpm/Electron toolchain for local dev + CI.
Changes:
- Switched custom protocol interception from
protocol.registerHttpProtocoltoprotocol.handle, returning aResponse. - Bumped Electron Forge packages, Electron, Node types, and CI Node version.
- Standardized package manager behavior via
packageManagerand.npmrc(node-linker=hoisted).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/login.ts | Migrates custom protocol handling to protocol.handle and returns an HTTP-style Response. |
| package.json | Declares pnpm as package manager and bumps Electron/Electron Forge + Node typings. |
| .npmrc | Forces pnpm’s node-linker=hoisted to stabilize dependency layout. |
| .github/workflows/test.yml | Updates CI Node version and stops pinning pnpm action version. |
| .github/workflows/publish.yml | Updates CI Node version and stops pinning pnpm action version. |
| .github/workflows/publish-beta.yml | Updates CI Node version and stops pinning pnpm action version. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
src/modules/login.ts:1
req.url.split("token=")[1]is brittle and can mis-parse or throw (e.g., iftokenisn’t present, appears after other query params, or is followed by&...). Use URL parsing (new URL(req.url).searchParams.get("token")) to reliably extract the token value (and then decode) before base64-decoding/splitting.
import path from "path"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
12
to
16
| - uses: pnpm/action-setup@v3 | ||
| with: | ||
| version: 8 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| node-version: 22 | ||
| cache: "pnpm" |
Comment on lines
17
to
+20
| - uses: pnpm/action-setup@v3 | ||
| with: | ||
| version: 8 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| node-version: 22 |
Comment on lines
19
to
+22
| - uses: pnpm/action-setup@v3 | ||
| with: | ||
| version: 8 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| node-version: 22 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Electron 29 has been EOL since 2024, and the last published release (v1.0.3, Sep 2023) still ships Electron 22. This is starting to hurt downstream packaging: the AUR package (
webeep-sync-bin) has to depend on the unmaintainedelectron22, which for some users means building all of Chromium from source just to sync WeBeep files.This PR brings the toolchain to current versions, and would make a new release immediately packageable against a modern system Electron. Related: #133 (new release with pnpm).
What changed
^42.4.0(current stable) and@types/node ^22src/modules/login.ts: the only code change required —protocol.registerHttpProtocolwas removed in Electron 36, so themoodlemobile://token interceptor now usesprotocol.handle. Same semantics: the handler fires on the redirect, extracts the token, and destroys the login window (the returnedResponseis never seen).^7.11.2, which with pnpm requiresnode-linker=hoisted→ added.npmrcpackageManager: pnpm@10.27.0pinned in package.json;pnpm/action-setupnow reads it instead of the hardcodedversion: 8(the v9 lockfile in this PR is not readable by pnpm 8)Validation
On Linux (Arch, Wayland):
pnpm install --frozen-lockfile,pnpm lint,pnpm type-checkall pass, webpack bundles build, and the app boots cleanly on Electron 42 (store manifest migration runs, tray and main window come up). I could not exercise the full SSO login in a sandboxed environment — I'll verify it with my Polimi account and report in a comment.