Skip to content
This repository has been archived by the owner on Aug 23, 2024. It is now read-only.

Upgrade to electron 22 #397

Merged
merged 15 commits into from
Feb 28, 2023
Merged

Upgrade to electron 22 #397

merged 15 commits into from
Feb 28, 2023

Conversation

kgarner7
Copy link
Contributor

I know there's work on a rewrite, but while this is still here, I'd like to at least provide updated packages.

Changes:

  • electron -> 21
    • Use ipcmain/renderer to send minimize/maximize messages; drop preload.ts
    • use electron-store instead of electron-settings. This does introduce a dependency loop (setDefaultSettings -> i18n -> ...), but it should be OK because i18n is called later in settings
    • to support testing electron store, new mock
    • new electron-notarize to correct package (warning when running yarn)
  • update other packages as possible

NOTE: I had to drop NodeRT because I could not get it to build on windows. I tried to use MediaSession (which is supposed to show notifications), but windows is still confused .-.

@jeffvli
Copy link
Owner

jeffvli commented Oct 30, 2022

I appreciate the work to keep Sonixd up to date!

Couple initial thoughts:

  • Is there a reason for electron-store over electron-settings? My assumption for this change is that electron-settings breaks with Electron 21?
  • I'd really like to keep NodeRT (or at least have a working alternative). I do know of some users who make use of NodeRT's Windows media API, so that would be a breaking change if removed.

I can do a bit of testing on my end to see if I can get it working once I get a chance!

@kgarner7
Copy link
Contributor Author

The reason for using electron-store actually relates to an upgrade from electron 13 -> 14 (https://www.electronjs.org/blog/electron-14-0/). Notably, the removal of the remote module. Unfortunately, electron-settings relies heavily on this (and using it as-is breaks).

Yes, it is possible to re-enable remote, but given that electron-settings hasn't been updated a little over 2 years, it seemed like using a different store that actually uses IPC would be better.

If you can confirm that NodeRT still works (or share how to get it properly set up), that would be lovely! I only have win11 (and had to zap my prior win 10 VM that was able to build sonixd...)

@jugendhacker
Copy link

This will also close #393

@jeffvli
Copy link
Owner

jeffvli commented Feb 27, 2023

@kgarner7 I'm going to attempt to merge this in. I pushed a PR over to your fork to re-add in the nodert dependencies since they still build fine on the publish pipelines. Seems like only the tests are failing.

@kgarner7
Copy link
Contributor Author

Done! Glad to see sonixd still getting some love

@jeffvli jeffvli marked this pull request as ready for review February 28, 2023 02:08
@jeffvli jeffvli force-pushed the upgrade-dependencies branch from 5dde42e to 0dc0da9 Compare February 28, 2023 04:17
@jeffvli jeffvli changed the title Upgrade to electron 21 Upgrade to electron 22 Feb 28, 2023
@jeffvli jeffvli merged commit 6f8de46 into jeffvli:main Feb 28, 2023
@jeffvli jeffvli linked an issue Feb 28, 2023 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Electron version unspported
3 participants