Upgrade Electron from v5 to v7 #8967
Conversation
Co-Authored-By: Josh Abernathy <joshaber@gmail.com>
Co-Authored-By: Josh Abernathy <joshaber@gmail.com>
Co-Authored-By: Josh Abernathy <joshaber@gmail.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
Commit box melts down off the screen Co-Authored-By: evelyn masso <outofambit@github.com>
doesn't jive with our new deps
i think i fixed this incorrectly for the electron 5 upgrade. hopefully this is more correct.
This replaces the patch for osx-notarize removed in 4898ffb, which was causing the build to fail See electron/packager#1069 Co-Authored-By: evelyn masso <outofambit@github.com>
node_modules/.bin/prettier --write app/src/ui/app.tsx
Co-Authored-By: evelyn masso <outofambit@github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
This reverts commit 535149e.
This reverts commit 05e9a3c.
For testing Electron 7 upgrade Co-Authored-By: evelyn masso <outofambit@github.com>
|
Still struggling to run this branch locally on a Windows machine |
There was a problem hiding this comment.
i helped with this but did a once-over to see if i had any lingering concerns. nothing major, just left a couple questions for @kuychaco
| @@ -172,7 +172,8 @@ export class AddExistingRepository extends React.Component< | |||
|
|
|||
| private showFilePicker = async () => { | |||
| const window = remote.getCurrentWindow() | |||
| const directory = remote.dialog.showOpenDialog(window, { | |||
| // eslint-disable-next-line no-sync | |||
| const directory = remote.dialog.showOpenDialogSync(window, { | |||
There was a problem hiding this comment.
i'm wondering if it makes more sense to await the non-sync version of these methods just to avoid the eslint-disable-next-line noise? the no-sync rule is pretty literal (it just flags methods that end in "sync"). i think functionally its the same though.
|
@say25 what issues are you seeing? |
|
@outofambit I'm seeing the application hang in start up and get in a state where the UI doesn't show up. |
|
|
Co-Authored-By: evelyn masso <outofambit@github.com>
There was a problem hiding this comment.
Given that this PR removes the last (and only) patch file (for electron-notarize), could we also remove the patch-package package from packages.json (and from the postinstall script)? See #8555 for the details on what got added in order to support the patch file.
app/src/ui/lib/dark-theme.ts
Outdated
| // remote is an IPC call, so if we know there's no point making this call | ||
| // we should avoid paying the IPC tax | ||
| return remote.systemPreferences.isDarkMode() | ||
| return nativeTheme.shouldUseDarkColors |
There was a problem hiding this comment.
Reading that documentation it would appear to me as if the nativeTheme module is only available in the main process. I believe this will fail at runtime
@say25 I was able to build and run this on Windows. Could you try deleting the |
|
Talked with @outofambit. I did a |
Co-Authored-By: Katrina Uychaco <katrina@github.com>
Co-Authored-By: Markus Olsson <niik@users.noreply.github.com>
Co-Authored-By: Markus Olsson <niik@users.noreply.github.com>
Co-Authored-By: evelyn masso <outofambit@github.com>
say25
added
the
electron
This PR supersedes #8791 (upgrading from Electron v5 to v6) and #8959 (upgrade to Electron v7).
Major changes for v5 to v6:
app.setAppLogsPathMajor changes for v6 to v7:
Worth noting is that upgrading to Electron 7 (or 6) will not remove support for any versions of macOS, according to the electron docs (https://electronjs.org/docs/tutorial/support/history)
Supported Platforms (for v5, v6, and v7)
**macOS** Only 64bit binaries are provided for macOS, and the minimum macOS version supported is macOS 10.10 (Yosemite).Windows
Windows 7 and later are supported, older operating systems are not supported (and do not work).
Both ia32 (x86) and x64 (amd64) binaries are provided for Windows. Running Electron apps on Windows for ARM devices is possible by using the ia32 binary.
We did QA testing to check for known issues for Electron 6 (see #8791 (comment)) and Electron 7 (see #8959 (comment))
Special shout outs to @joshaber, @outofambit, and @tierninho for contributions to this upgrade process 🙏❤️