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

Studio: Update electron-forge to the latest version 7.6.0 #795

Merged

Conversation

nightnei
Copy link
Contributor

@nightnei nightnei commented Jan 9, 2025

Related issues

Proposed Changes

Here we reverted electron-forge to 7.3.1, but the actual issue was connected not to electron-forge, it was about @electron/packager, which we pinned to 18.1.3 version. So actually, there is no sense to keep pinned both - electron-forge and @electron/packager. As a result, since @electron/packager is anyway has pinned version, then we are safe to continue updating electron-forge to new versions, as long as it works well.

With this PR I propose to update electron-forge to the latest version and create a new issue to remove overrides for "@electron/packager": "18.1.3", when the package is fixed.

In the history of commits of this PR you can see that if we unpin override for @electron/packager then it has still the old error. And actually the open issue confirms that the issue still exists.
And I tested - the latest electron-forge works well.

Testing Instructions

  1. Run Studio (npm start)
  2. Smoke test it
  3. Run npm run make
  4. Install Studio from ./out directory
  5. Smoke test it
  6. Obvious, but also take a look that all CI points of this PR are green (The main important one is Windows Dev Build)

@nightnei nightnei marked this pull request as ready for review January 9, 2025 14:38
@nightnei nightnei force-pushed the nightnei/updateElectronForge branch from 7c94f89 to e53605c Compare January 9, 2025 14:55
@nightnei nightnei changed the title chore(electron): update to latest version 33.3.1 Update electron-forge to the latest version 7.6.0 Jan 9, 2025
@nightnei nightnei requested a review from wojtekn January 9, 2025 16:06
@@ -55,6 +55,7 @@ const config: ForgeConfig = {
? []
: [
new MakerDMG(
// @ts-expect-error - https://github.com/electron/forge/issues/3712
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is bug in the MakerDMG - appPath is required by mistake, and it should be optional in the next release. So we will remove this @ts-expect-error when it's fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug is closed. Does it mean it was not released yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They made it here optional, but looks like it wasn't in 7.6.0 release (according to this comment)

Copy link
Contributor

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested approach looks great, thanks for linking to the issue in Electron repository.

The app builds and starts on my local without issues.

@@ -55,6 +55,7 @@ const config: ForgeConfig = {
? []
: [
new MakerDMG(
// @ts-expect-error - https://github.com/electron/forge/issues/3712
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug is closed. Does it mean it was not released yet?

@nightnei nightnei changed the title Update electron-forge to the latest version 7.6.0 Studio: Update electron-forge to the latest version 7.6.0 Jan 10, 2025
@wojtekn wojtekn merged commit 6c88103 into nightnei/updateElectronToLatest33v Jan 14, 2025
12 checks passed
@wojtekn wojtekn deleted the nightnei/updateElectronForge branch January 14, 2025 08: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.

2 participants